From 8585c470e8b9246b87686945faf26c356102443f Mon Sep 17 00:00:00 2001 From: Sohan Kunkerkar Date: Thu, 21 Apr 2022 11:52:40 -0400 Subject: [PATCH] internal/exec: delete enablement symlinks when disabling unit For services where FCOS ships a symlink in /etc, if the user tries to disable the service via Ignition, systemd ignores the disablement directive in the preset. This behavior can be fixed by deleting enablement symlinks for a given unit before recording in the preset directive. This is a short-term solution until the upstream systemd PR (https://github.com/systemd/systemd/pull/15205) gets accepted. Fixes https://github.com/coreos/fedora-coreos-tracker/issues/392 --- internal/distro/distro.go | 50 ++++++++++++++-------------- internal/exec/util/unit.go | 19 +++++++++++ tests/positive/files/units.go | 61 +++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 24 deletions(-) diff --git a/internal/distro/distro.go b/internal/distro/distro.go index 4096b04af1..61ca87aed9 100644 --- a/internal/distro/distro.go +++ b/internal/distro/distro.go @@ -32,18 +32,19 @@ var ( systemConfigDir = "/usr/lib/ignition" // Helper programs - groupaddCmd = "groupadd" - groupdelCmd = "groupdel" - mdadmCmd = "mdadm" - mountCmd = "mount" - sgdiskCmd = "sgdisk" - modprobeCmd = "modprobe" - udevadmCmd = "udevadm" - usermodCmd = "usermod" - useraddCmd = "useradd" - userdelCmd = "userdel" - setfilesCmd = "setfiles" - wipefsCmd = "wipefs" + groupaddCmd = "groupadd" + groupdelCmd = "groupdel" + mdadmCmd = "mdadm" + mountCmd = "mount" + sgdiskCmd = "sgdisk" + modprobeCmd = "modprobe" + udevadmCmd = "udevadm" + usermodCmd = "usermod" + useraddCmd = "useradd" + userdelCmd = "userdel" + setfilesCmd = "setfiles" + wipefsCmd = "wipefs" + systemctlCmd = "systemctl" // Filesystem tools btrfsMkfsCmd = "mkfs.btrfs" @@ -84,18 +85,19 @@ func KernelCmdlinePath() string { return kernelCmdlinePath } func BootIDPath() string { return bootIDPath } func SystemConfigDir() string { return fromEnv("SYSTEM_CONFIG_DIR", systemConfigDir) } -func GroupaddCmd() string { return groupaddCmd } -func GroupdelCmd() string { return groupdelCmd } -func MdadmCmd() string { return mdadmCmd } -func MountCmd() string { return mountCmd } -func SgdiskCmd() string { return sgdiskCmd } -func ModprobeCmd() string { return modprobeCmd } -func UdevadmCmd() string { return udevadmCmd } -func UsermodCmd() string { return usermodCmd } -func UseraddCmd() string { return useraddCmd } -func UserdelCmd() string { return userdelCmd } -func SetfilesCmd() string { return setfilesCmd } -func WipefsCmd() string { return wipefsCmd } +func GroupaddCmd() string { return groupaddCmd } +func GroupdelCmd() string { return groupdelCmd } +func MdadmCmd() string { return mdadmCmd } +func MountCmd() string { return mountCmd } +func SgdiskCmd() string { return sgdiskCmd } +func ModprobeCmd() string { return modprobeCmd } +func UdevadmCmd() string { return udevadmCmd } +func UsermodCmd() string { return usermodCmd } +func UseraddCmd() string { return useraddCmd } +func UserdelCmd() string { return userdelCmd } +func SetfilesCmd() string { return setfilesCmd } +func WipefsCmd() string { return wipefsCmd } +func SystemctlCmd() string { return systemctlCmd } func BtrfsMkfsCmd() string { return btrfsMkfsCmd } func Ext4MkfsCmd() string { return ext4MkfsCmd } diff --git a/internal/exec/util/unit.go b/internal/exec/util/unit.go index 01321cfa86..d42c51ad35 100644 --- a/internal/exec/util/unit.go +++ b/internal/exec/util/unit.go @@ -18,10 +18,12 @@ import ( "fmt" "net/url" "os" + "os/exec" "path/filepath" "syscall" "github.com/coreos/ignition/v2/config/v3_4_experimental/types" + "github.com/coreos/ignition/v2/internal/distro" "github.com/vincent-petithory/dataurl" ) @@ -151,6 +153,23 @@ func (ut Util) EnableUnit(enabledUnit string) error { } func (ut Util) DisableUnit(disabledUnit string) error { + // We need to delete any enablement symlinks for a unit before sending it to a + // preset directive. This will help to disable that unit completely. + // For more information: https://github.com/coreos/fedora-coreos-tracker/issues/392 + // This is a short-term solution until the upstream systemd PR + // (https://github.com/systemd/systemd/pull/15205) gets accepted. + if err := ut.Logger.LogOp( + func() error { + args := []string{"--root", ut.DestDir, "disable", disabledUnit} + if output, err := exec.Command(distro.SystemctlCmd(), args...).CombinedOutput(); err != nil { + return fmt.Errorf("cannot remove symlink(s) for %s: %v: %q", disabledUnit, err, string(output)) + } + return nil + }, + "removing enablement symlink(s) for %q", disabledUnit, + ); err != nil { + return err + } return ut.appendLineToPreset(fmt.Sprintf("disable %s", disabledUnit)) } diff --git a/tests/positive/files/units.go b/tests/positive/files/units.go index e9d4a6f461..b89744794e 100644 --- a/tests/positive/files/units.go +++ b/tests/positive/files/units.go @@ -24,6 +24,7 @@ func init() { register.Register(register.PositiveTest, CreateEmptyDropinsUnit()) register.Register(register.PositiveTest, TestUnmaskUnit()) register.Register(register.PositiveTest, TestMaskUnit()) + register.Register(register.PositiveTest, RemoveEnablementSymLinksforUnit()) } func CreateInstantiatedService() types.Test { @@ -259,3 +260,63 @@ func TestMaskUnit() types.Test { ConfigMinVersion: configMinVersion, } } + +// RemoveEnablementSymLinksforUnit checks if Ignition +// removes the enablement symlink for a given systemd +// unit marked as disabled. Also, verifies that the code +// doesn't error out when a non-existent unit marked as +// disabled. +func RemoveEnablementSymLinksforUnit() types.Test { + name := "unit.remove.symlinks" + in := types.GetBaseDisk() + out := types.GetBaseDisk() + config := `{ + "ignition": { "version": "$version" }, + "systemd": { + "units": [ + { + "enabled": false, + "name": "foo.service" + }, + { + "enabled": false, + "name": "enoent.service" + } + ] + } + }` + in[0].Partitions.AddLinks("ROOT", []types.Link{ + { + Node: types.Node{ + Directory: "/etc/systemd/system/multi-user.target.wants", + Name: "foo.service", + }, + Target: "/usr/lib/systemd/system/foo.service", + Hard: false, + }, + }) + in[0].Partitions.AddFiles("ROOT", []types.File{ + { + Node: types.Node{ + Name: "foo.service", + Directory: "usr/lib/systemd/system", + }, + Contents: "[Unit]\nDescription=f\n[Service]\nType=oneshot\nExecStart=/usr/bin/true\n[Install]\nWantedBy=multi-user.target\n", + }, + }) + out[0].Partitions.AddRemovedNodes("ROOT", []types.Node{ + { + Directory: "/etc/systemd/system/multi-user.target.wants", + Name: "foo.service", + }, + }) + configMinVersion := "3.0.0" + + return types.Test{ + Name: name, + In: in, + Out: out, + Config: config, + ConfigMinVersion: configMinVersion, + } +}