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

o/devicestate, tests: setup bind mount from /run/mnt/ubuntu-seed to /var/lib/snapd/seed on hybrid system install #15006

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions overlord/devicestate/devicestate_install_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"os"
"os/exec"
"path/filepath"
"strings"

. "gopkg.in/check.v1"

Expand All @@ -45,6 +46,7 @@ import (
"github.com/snapcore/snapd/secboot/keys"
"github.com/snapcore/snapd/seed"
"github.com/snapcore/snapd/snap"
"github.com/snapcore/snapd/systemd"
"github.com/snapcore/snapd/testutil"
"github.com/snapcore/snapd/timings"
)
Expand Down Expand Up @@ -421,7 +423,7 @@ func (s *deviceMgrInstallAPISuite) testInstallFinishStep(c *C, opts finishStepOp
Name: "pc-kernel",
Revision: snap.R(1),
MountPoint: kernelMountDir,
IsCore: false,
IsCore: !opts.installClassic,
ModulesComps: modulesComps,
NeedsDriversTree: true,
})
Expand Down Expand Up @@ -588,10 +590,44 @@ func (s *deviceMgrInstallAPISuite) testInstallFinishStep(c *C, opts finishStepOp
c.Check(mountVolsCalls, Equals, 1)
c.Check(saveStorageTraitsCalls, Equals, 1)

if !opts.installClassic {
if !opts.installClassic || opts.hasSystemSeed {
c.Check(seedCopyCalled, Equals, true)
}

// on hybrid systems (classic systems that'll have a seed), we expect a bind
// mount from the seed that is mounted /run/mnt/ubuntu-seed from the
// initramfs
unitFile := systemd.EscapeUnitNamePath(dirs.SnapSeedDir) + ".mount"
unitPath := filepath.Join(
boot.InstallUbuntuDataDir,
"etc/systemd/system",
unitFile,
)
if opts.installClassic && opts.hasSystemSeed {
unitContents, err := os.ReadFile(unitPath)
c.Assert(err, IsNil)

contents := string(unitContents)
c.Check(strings.Contains(contents, fmt.Sprintf("Where=%s", dirs.SnapSeedDir)), Equals, true)
c.Check(strings.Contains(contents, fmt.Sprintf("What=%s", boot.InitramfsUbuntuSeedDir)), Equals, true)
c.Check(strings.Contains(contents, "Options=bind"), Equals, true)
c.Check(strings.Contains(contents, "Type=none"), Equals, true)
c.Check(strings.Contains(contents, "Before=snapd.mounts.target"), Equals, true)
c.Check(strings.Contains(contents, "WantedBy=snapd.mounts.target"), Equals, true)

unitSymlinkPath := filepath.Join(boot.InstallUbuntuDataDir, "etc/systemd/system/snapd.mounts.target.wants", unitFile)
info, err := os.Lstat(unitSymlinkPath)
c.Assert(err, IsNil)

c.Check(info.Mode()&os.ModeSymlink != 0, Equals, true)
linkTarget, err := os.Readlink(unitSymlinkPath)
c.Assert(err, IsNil)

c.Check(linkTarget, Equals, filepath.Join(dirs.GlobalRootDir, "etc/systemd/system", unitFile))
} else {
c.Check(unitPath, testutil.FileAbsent)
}

snapdVarDir := "mnt/ubuntu-data/system-data/var/lib/snapd"
if opts.installClassic {
snapdVarDir = "mnt/ubuntu-data/var/lib/snapd"
Expand Down
40 changes: 35 additions & 5 deletions overlord/devicestate/handlers_install.go
Original file line number Diff line number Diff line change
Expand Up @@ -1076,13 +1076,9 @@

snapInfos := systemAndSnaps.InfosByType
snapSeeds := systemAndSnaps.SeedSnapsByType
deviceCtx, err := DeviceCtx(st, t, nil)
if err != nil {
return fmt.Errorf("cannot get device context: %v", err)
}

// Find out kernel-modules components in the seed
isCore := !deviceCtx.Classic()
isCore := !systemAndSnaps.Model.Classic()
kBootInfo := kBootInfo(systemAndSnaps, kernMntPoint, mntPtForComps, isCore)

logger.Debugf("writing content to partitions")
Expand Down Expand Up @@ -1129,6 +1125,40 @@
}, perfTimings); err != nil {
return fmt.Errorf("cannot copy seed: %w", err)
}

hybrid := systemAndSnaps.Model.Classic() && systemAndSnaps.Model.KernelSnap() != nil

Choose a reason for hiding this comment

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

This check made me realize that a check that we do a few lines before (https://github.com/canonical/snapd/blob/master/overlord/devicestate/handlers_install.go#L1085-L1086) is actually wrong as indeed the device context seen by doInstallFinish is the one of the installer, not the one for the model we are installing (thanks for this!). It is not surprising that this has not exploded yet as we don't have that many tests for using the installer API to install UC. Would you mind fixing that too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch, fixed in 7fff430.

if hybrid {
// boot.InitramfsUbuntuSeedDir (/run/mnt/ubuntu-data, usually) is a
// mountpoint on hybrid system that is set up in the initramfs.
// setting up this bind mount ensures that the system can be seeded
// on boot.
unitName, _, err := systemd.EnsureMountUnitFileContent(&systemd.MountUnitOptions{
Lifetime: systemd.Persistent,
Description: "Bind mount seed partition",
What: boot.InitramfsUbuntuSeedDir,
PreventRestartIfModified: true,
Where: dirs.SnapSeedDir,
Fstype: "none",
Options: []string{"bind", "ro"},
RootDir: boot.InstallUbuntuDataDir,
})
if err != nil {
return fmt.Errorf("cannot create mount unit for seed: %w", err)
}

Check warning on line 1147 in overlord/devicestate/handlers_install.go

View check run for this annotation

Codecov / codecov/patch

overlord/devicestate/handlers_install.go#L1146-L1147

Added lines #L1146 - L1147 were not covered by tests

enabledUnitDir := filepath.Join(boot.InstallUbuntuDataDir, "etc/systemd/system/snapd.mounts.target.wants")
if err := os.MkdirAll(enabledUnitDir, 0755); err != nil {
return fmt.Errorf("cannot create directory for systemd unit: %w", err)
}

Check warning on line 1152 in overlord/devicestate/handlers_install.go

View check run for this annotation

Codecov / codecov/patch

overlord/devicestate/handlers_install.go#L1151-L1152

Added lines #L1151 - L1152 were not covered by tests

err = os.Symlink(
filepath.Join(dirs.GlobalRootDir, "etc/systemd/system", unitName),
filepath.Join(enabledUnitDir, unitName),
)
if err != nil {
return fmt.Errorf("cannot create symlink to enable systemd unit: %w", err)
}

Check warning on line 1160 in overlord/devicestate/handlers_install.go

View check run for this annotation

Codecov / codecov/patch

overlord/devicestate/handlers_install.go#L1159-L1160

Added lines #L1159 - L1160 were not covered by tests
}
}

if err := installSaveStorageTraits(systemAndSnaps.Model, mergedVols, encryptSetupData); err != nil {
Expand Down
25 changes: 21 additions & 4 deletions tests/lib/muinstaller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,6 @@ func fillPartiallyDefinedVolume(vol *gadget.Volume, bootDevice string) error {
}

func run(seedLabel, bootDevice, rootfsCreator, optionalInstallPath string) error {
isCore := rootfsCreator == ""
logger.Noticef("installing on %q", bootDevice)

cli := client.New(nil)
Expand Down Expand Up @@ -567,14 +566,21 @@ func run(seedLabel, bootDevice, rootfsCreator, optionalInstallPath string) error
if err != nil {
return fmt.Errorf("cannot create filesystems: %v", err)
}

hasSystemSeed := checkForRole(details, gadget.SystemSeed)
isCore := rootfsCreator == ""
if isCore || !hasSystemSeed {
if err := copySeedToDataPartition(); err != nil {
return fmt.Errorf("cannot create seed on data partition: %v", err)
}
}

if !isCore {
if err := createClassicRootfsIfNeeded(rootfsCreator); err != nil {
return fmt.Errorf("cannot create classic rootfs: %v", err)
}
}
if err := copySeedToDataPartition(); err != nil {
return fmt.Errorf("cannot create seed on data partition: %v", err)
}

if err := unmountFilesystems(mntPts); err != nil {
return fmt.Errorf("cannot unmount filesystems: %v", err)
}
Expand All @@ -586,6 +592,17 @@ func run(seedLabel, bootDevice, rootfsCreator, optionalInstallPath string) error
return nil
}

func checkForRole(details *client.SystemDetails, role string) bool {
for _, v := range details.Volumes {
for _, vs := range v.Structure {
if vs.Role == role {
return true
}
}
}
return false
}

func main() {
seedLabel := flag.String("label", "", "seed label (required)")
bootDevice := flag.String("device", "", "target device (required)")
Expand Down
1 change: 0 additions & 1 deletion tests/lib/tools/setup_nested_hybrid_system.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ run_muinstaller() {
./classic-seed

mv ./classic-seed/system-seed/systems/* "./classic-seed/system-seed/systems/${label}"
cp -a ./classic-seed/system-seed/ /var/lib/snapd/seed

if [ -n "${store_dir}" ]; then
# if we have a store setup, then we should take it down for now
Expand Down
7 changes: 7 additions & 0 deletions tests/nested/manual/muinstaller-real/task.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,13 @@ execute: |
echo "asserted snaps"
remote.exec "ls /run/mnt/ubuntu-seed/snaps"

# on hybrid systems, we bind mount the seed from the mount that is set up by
# snap-bootstrap (/run/mnt/ubuntu-seed) to /var/lib/snapd/seed. we inject the
# systemd mount unit for the bind mount at install time, during the
# "install-finish" task.
test "$(remote.exec "findmnt /run/mnt/ubuntu-seed/ -o source -n")" = "$(remote.exec "findmnt /var/lib/snapd/seed -o source -n")"
remote.exec "systemctl is-active var-lib-snapd-seed.mount"

# check for unasserted snaps
for sn in pc pc-kernel snapd; do
sn_version=$(remote.exec "snap list ${sn}" | awk 'NR != 1 { print $2 }' | sed 's/+fake1//')
Expand Down
Loading