From 979f59f041d9c9353d31a699de9acb4cdf2cce3d Mon Sep 17 00:00:00 2001 From: Zack Zlotnik Date: Wed, 27 Mar 2024 16:58:59 -0400 Subject: [PATCH] make the implementation more MCD-like --- pkg/daemon/daemon.go | 4 ++ pkg/daemon/update.go | 61 +++++++++++++------ .../machine-config-daemon-revert.service.yaml | 1 + test/e2e-techpreview/onclusterbuild_test.go | 2 + 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index aecd68c084..a80a14af0c 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -1292,6 +1292,10 @@ func (dn *Daemon) RunFirstbootCompleteMachineconfig() error { return fmt.Errorf("failed to rename encapsulated MachineConfig after processing on firstboot: %w", err) } + if err := dn.toggleRevertSystemdUnit(&mc, false); err != nil { + return err + } + dn.skipReboot = false return dn.reboot(fmt.Sprintf("Completing firstboot provisioning to %s", mc.GetName())) } diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 9834ce84e7..55f5deb8ad 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -41,6 +41,7 @@ import ( pivottypes "github.com/openshift/machine-config-operator/pkg/daemon/pivot/types" pivotutils "github.com/openshift/machine-config-operator/pkg/daemon/pivot/utils" "github.com/openshift/machine-config-operator/pkg/upgrademonitor" + "github.com/openshift/machine-config-operator/test/helpers" ) const ( @@ -71,6 +72,9 @@ const ( // ImageRegistryDrainOverrideConfigmap is the name of the Configmap a user can apply to force all // image registry changes to not drain ImageRegistryDrainOverrideConfigmap = "image-registry-override-drain" + + // name of the systemd unit that re-bootstraps a node after reverting from layered to non-layered + layeringRevertSystemdUnitName = "machine-config-daemon-revert.service" ) func getNodeRef(node *corev1.Node) *corev1.ObjectReference { @@ -1003,25 +1007,10 @@ func (dn *Daemon) finalizeRevertToNonLayering(newConfig *mcfgv1.MachineConfig) e klog.Infof("Wrote MachineConfig %q to %q", newConfig.Name, constants.MachineConfigEncapsulatedPath) - clonedRevertUnitName := "machine-config-daemon-revert-layered.service" - if _, err := exec.Command("cp", filepath.Join(pathSystemd, "machine-config-daemon-revert.service"), filepath.Join(pathSystemd, clonedRevertUnitName)).CombinedOutput(); err != nil { - return fmt.Errorf("could not clone systemd unit: %w", err) - } - - klog.Infof("Cloned systemd unit to %q", filepath.Join(pathSystemd, clonedRevertUnitName)) - - if _, err := exec.Command("systemctl", "daemon-reload").CombinedOutput(); err != nil { - return fmt.Errorf("could not reload systemd daemon: %w", err) - } - - klog.Infof("Reloaded systemd daemon") - - if _, err := exec.Command("systemctl", "enable", clonedRevertUnitName).CombinedOutput(); err != nil { - return fmt.Errorf("could not enable systemd unit %q: %w", clonedRevertUnitName, err) + if err := dn.toggleRevertSystemdUnit(newConfig, true); err != nil { + return err } - klog.Infof("Enabled systemd unit %q", clonedRevertUnitName) - // Clear the current image field odc := &onDiskConfig{ currentImage: "", @@ -2910,3 +2899,41 @@ func (dn *Daemon) hasImageRegistryDrainOverrideConfigMap() (bool, error) { return false, fmt.Errorf("Error fetching image registry drain override configmap: %w", err) } + +func (dn *Daemon) toggleRevertSystemdUnit(mc *mcfgv1.MachineConfig, enabled bool) error { + clonedUnit, err := getRevertSystemdUnitFromMachineConfig(mc) + if err != nil { + return err + } + + clonedUnit.Enabled = helpers.BoolToPtr(enabled) + clonedUnit.Mask = nil + + if err := dn.writeUnits([]ign3types.Unit{*clonedUnit}); err != nil { + return err + } + + if !enabled { + return os.RemoveAll(filepath.Join(pathSystemd, clonedUnit.Name)) + } + + return nil +} + +func getRevertSystemdUnitFromMachineConfig(mc *mcfgv1.MachineConfig) (*ign3types.Unit, error) { + ignConfig, err := ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw) + if err != nil { + return nil, err + } + + for _, unit := range ignConfig.Systemd.Units { + unit := unit + if unit.Name == layeringRevertSystemdUnitName { + unit.Name = "machine-config-daemon-revert-layered.service" + unit.Mask = nil + return &unit, nil + } + } + + return nil, fmt.Errorf("could not find %s in MachineConfig %s", layeringRevertSystemdUnitName, mc.Name) +} diff --git a/templates/common/_base/units/machine-config-daemon-revert.service.yaml b/templates/common/_base/units/machine-config-daemon-revert.service.yaml index fa5cbc3873..124715ef1b 100644 --- a/templates/common/_base/units/machine-config-daemon-revert.service.yaml +++ b/templates/common/_base/units/machine-config-daemon-revert.service.yaml @@ -1,4 +1,5 @@ name: machine-config-daemon-revert.service +mask: true enabled: false contents: | [Unit] diff --git a/test/e2e-techpreview/onclusterbuild_test.go b/test/e2e-techpreview/onclusterbuild_test.go index 4998a40f39..f279bff71e 100644 --- a/test/e2e-techpreview/onclusterbuild_test.go +++ b/test/e2e-techpreview/onclusterbuild_test.go @@ -145,6 +145,8 @@ func assertNodeRevertsToNonLayered(t *testing.T, cs *framework.ClientSet, node c assert.Contains(t, helpers.GetRPMOstreeStatus(t, cs, node), workerMC.Spec.OSImageURL, "node %q did not rollback to OS image %q", node.Name, workerMC.Spec.OSImageURL) t.Logf("Node %s has reverted to OS image %q", node.Name, workerMC.Spec.OSImageURL) + + helpers.AssertFileNotOnNode(t, cs, node, "/etc/systemd/system/machine-config-daemon-revert-layered.service") } // This test extracts the /etc/yum.repos.d and /etc/pki/rpm-gpg content from a