From b5f33d28295f9e2d2733ffc6390ac3754bbb0724 Mon Sep 17 00:00:00 2001
From: Sohan Kunkerkar <sohank2602@gmail.com>
Date: Thu, 21 Apr 2022 11:52:40 -0400
Subject: [PATCH] internal/exec: delete enablement symlinks when disabling unit

FCOS ships some pre-created enablement symlinks, and the systemd
does nothing even after recording in the preset that they should be
removed. In order to fix this behavior, we're not only deleting
enablement symlinks for a given unit but also recording in the preset
directive to disable that unit completely. 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     |  2 ++
 internal/exec/util/unit.go    | 19 +++++++++++
 tests/positive/files/units.go | 61 +++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)

diff --git a/internal/distro/distro.go b/internal/distro/distro.go
index 4096b04af1..7c2b9f9a87 100644
--- a/internal/distro/distro.go
+++ b/internal/distro/distro.go
@@ -44,6 +44,7 @@ var (
 	userdelCmd  = "userdel"
 	setfilesCmd = "setfiles"
 	wipefsCmd   = "wipefs"
+	systemtlCmd = "systemctl"
 
 	// Filesystem tools
 	btrfsMkfsCmd = "mkfs.btrfs"
@@ -96,6 +97,7 @@ func UseraddCmd() string  { return useraddCmd }
 func UserdelCmd() string  { return userdelCmd }
 func SetfilesCmd() string { return setfilesCmd }
 func WipefsCmd() string   { return wipefsCmd }
+func SystemCtl() string   { return systemtlCmd }
 
 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..a39b93dcf3 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.SystemCtl(), 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,
+	}
+}