Skip to content

daemon, o/snapstate: implement snapstate.InstallPath in terms of refresh#17166

Open
andrewphelpsj wants to merge 2 commits into
canonical:masterfrom
andrewphelpsj:seed-refresh-file-update
Open

daemon, o/snapstate: implement snapstate.InstallPath in terms of refresh#17166
andrewphelpsj wants to merge 2 commits into
canonical:masterfrom
andrewphelpsj:seed-refresh-file-update

Conversation

@andrewphelpsj

Copy link
Copy Markdown
Member

This lets us use the existing seed-refresh implementation for free during single-path installation. Tests are added for this behavior.

@github-actions github-actions Bot added the Needs Documentation -auto- Label automatically added which indicates the change needs documentation label Jun 5, 2026
@codecov

codecov Bot commented Jun 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.12%. Comparing base (49b363c) to head (cf03f3b).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #17166      +/-   ##
==========================================
- Coverage   79.12%   79.12%   -0.01%     
==========================================
  Files        1385     1388       +3     
  Lines      193310   193402      +92     
  Branches     2466     2466              
==========================================
+ Hits       152954   153020      +66     
- Misses      31165    31185      +20     
- Partials     9191     9197       +6     
Flag Coverage Δ
unittests 79.12% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@miguelpires miguelpires left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question

Comment thread overlord/snapstate/seed.go Outdated
Comment thread overlord/snapstate/snapstate.go
@andrewphelpsj andrewphelpsj requested a review from miguelpires June 5, 2026 17:06
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Thu Jun 11 18:42:06 UTC 2026
The following results are from: https://github.com/canonical/snapd/actions/runs/27362591376

Failures:

Preparing:

  • openstack:ubuntu-20.04-64:tests/main/nss-modules:winbind
  • openstack:ubuntu-24.04-64:tests/main/apparmor-prompting-integration-tests:create_multiple_deny

Executing:

  • openstack:arch-linux-64:tests/main/desktop-file-ids
  • openstack-ext:ubuntu-22.04-64:tests/nested/manual/seed-refresh-back-to-revision:revert_app
  • openstack:ubuntu-core-18-64:tests/main/security-device-cgroups:kmsg
  • openstack:ubuntu-core-20-64:tests/main/catalog-update
  • openstack:ubuntu-core-24-64:tests/main/snap-user-service-socket-activation
  • openstack:ubuntu-26.04-64:tests/main/interfaces-snap-interfaces-requests-control
  • openstack:ubuntu-20.04-64:tests/main/degraded
  • openstack:ubuntu-24.04-64:tests/main/static

Restoring:

  • openstack-ext:ubuntu-22.04-64:tests/nested/manual/seed-refresh-back-to-revision:revert_app
  • openstack-ext:ubuntu-22.04-64:tests/nested/manual/
  • openstack-ext:ubuntu-22.04-64:
  • openstack:ubuntu-24.04-64:tests/main/apparmor-prompting-integration-tests:create_multiple_deny
  • openstack:ubuntu-24.04-64:tests/main/
  • openstack:ubuntu-24.04-64:
  • openstack:ubuntu-24.04-64:tests/main/static
  • openstack:ubuntu-24.04-64:tests/main/
  • openstack:ubuntu-24.04-64:

Skipped tests from snapd-testing-skip

If you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list (without variants) of the below tests you wish to run (unskip plus test list must be valid yaml)

  • openstack:ubuntu-24.04-64:tests/main/i18n
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-flag-restart
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:audio_record_single
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:audio_record_timespan_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:audio_record_timespan_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_multiple_actioned_by_other_pid_always_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_multiple_actioned_by_other_pid_always_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_multiple_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_multiple_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_multiple_not_actioned_by_other_pid_single_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_multiple_not_actioned_by_other_pid_single_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_write_chmod_same_fd_single_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_write_chmod_same_path_single_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:create_write_write_same_path_single_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:download_file_conflict
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:download_file_defaults
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:download_file_safer
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:read_single_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:read_single_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:special_characters
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:timespan_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:timespan_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:write_read_multiple_actioned_by_other_pid_allow_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:write_read_multiple_actioned_by_other_pid_deny_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:write_single_allow
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests:write_single_deny
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-prompt-restoration
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:audiorecord_allow_forever
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:audiorecord_allow_session
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:audiorecord_allow_single
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:audiorecord_allow_timespan
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:audiorecord_deny_forever
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:audiorecord_deny_session
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:audiorecord_deny_single
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:audiorecord_deny_timespan
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:camera_allow_forever
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:camera_allow_session
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:camera_allow_single
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:camera_allow_timespan
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:camera_deny_forever
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:camera_deny_session
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:camera_deny_single
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:camera_deny_timespan
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:home_allow_forever
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:home_allow_session
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:home_allow_single
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:home_allow_timespan
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:home_deny_forever
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:home_deny_session
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:home_deny_single
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke:home_deny_timespan
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-snapd-startup
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-support
  • openstack:ubuntu-26.04-64:tests/main/i18n
  • openstack:ubuntu-26.04-64:tests/main/interfaces-requests-activates-handlers

@andrewphelpsj andrewphelpsj force-pushed the seed-refresh-file-update branch 2 times, most recently from 0894708 to 5b7a043 Compare June 10, 2026 16:41
miguelpires
miguelpires previously approved these changes Jun 11, 2026

@miguelpires miguelpires left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@pedronis pedronis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

behavior and symmetry question

Comment thread overlord/snapstate/snapstate.go Outdated
// Note that the state must be locked by the caller.
// The provided SideInfo can contain just a name which results in a
// local revision and sideloading, or full metadata in which case it
// the snap will appear as installed from the store.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the comment here probably needs to explain the behavioral differences between this and InstallOne with PathInstallGoal or maybe the issue is that we want InstallOne with that to also do the right thing?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update the comment on the differences, though I'm not clear on what we want InstallOne with that to also do the right thing might mean?

Do you mean that we should consider making that trigger a seed-refresh?

This lets us use the existing seed-refresh implementation for free
during single-path installation. Tests are added for this behavior.
@andrewphelpsj andrewphelpsj force-pushed the seed-refresh-file-update branch from 5b7a043 to cf03f3b Compare June 11, 2026 16:41
@andrewphelpsj andrewphelpsj requested a review from pedronis June 11, 2026 16:41
@pedronis pedronis dismissed miguelpires’s stale review June 15, 2026 11:57

will need to be reviewed from scratch

@pedronis pedronis removed their request for review June 15, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Documentation -auto- Label automatically added which indicates the change needs documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants