From 67df6d60c3f780fc2a32985a50cb8cb6ac7dceee Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 27 Jun 2024 11:47:12 +0200 Subject: [PATCH 1/2] cirrus: fix broken macos artifacts URL The task got renamed but didn't fix the URL for the machine test task only the artifacts task url was fixed. Fixes 439fe90208 ("Minor: Rename the OSX Cross task") Signed-off-by: Paul Holzinger --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index f4efb1a797..ee4ab27b5c 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -796,7 +796,7 @@ podman_machine_mac_task: clone_script: # artifacts from osx_alt_build_task - mkdir -p $CIRRUS_WORKING_DIR - cd $CIRRUS_WORKING_DIR - - $ARTCURL/OSX%20Cross/repo/repo.tbz + - $ARTCURL/Build%20for%20MacOS%20amd64%2Barm64/repo/repo.tbz - tar xjf repo.tbz # This host is/was shared with potentially many other CI tasks. # The previous task may have been canceled or aborted. From fdb736d282800ec855b78e399b3a8bdd0dab9d77 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 27 Jun 2024 13:54:04 +0200 Subject: [PATCH 2/2] apple virtiofs: fix racy mount setup One problem on FCOS is that the root directory is immutable, as such in order to mount arbitrary paths from the host we must make it mutable again and create these dir on boot in order to be able to mount there. The current logic was racy as it used one unit for each path and they all did chattr -i /; mkdir -p $path; chattr -i / and systemd can run these units in parallel. That means it was possible for another unit to make / immutable before the unit could do the mkdir. I pointed this out on the original PR[1] but we never followed up on it... Now this here changes several things. First have one unit that does the chattr -i / (immutable-root-off.service), it is hooked into remote-fs-pre.target which means it is executed before the network mounts (virtiofs) are done. Then we have another unit that does chattr +i / (immutable-root-on.service) which turn the immutable root back on after remote-fs.target which means all mount are done at this point. Additionally the automount unit is removed because it does not add any value for us and it was borken anyway as it used the virtiofs tag as path so systemd just ignored it. [1] https://github.com/containers/podman/pull/20612#discussion_r1384846241 Fixes #22569 Signed-off-by: Paul Holzinger --- pkg/machine/apple/apple.go | 75 ++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/pkg/machine/apple/apple.go b/pkg/machine/apple/apple.go index 2aa17aa941..b69522524a 100644 --- a/pkg/machine/apple/apple.go +++ b/pkg/machine/apple/apple.go @@ -72,18 +72,7 @@ func GenerateSystemDFilesForVirtiofsMounts(mounts []machine.VirtIoFs) ([]ignitio unitFiles := make([]ignition.Unit, 0, len(mounts)) for _, mnt := range mounts { - // Here we are looping the mounts and for each mount, we are adding two unit files - // for virtiofs. One unit file is the mount itself and the second is to automount it - // on boot. - autoMountUnit := parser.NewUnitFile() - autoMountUnit.Add("Automount", "Where", "%s") - autoMountUnit.Add("Install", "WantedBy", "multi-user.target") - autoMountUnit.Add("Unit", "Description", "Mount virtiofs volume %s") - autoMountUnitFile, err := autoMountUnit.ToString() - if err != nil { - return nil, err - } - + // Create mount unit for each mount mountUnit := parser.NewUnitFile() mountUnit.Add("Mount", "What", "%s") mountUnit.Add("Mount", "Where", "%s") @@ -95,49 +84,57 @@ func GenerateSystemDFilesForVirtiofsMounts(mounts []machine.VirtIoFs) ([]ignitio return nil, err } - virtiofsAutomount := ignition.Unit{ - Enabled: ignition.BoolToPtr(true), - Name: fmt.Sprintf("%s.automount", parser.PathEscape(mnt.Target)), - Contents: ignition.StrToPtr(fmt.Sprintf(autoMountUnitFile, mnt.Tag, mnt.Target)), - } virtiofsMount := ignition.Unit{ Enabled: ignition.BoolToPtr(true), Name: fmt.Sprintf("%s.mount", parser.PathEscape(mnt.Target)), Contents: ignition.StrToPtr(fmt.Sprintf(mountUnitFile, mnt.Tag, mnt.Target)), } - // This "unit" simulates something like systemctl enable virtiofs-mount-prepare@ - enablePrep := ignition.Unit{ - Enabled: ignition.BoolToPtr(true), - Name: fmt.Sprintf("virtiofs-mount-prepare@%s.service", parser.PathEscape(mnt.Target)), - } - - unitFiles = append(unitFiles, virtiofsAutomount, virtiofsMount, enablePrep) + unitFiles = append(unitFiles, virtiofsMount) } - // mount prep is a way to workaround the FCOS limitation of creating directories + // This is a way to workaround the FCOS limitation of creating directories // at the rootfs / and then mounting to them. - mountPrep := parser.NewUnitFile() - mountPrep.Add("Unit", "Description", "Allow virtios to mount to /") - mountPrep.Add("Unit", "DefaultDependencies", "no") - mountPrep.Add("Unit", "ConditionPathExists", "!%f") + immutableRootOff := parser.NewUnitFile() + immutableRootOff.Add("Unit", "Description", "Allow systemd to create mount points on /") + immutableRootOff.Add("Unit", "DefaultDependencies", "no") - mountPrep.Add("Service", "Type", "oneshot") - mountPrep.Add("Service", "ExecStartPre", "chattr -i /") - mountPrep.Add("Service", "ExecStart", "mkdir -p '%f'") - mountPrep.Add("Service", "ExecStopPost", "chattr +i /") + immutableRootOff.Add("Service", "Type", "oneshot") + immutableRootOff.Add("Service", "ExecStart", "chattr -i /") - mountPrep.Add("Install", "WantedBy", "remote-fs.target") - mountPrepFile, err := mountPrep.ToString() + immutableRootOff.Add("Install", "WantedBy", "remote-fs-pre.target") + immutableRootOffFile, err := immutableRootOff.ToString() if err != nil { return nil, err } - virtioFSChattr := ignition.Unit{ - Contents: ignition.StrToPtr(mountPrepFile), - Name: "virtiofs-mount-prepare@.service", + immutableRootOffUnit := ignition.Unit{ + Contents: ignition.StrToPtr(immutableRootOffFile), + Name: "immutable-root-off.service", + Enabled: ignition.BoolToPtr(true), } - unitFiles = append(unitFiles, virtioFSChattr) + unitFiles = append(unitFiles, immutableRootOffUnit) + + immutableRootOn := parser.NewUnitFile() + immutableRootOn.Add("Unit", "Description", "Set / back to immutable after mounts are done") + immutableRootOn.Add("Unit", "DefaultDependencies", "no") + immutableRootOn.Add("Unit", "After", "remote-fs.target") + + immutableRootOn.Add("Service", "Type", "oneshot") + immutableRootOn.Add("Service", "ExecStart", "chattr +i /") + + immutableRootOn.Add("Install", "WantedBy", "remote-fs.target") + immutableRootOnFile, err := immutableRootOn.ToString() + if err != nil { + return nil, err + } + + immutableRootOnUnit := ignition.Unit{ + Contents: ignition.StrToPtr(immutableRootOnFile), + Name: "immutable-root-on.service", + Enabled: ignition.BoolToPtr(true), + } + unitFiles = append(unitFiles, immutableRootOnUnit) return unitFiles, nil }