o/snapstate: fix validation sets restoring and snap revert on failed refresh #11826
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maybeRestoreValidationSetsAndRevertSnaps() doesn't conflict with the main change where check-rerefresh runs. Add more tests.
stolowski
force-pushed
the
validation-sets/fix-refresh-revert
branch
from
7fc34fc
to
5e7b9f4
Compare
stolowski
added
the
Needs Samuele review
refreshed snaps (and can be reverted).
MiguelPires
approved these changes
May 30, 2022
overlord/snapstate/handlers.go
Outdated
| for sn := range verr.WrongRevisionSnaps { | ||
| wrongRevSnaps = append(wrongRevSnaps, sn) | ||
| } | ||
| logger.Debugf("refreshed snaps: %s, snaps at wrong revisions: %s", strutil.Quoted(refreshedSnaps), wrongRevSnaps) |
There was a problem hiding this comment.
Should wrongRevSnaps also be quoted?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The attempt to revert snaps in
maybeRestoreValidationSetsAndRevertSnaps(in check-rerefresh, after partially successfull or failed refresh) would incorrectly conflict with own change, so revert wouldn't actually happen and check-rerefresh would report conflict error. This wasn't detected by the existing test due to insufficient mocking of the install change.This PR passes own change around down to Revert (so conflict with own change gets ignored) and improves the test of actual revert. New tests cases for MaybeRestoreValidationSetsAndRevertSnaps are added to cover all special cases inside this function.
The related bug is https://bugs.launchpad.net/snapd/+bug/1974188 but I'm still not sure why check-rerefresh didn't report any error there (yet the system ended up in "invalid" enforcing state; there is likely another problem in this area).