Make snapshotter root dynamic based on the reported mountpoint #1971
Conversation
Signed-off-by: David Cassany <dcassany@suse.com>
f278159 to
4b08eb8
Compare
Signed-off-by: David Cassany <dcassany@suse.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1971 +/- ##
==========================================
- Coverage 73.11% 72.97% -0.15%
==========================================
Files 74 74
Lines 8671 8722 +51
==========================================
+ Hits 6340 6365 +25
- Misses 1819 1838 +19
- Partials 512 519 +7 ☔ View full report in Codecov by Sentry. |
| @@ -70,6 +70,7 @@ if [ "${snapshotter}" == "btrfs" ]; then | |||
| echo "[Unit]" | |||
| echo "Before=initrd-root-fs.target" | |||
| echo "DefaultDependencies=no" | |||
| echo "RequiresMountsFor=/sysroot" | |||
There was a problem hiding this comment.
To ensure root is mounted first and so that appears a the top of /proc/mounts.
pkg/action/upgrade.go
Outdated
| @@ -256,7 +256,7 @@ func (u *UpgradeAction) Run() (err error) { | |||
| } | |||
|
|
|||
| // Init snapshotter | |||
| err = u.snapshotter.InitSnapshotter(u.spec.Partitions.State.MountPoint) | |||
| err = u.snapshotter.InitSnapshotter(u.spec.Partitions.State.MountPoint, u.spec.Partitions.EFI.MountPoint) | |||
There was a problem hiding this comment.
This is a draft to actually verify the fix, however I am starting to think that probably we should just define the method having u.spec.Partitions as input, instead of just the mountpoints.
@frelon any thoughts or ideas regarding this? We could even consider moving the responsability of mounting and setting the partitions as needed here in InitSnapshotter, now it just happens right before calling this method.
There was a problem hiding this comment.
It would make sense for the snapshotter to know about the EFI partition if we want to support having the kernel/initrd there in the future as well.
There was a problem hiding this comment.
In the latest implementation it takes the State partition struct and the EFI mountpoint. Adding to the EFI partition struct should be pretty trivial in the future too if needed. The actual mess was mostly with the state partition and its multiple mount points. For EFI having the appropriate path is enough for now.
Signed-off-by: David Cassany <dcassany@suse.com>
7ed64aa to
fd93c1b
Compare
Signed-off-by: David Cassany <dcassany@suse.com>
Signed-off-by: David Cassany <dcassany@suse.com>
| return nil | ||
| } | ||
|
|
||
| func findStateMount(runner v1.Runner, device string) (rootDir string, stateMount string, err error) { |
There was a problem hiding this comment.
This is only executed on Active and Passive modes and essentially checks the current state mountpoints and sets the current snapshot root to the snapshotter root and the sets the state mountpoint to the actual top level volume (this is the /run/initramfs/elemental-state mountpoint).
An attempt to fix #1972 and fix #1973