Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure subvolume snapshots is mounted to list snapshots #2098

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

davidcassany
Copy link
Contributor

@davidcassany davidcassany commented Jun 10, 2024

This is fixes an issue about how we were updating the state.yaml file after upgrades. For the corner case of running upgrades from the recovery system the <state_partition>/@/.snapshots is unmounted before upgrading the state.yaml and this mount is required to properly list available snapshots and reflect that in state.yaml.

Signed-off-by: David Cassany <dcassany@suse.com>
@davidcassany davidcassany added the kind/bug Something isn't working label Jun 10, 2024
@davidcassany davidcassany requested a review from a team as a code owner June 10, 2024 09:47
Copy link
Contributor

@kkaempf kkaempf left a comment

Choose a reason for hiding this comment

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

I see you provide a snapshotsMount implementation but where's the corresponding snapshotsUmount one ?

@davidcassany
Copy link
Contributor Author

Yes, in fact there used to be only a snapshotsUmount one, but no snapshotsMount. It was this way because there was the assumption it was only mounted once and at the very same stage for all cases. This turns not to be accurate enough when upgrading the snapshot from the recovery system (likely to be a squashfs image on a different partition).

@@ -68,6 +68,7 @@ type Btrfs struct {
installing bool
snapperArgs []string
snapshotsUmount func() error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the umount function, which was already coded and used before this PR.

Copy link
Member

@fgiudici fgiudici left a comment

Choose a reason for hiding this comment

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

LGTM

@davidcassany davidcassany merged commit 897d872 into rancher:main Jun 10, 2024
29 checks passed
@davidcassany davidcassany deleted the fix_upgrade_from_recovery branch June 10, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants