Use parser.UnitFile

Uses the systemd unit file parser to build unit files instead of having
them be just blocks of hard-coded strings.

Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>

pkg/machine/applehv/machine.go

@@ -26,6 +26,7 @@ import (
 	"github.com/containers/podman/v4/pkg/machine/vmconfigs"
     "github.com/containers/podman/v4/pkg/machine/ignition"
 	"github.com/containers/podman/v4/pkg/strongunits"
+	"github.com/containers/podman/v4/pkg/systemd/parser"
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/podman/v4/utils"
 	"github.com/containers/storage/pkg/lockfile"
@@ -46,21 +47,13 @@ const (
 	apiUpTimeout         = 20 * time.Second
 )
 
-// appleHVReadyUnit is a unit file that sets up the virtual serial device
-// where when the VM is done configuring, it will send an ack
-// so a listening host knows it can begin interacting with it
-const appleHVReadyUnit = `[Unit]
-Requires=dev-virtio\\x2dports-%s.device
-After=remove-moby.service sshd.socket sshd.service
-OnFailure=emergency.target
-OnFailureJobMode=isolate
-[Service]
-Type=oneshot
-RemainAfterExit=yes
-ExecStart=/bin/sh -c '/usr/bin/echo Ready | socat - VSOCK-CONNECT:2:1025'
-[Install]
-RequiredBy=default.target
-`
+// VfkitHelper describes the use of vfkit: cmdline and endpoint
+type VfkitHelper struct {
+	LogLevel        logrus.Level
+	Endpoint        string
+	VfkitBinaryPath *define.VMFile
+	VirtualMachine  *vfConfig.VirtualMachine
+}
 
 type MacMachine struct {
 	// ConfigPath is the fully qualified path to the configuration file
@@ -274,10 +267,15 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 		return false, err
 	}
 
+	readyUnitFile, err := createReadyUnitFile()
+	if err != nil {
+		return false, err
+	}
+
 	builder.WithUnit(ignition.Unit{
 		Enabled:  ignition.BoolToPtr(true),
 		Name:     "ready.service",
-		Contents: ignition.StrToPtr(fmt.Sprintf(appleHVReadyUnit, "vsock")),
+		Contents: ignition.StrToPtr(readyUnitFile),
 	})
 	builder.WithUnit(generateSystemDFilesForVirtiofsMounts(virtiofsMnts)...)
 
@@ -288,6 +286,13 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	return err == nil, err
 }
 
+func createReadyUnitFile() (string, error) {
+	readyUnit := ignition.DefaultReadyUnitFile()
+	readyUnit.Add("Unit", "Requires", "dev-virtio\\x2dports-vsock.device")
+	readyUnit.Add("Service", "ExecStart", "/bin/sh -c '/usr/bin/echo Ready | socat - VSOCK-CONNECT:2:1025'")
+	return readyUnit.ToString()
+}
+
 func (m *MacMachine) removeSSHKeys() error {
 	if err := os.Remove(fmt.Sprintf("%s.pub", m.IdentityPath)); err != nil {
 		logrus.Error(err)
@@ -1111,31 +1116,34 @@ func generateSystemDFilesForVirtiofsMounts(mounts []machine.VirtIoFs) []ignition
 		// 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 := `[Automount]
-Where=%s
-[Install]
-WantedBy=multi-user.target
+		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 {
+			logrus.Warnf(err.Error())
+		}
 
-[Unit]
-Description=Mount virtiofs volume %s
-`
-		mountUnit := `[Mount]
-What=%s
-Where=%s
-Type=virtiofs
+		mountUnit := parser.NewUnitFile()
+		mountUnit.Add("Mount", "What", "%s")
+		mountUnit.Add("Mount", "Where", "%s")
+		mountUnit.Add("Mount", "Type", "virtiofs")
+		mountUnit.Add("Install", "WantedBy", "multi-user.target")
+		mountUnitFile, err := mountUnit.ToString()
+		if err != nil {
+			logrus.Warnf(err.Error())
+		}
 
-[Install]
-WantedBy=multi-user.target
-`
 		virtiofsAutomount := ignition.Unit{
 			Enabled:  ignition.BoolToPtr(true),
 			Name:     fmt.Sprintf("%s.automount", mnt.Tag),
-			Contents: ignition.StrToPtr(fmt.Sprintf(autoMountUnit, mnt.Target, mnt.Target)),
+			Contents: ignition.StrToPtr(fmt.Sprintf(autoMountUnitFile, mnt.Target, mnt.Target)),
 		}
 		virtiofsMount := ignition.Unit{
 			Enabled:  ignition.BoolToPtr(true),
 			Name:     fmt.Sprintf("%s.mount", mnt.Tag),
-			Contents: ignition.StrToPtr(fmt.Sprintf(mountUnit, mnt.Tag, mnt.Target)),
+			Contents: ignition.StrToPtr(fmt.Sprintf(mountUnitFile, mnt.Tag, mnt.Target)),
 		}
 
 		// This "unit" simulates something like systemctl enable virtiofs-mount-prepare@
@@ -1149,23 +1157,24 @@ WantedBy=multi-user.target
 
 	// mount prep is a way to workaround the FCOS limitation of creating directories
 	// at the rootfs / and then mounting to them.
-	mountPrep := `
-[Unit]
-Description=Allow virtios to mount to /
-DefaultDependencies=no
-ConditionPathExists=!%f
+	mountPrep := parser.NewUnitFile()
+	mountPrep.Add("Unit", "Description", "Allow virtios to mount to /")
+	mountPrep.Add("Unit", "DefaultDependencies", "no")
+	mountPrep.Add("Unit", "ConditionPathExists", "!%f")
 
-[Service]
-Type=oneshot
-ExecStartPre=chattr -i /
-ExecStart=mkdir -p '%f'
-ExecStopPost=chattr +i /
+	mountPrep.Add("Service", "Type", "oneshot")
+	mountPrep.Add("Service", "ExecStartPre", "chattr -i /")
+	mountPrep.Add("Service", "ExecStart", "mkdir -p '%f'")
+	mountPrep.Add("Service", "ExecStopPost", "chattr +i /")
+
+	mountPrep.Add("Install", "WantedBy", "remote-fs.target")
+	mountPrepFile, err := mountPrep.ToString()
+	if err != nil {
+		logrus.Warnf(err.Error())
+	}
 
-[Install]
-WantedBy=remote-fs.target
-`
 	virtioFSChattr := ignition.Unit{
-		Contents: ignition.StrToPtr(mountPrep),
+		Contents: ignition.StrToPtr(mountPrepFile),
 		Name:     "virtiofs-mount-prepare@.service",
 	}
 	unitFiles = append(unitFiles, virtioFSChattr)

pkg/machine/hyperv/machine.go

@@ -24,6 +24,7 @@ import (
 	"github.com/containers/podman/v4/pkg/machine/vmconfigs"
 	"github.com/containers/podman/v4/pkg/machine/ignition"
 	"github.com/containers/podman/v4/pkg/strongunits"
+	"github.com/containers/podman/v4/pkg/systemd/parser"
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/podman/v4/utils"
 	"github.com/containers/storage/pkg/lockfile"
@@ -46,40 +47,6 @@ const (
 	apiUpTimeout         = 20 * time.Second
 )
 
-// hyperVReadyUnit is a unit file that sets up the virtual serial device
-// where when the VM is done configuring, it will send an ack
-// so a listening host knows it can begin interacting with it
-//
-// VSOCK-CONNECT:2 <- shortcut to connect to the hostvm
-const hyperVReadyUnit = `[Unit]
-After=remove-moby.service sshd.socket sshd.service
-After=systemd-user-sessions.service
-OnFailure=emergency.target
-OnFailureJobMode=isolate
-[Service]
-Type=oneshot
-RemainAfterExit=yes
-ExecStart=/bin/sh -c '/usr/bin/echo Ready | socat - VSOCK-CONNECT:2:%d'
-[Install]
-RequiredBy=default.target
-`
-
-// hyperVVsockNetUnit is a systemd unit file that calls the vm helper utility
-// needed to take traffic from a network vsock0 device to the actual vsock
-// and onto the host
-const hyperVVsockNetUnit = `
-[Unit]
-Description=vsock_network
-After=NetworkManager.service
-
-[Service]
-ExecStart=/usr/libexec/podman/gvforwarder -preexisting -iface vsock0 -url vsock://2:%d/connect
-ExecStartPost=/usr/bin/nmcli c up vsock0
-
-[Install]
-WantedBy=multi-user.target
-`
-
 const hyperVVsockNMConnection = `
 [connection]
 id=vsock0
@@ -278,14 +245,24 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 		return false, err
 	}
 
+	readyUnitFile, err := createReadyUnit(m.ReadyHVSock.Port)
+	if err != nil {
+		return false, err
+	}
+
 	builder.WithUnit(ignition.Unit{
 		Enabled:  ignition.BoolToPtr(true),
 		Name:     "ready.service",
-		Contents: ignition.StrToPtr(fmt.Sprintf(hyperVReadyUnit, m.ReadyHVSock.Port)),
+		Contents: ignition.StrToPtr(readyUnitFile),
 	})
 
+	netUnitFile, err := createNetworkUnit(m.NetworkHVSock.Port)
+	if err != nil {
+		return false, err
+	}
+
 	builder.WithUnit(ignition.Unit{
-		Contents: ignition.StrToPtr(fmt.Sprintf(hyperVVsockNetUnit, m.NetworkHVSock.Port)),
+		Contents: ignition.StrToPtr(netUnitFile),
 		Enabled:  ignition.BoolToPtr(true),
 		Name:     "vsock-network.service",
 	})
@@ -316,6 +293,23 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 	return err == nil, err
 }
 
+func createReadyUnit(readyPort uint64) (string, error) {
+	readyUnit := ignition.DefaultReadyUnitFile()
+	readyUnit.Add("Unit", "After", "systemd-user-sessions.service")
+	readyUnit.Add("Service", "ExecStart", fmt.Sprintf("/bin/sh -c '/usr/bin/echo Ready | socat - VSOCK-CONNECT:2:%d'", readyPort))
+	return readyUnit.ToString()
+}
+
+func createNetworkUnit(netPort uint64) (string, error) {
+	netUnit := parser.NewUnitFile()
+	netUnit.Add("Unit", "Description", "vsock_network")
+	netUnit.Add("Unit", "After", "NetworkManager.service")
+	netUnit.Add("Service", "ExecStart", fmt.Sprintf("/usr/libexec/podman/gvforwarder -preexisting -iface vsock0 -url vsock://2:%d/connect", netPort))
+	netUnit.Add("Service", "ExecStartPost", "/usr/bin/nmcli c up vsock0")
+	netUnit.Add("Install", "WantedBy", "multi-user.target")
+	return netUnit.ToString()
+}
+
 func (m *HyperVMachine) unregisterMachine() error {
 	vmm := hypervctl.NewVirtualMachineManager()
 	vm, err := vmm.GetMachine(m.Name)

pkg/machine/ignition/ignition.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 
 	"github.com/containers/podman/v4/pkg/machine/define"
+	"github.com/containers/podman/v4/pkg/systemd/parser"
 	"github.com/sirupsen/logrus"
 )
 
@@ -134,23 +135,24 @@ func (ign *DynamicIgnition) GenerateIgnitionConfig() error {
 		ignStorage.Links = append(ignStorage.Links, tzLink)
 	}
 
-	deMoby := `[Unit]
-Description=Remove moby-engine
-# Run once for the machine
-After=systemd-machine-id-commit.service
-Before=zincati.service
-ConditionPathExists=!/var/lib/%N.stamp
+	deMoby := parser.NewUnitFile()
+	deMoby.Add("Unit", "Description", "Remove moby-engine")
+	deMoby.Add("Unit", "After", "systemd-machine-id-commit.service")
+	deMoby.Add("Unit", "Before", "zincati.service")
+	deMoby.Add("Unit", "ConditionPathExists", "!/var/lib/%N.stamp")
 
-[Service]
-Type=oneshot
-RemainAfterExit=yes
-ExecStart=/usr/bin/rpm-ostree override remove moby-engine
-ExecStart=/usr/bin/rpm-ostree ex apply-live --allow-replacement
-ExecStartPost=/bin/touch /var/lib/%N.stamp
+	deMoby.Add("Service", "Type", "oneshot")
+	deMoby.Add("Service", "RemainAfterExit", "yes")
+	deMoby.Add("Service", "ExecStart", "/usr/bin/rpm-ostree override remove moby-engine")
+	deMoby.Add("Service", "ExecStart", "/usr/bin/rpm-ostree ex apply-live --allow-replacement")
+	deMoby.Add("Service", "ExecStartPost", "/bin/touch /var/lib/%N.stamp")
+
+	deMoby.Add("Install", "WantedBy", "default.target")
+	deMobyFile, err := deMoby.ToString()
+	if err != nil {
+		return err
+	}
 
-[Install]
-WantedBy=default.target
-`
 	// This service gets environment variables that are provided
 	// through qemu fw_cfg and then sets them into systemd/system.conf.d,
 	// profile.d and environment.d files
@@ -158,34 +160,39 @@ WantedBy=default.target
 	// Currently, it is used for propagating
 	// proxy settings e.g. HTTP_PROXY and others, on a start avoiding
 	// a need of re-creating/re-initiating a VM
-	envset := `[Unit]
-Description=Environment setter from QEMU FW_CFG
-[Service]
-Type=oneshot
-RemainAfterExit=yes
-Environment=FWCFGRAW=/sys/firmware/qemu_fw_cfg/by_name/opt/com.coreos/environment/raw
-Environment=SYSTEMD_CONF=/etc/systemd/system.conf.d/default-env.conf
-Environment=ENVD_CONF=/etc/environment.d/default-env.conf
-Environment=PROFILE_CONF=/etc/profile.d/default-env.sh
-ExecStart=/usr/bin/bash -c '/usr/bin/test -f ${FWCFGRAW} &&\
-	echo "[Manager]\n#Got from QEMU FW_CFG\nDefaultEnvironment=$(/usr/bin/base64 -d ${FWCFGRAW} | sed -e "s+|+ +g")\n" > ${SYSTEMD_CONF} ||\
-	echo "[Manager]\n#Got nothing from QEMU FW_CFG\n#DefaultEnvironment=\n" > ${SYSTEMD_CONF}'
-ExecStart=/usr/bin/bash -c '/usr/bin/test -f ${FWCFGRAW} && (\
-	echo "#Got from QEMU FW_CFG"> ${ENVD_CONF};\
-	IFS="|";\
-	for iprxy in $(/usr/bin/base64 -d ${FWCFGRAW}); do\
-		echo "$iprxy" >> ${ENVD_CONF}; done ) || \
-	echo "#Got nothing from QEMU FW_CFG"> ${ENVD_CONF}'
-ExecStart=/usr/bin/bash -c '/usr/bin/test -f ${FWCFGRAW} && (\
-	echo "#Got from QEMU FW_CFG"> ${PROFILE_CONF};\
-	IFS="|";\
-	for iprxy in $(/usr/bin/base64 -d ${FWCFGRAW}); do\
-		echo "export $iprxy" >> ${PROFILE_CONF}; done ) || \
-	echo "#Got nothing from QEMU FW_CFG"> ${PROFILE_CONF}'
-ExecStartPost=/usr/bin/systemctl daemon-reload
-[Install]
-WantedBy=sysinit.target
-`
+
+	envset := parser.NewUnitFile()
+	envset.Add("Unit", "Description", "Environment setter from QEMU FW_CFG")
+
+	envset.Add("Service", "Type", "oneshot")
+	envset.Add("Service", "RemainAfterExit", "yes")
+	envset.Add("Service", "Environment", "FWCFGRAW=/sys/firmware/qemu_fw_cfg/by_name/opt/com.coreos/environment/raw")
+	envset.Add("Service", "Environment", "SYSTEMD_CONF=/etc/systemd/system.conf.d/default-env.conf")
+	envset.Add("Service", "Environment", "ENVD_CONF=/etc/environment.d/default-env.conf")
+	envset.Add("Service", "Environment", "PROFILE_CONF=/etc/profile.d/default-env.sh")
+	envset.Add("Service", "ExecStart", `/usr/bin/bash -c '/usr/bin/test -f ${FWCFGRAW} &&\
+        echo "[Manager]\n#Got from QEMU FW_CFG\nDefaultEnvironment=$(/usr/bin/base64 -d ${FWCFGRAW} | sed -e "s+|+ +g")\n" > ${SYSTEMD_CONF} ||\
+        echo "[Manager]\n#Got nothing from QEMU FW_CFG\n#DefaultEnvironment=\n" > ${SYSTEMD_CONF}'`)
+	envset.Add("Service", "ExecStart", `/usr/bin/bash -c '/usr/bin/test -f ${FWCFGRAW} && (\
+        echo "#Got from QEMU FW_CFG"> ${ENVD_CONF};\
+        IFS="|";\
+        for iprxy in $(/usr/bin/base64 -d ${FWCFGRAW}); do\
+            echo "$iprxy" >> ${ENVD_CONF}; done ) || \
+        echo "#Got nothing from QEMU FW_CFG"> ${ENVD_CONF}'`)
+	envset.Add("Service", "ExecStart", `/usr/bin/bash -c '/usr/bin/test -f ${FWCFGRAW} && (\
+        echo "#Got from QEMU FW_CFG"> ${PROFILE_CONF};\
+        IFS="|";\
+        for iprxy in $(/usr/bin/base64 -d ${FWCFGRAW}); do\
+            echo "export $iprxy" >> ${PROFILE_CONF}; done ) || \
+        echo "#Got nothing from QEMU FW_CFG"> ${PROFILE_CONF}'`)
+	envset.Add("Service", "ExecStartPost", "/usr/bin/systemctl daemon-reload")
+
+	envset.Add("Install", "WantedBy", "sysinit.target")
+	envsetFile, err := envset.ToString()
+	if err != nil {
+		return err
+	}
+
 	ignSystemd := Systemd{
 		Units: []Unit{
 			{
@@ -205,7 +212,7 @@ WantedBy=sysinit.target
 			{
 				Enabled:  BoolToPtr(true),
 				Name:     "remove-moby.service",
-				Contents: &deMoby,
+				Contents: &deMobyFile,
 			},
 			{
 				// Disable auto-updating of fcos images
@@ -220,7 +227,7 @@ WantedBy=sysinit.target
 		qemuUnit := Unit{
 			Enabled:  BoolToPtr(true),
 			Name:     "envset-fwcfg.service",
-			Contents: &envset,
+			Contents: &envsetFile,
 		}
 		ignSystemd.Units = append(ignSystemd.Units, qemuUnit)
 	}
@@ -299,13 +306,16 @@ func getDirs(usrName string) []Directory {
 func getFiles(usrName string, uid int, rootful bool, vmtype define.VMType) []File {
 	files := make([]File, 0)
 
-	lingerExample := `[Unit]
-Description=A systemd user unit demo
-After=network-online.target
-Wants=network-online.target podman.socket
-[Service]
-ExecStart=/usr/bin/sleep infinity
-`
+	lingerExample := parser.NewUnitFile()
+	lingerExample.Add("Unit", "Description", "A systemd user unit demo")
+	lingerExample.Add("Unit", "After", "network-online.target")
+	lingerExample.Add("Unit", "Wants", "network-online.target podman.socket")
+	lingerExample.Add("Service", "ExecStart", "/usr/bin/sleep infinity")
+	lingerExampleFile, err := lingerExample.ToString()
+	if err != nil {
+		logrus.Warnf(err.Error())
+	}
+
 	containers := `[containers]
 netns="bridge"
 `
@@ -336,7 +346,7 @@ Delegate=memory pids cpu io
 		FileEmbedded1: FileEmbedded1{
 			Append: nil,
 			Contents: Resource{
-				Source: EncodeDataURLPtr(lingerExample),
+				Source: EncodeDataURLPtr(lingerExampleFile),
 			},
 			Mode: IntToPtr(0744),
 		},
@@ -731,3 +741,14 @@ func (i *IgnitionBuilder) BuildWithIgnitionFile(ignPath string) error {
 func (i *IgnitionBuilder) Build() error {
 	return i.dynamicIgnition.Write()
 }
+
+func DefaultReadyUnitFile() parser.UnitFile {
+	u := parser.NewUnitFile()
+	u.Add("Unit", "After", "remove-moby.service sshd.socket sshd.service")
+	u.Add("Unit", "OnFailure", "emergency.target")
+	u.Add("Unit", "OnFailureJobMode", "isolate")
+	u.Add("Service", "Type", "oneshot")
+	u.Add("Service", "RemainAfterExit", "yes")
+	u.Add("Install", "RequiredBy", "default.target")
+	return *u
+}

pkg/machine/qemu/machine.go

@@ -47,23 +47,6 @@ const (
 	dockerConnectTimeout = 5 * time.Second
 )
 
-// qemuReadyUnit is a unit file that sets up the virtual serial device
-// where when the VM is done configuring, it will send an ack
-// so a listening host tknows it can begin interacting with it
-const qemuReadyUnit = `[Unit]
-Requires=dev-virtio\\x2dports-%s.device
-After=remove-moby.service sshd.socket sshd.service
-After=systemd-user-sessions.service
-OnFailure=emergency.target
-OnFailureJobMode=isolate
-[Service]
-Type=oneshot
-RemainAfterExit=yes
-ExecStart=/bin/sh -c '/usr/bin/echo Ready >/dev/%s'
-[Install]
-RequiredBy=default.target
-`
-
 type MachineVM struct {
 	// ConfigPath is the path to the configuration file
 	ConfigPath define.VMFile
@@ -230,10 +213,14 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 		return false, err
 	}
 
+	readyUnitFile, err := createReadyUnitFile()
+	if err != nil {
+		return false, err
+	}
 	readyUnit := ignition.Unit{
 		Enabled:  ignition.BoolToPtr(true),
 		Name:     "ready.service",
-		Contents: ignition.StrToPtr(fmt.Sprintf(qemuReadyUnit, "vport1p1", "vport1p1")),
+		Contents: ignition.StrToPtr(readyUnitFile),
 	}
 	builder.WithUnit(readyUnit)
 
@@ -243,6 +230,14 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	return err == nil, err
 }
 
+func createReadyUnitFile() (string, error) {
+	readyUnit := ignition.DefaultReadyUnitFile()
+	readyUnit.Add("Unit", "Requires", "dev-virtio\\x2dports-vport1p1.device")
+	readyUnit.Add("Unit", "After", "systemd-user-sessions.service")
+	readyUnit.Add("Service", "ExecStart", "/bin/sh -c '/usr/bin/echo Ready >/dev/vport1p1'")
+	return readyUnit.ToString()
+}
+
 func (v *MachineVM) removeSSHKeys() error {
 	if err := os.Remove(fmt.Sprintf("%s.pub", v.IdentityPath)); err != nil {
 		logrus.Error(err)