Merge pull request #1352 from sohankunkerkar/remove-symlink

internal/exec: delete enablement symlinks when disabling unit
coreos · May 6, 2022 · 12a4b10 · 12a4b10
2 parents 97c5c99 + 397aa6a
commit 12a4b10
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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,
+	}
+}