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

andrewphelpsj · 2025-01-30T16:06:32Z

Hybrid systems might not have this mountpoint setup in the pre-existing rootfs. Here, this change makes it so when we install a hybrid system and copy the seed over, we also install and enable a mount unit that will bind mount /run/mnt/ubuntu-seed (which is mounted in the initramfs) to /var/lib/snapd/seed.

github-actions · 2025-01-30T16:41:56Z

Tue Feb 4 18:08:31 UTC 2025
The following results are from: https://github.com/canonical/snapd/actions/runs/13137962897

Failures:

Executing:

google-nested:ubuntu-22.04-64:tests/nested/manual/muinstaller-real:partial
google-nested:ubuntu-22.04-64:tests/nested/manual/muinstaller-real:plain
google-nested:ubuntu-22.04-64:tests/nested/manual/muinstaller-real:encrypted
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-real:seeded
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-real:partial
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-real:plain
google-nested:ubuntu-24.04-64:tests/nested/manual/muinstaller-real:encrypted
google:ubuntu-20.04-64:tests/main/snapd-snap:lxd

Restoring:

openstack:fedora-41-64:tests/main/snap-services

codecov · 2025-01-30T17:06:46Z

Codecov Report

Attention: Patch coverage is 70.96774% with 9 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@6fa2df6). Learn more about missing BASE report.
Report is 55 commits behind head on master.

Files with missing lines	Patch %	Lines
overlord/devicestate/handlers_install.go	70.96%	6 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #15006   +/-   ##
=========================================
  Coverage          ?   78.23%           
=========================================
  Files             ?     1160           
  Lines             ?   154012           
  Branches          ?        0           
=========================================
  Hits              ?   120496           
  Misses            ?    26094           
  Partials          ?     7422

Flag	Coverage Δ
unittests	`78.23% <70.96%> (?)`

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

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

alfonsosanchezbeato

Thanks, a couple of comments

tests/nested/manual/muinstaller-real/task.yaml

overlord/devicestate/handlers_install.go

alfonsosanchezbeato · 2025-01-30T18:28:57Z

overlord/devicestate/handlers_install.go

@@ -1129,6 +1129,39 @@ func (m *DeviceManager) doInstallFinish(t *state.Task, _ *tomb.Tomb) error {
 		}, perfTimings); err != nil {
 			return fmt.Errorf("cannot copy seed: %w", err)
 		}
+
+		hybrid := systemAndSnaps.Model.Classic() && systemAndSnaps.Model.KernelSnap() != nil


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?

Nice catch, fixed in 7fff430.

alfonsosanchezbeato

LGTM, thanks for the changes

mwhudson

Thanks for getting to this so quickly. I spotted a typo!

I presume to make this work I need to arrange for /var/lib/snapd/seed to be present but empty in the rootfs that I install?

mwhudson · 2025-02-02T21:26:30Z

overlord/devicestate/handlers_install.go

+
+		hybrid := systemAndSnaps.Model.Classic() && systemAndSnaps.Model.KernelSnap() != nil
+		if hybrid {
+			// boot.InitramfsUbuntuSeedDir (/run/mny/ubuntu-data, usually) is a


typo here, mny vs mnt

Good catch, thanks!

andrewphelpsj · 2025-02-02T21:32:52Z

@mwhudson good question, maybe we should verify that is true at install time, or even make it if it isn’t there?

alfonsosanchezbeato · 2025-02-03T12:55:07Z

Thanks for getting to this so quickly. I spotted a typo!

I presume to make this work I need to arrange for /var/lib/snapd/seed to be present but empty in the rootfs that I install?

systemd would take care of creating it if not present

…var/lib/snapd/seed on hybrid system install

…being installed, not current device context

andrewphelpsj requested a review from alfonsosanchezbeato January 30, 2025 16:06

alfonsosanchezbeato requested a review from Meulengracht January 30, 2025 16:11

andrewphelpsj force-pushed the create-hybrid-seed-mount-during-install branch from 4d973e6 to 1bc1544 Compare January 30, 2025 16:43

alfonsosanchezbeato reviewed Jan 30, 2025

View reviewed changes

andrewphelpsj force-pushed the create-hybrid-seed-mount-during-install branch from 1bc1544 to 7fff430 Compare January 30, 2025 18:47

andrewphelpsj requested a review from alfonsosanchezbeato January 30, 2025 18:47

alfonsosanchezbeato approved these changes Jan 31, 2025

View reviewed changes

mwhudson reviewed Feb 2, 2025

View reviewed changes

andrewphelpsj added 2 commits February 4, 2025 09:40

o/devicestate, tests: setup bind mount from /run/mnt/ubuntu-seed to /…

d69103c

…var/lib/snapd/seed on hybrid system install

o/devicestate: correctly construct KernelBootInfo from model that is …

70a55cb

…being installed, not current device context

andrewphelpsj force-pushed the create-hybrid-seed-mount-during-install branch from 7fff430 to 70a55cb Compare February 4, 2025 14:40

ndyer added this to the 2.68 milestone Feb 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

andrewphelpsj commented Jan 30, 2025

github-actions bot commented Jan 30, 2025 •

edited

Loading

codecov bot commented Jan 30, 2025 •

edited

Loading

alfonsosanchezbeato left a comment

alfonsosanchezbeato Jan 30, 2025

andrewphelpsj Jan 30, 2025

alfonsosanchezbeato left a comment

mwhudson left a comment

mwhudson Feb 2, 2025

andrewphelpsj Feb 2, 2025

andrewphelpsj commented Feb 2, 2025

alfonsosanchezbeato commented Feb 3, 2025

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

Are you sure you want to change the base?

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

Conversation

andrewphelpsj commented Jan 30, 2025

github-actions bot commented Jan 30, 2025 • edited Loading

Failures:

Executing:

Restoring:

codecov bot commented Jan 30, 2025 • edited Loading

Codecov Report

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

alfonsosanchezbeato Jan 30, 2025

Choose a reason for hiding this comment

andrewphelpsj Jan 30, 2025

Choose a reason for hiding this comment

alfonsosanchezbeato left a comment

Choose a reason for hiding this comment

mwhudson left a comment

Choose a reason for hiding this comment

mwhudson Feb 2, 2025

Choose a reason for hiding this comment

andrewphelpsj Feb 2, 2025

Choose a reason for hiding this comment

andrewphelpsj commented Feb 2, 2025

alfonsosanchezbeato commented Feb 3, 2025

github-actions bot commented Jan 30, 2025 •

edited

Loading

codecov bot commented Jan 30, 2025 •

edited

Loading