From ea59abee8cd93212bd2db90792ff1b6cfa56227f Mon Sep 17 00:00:00 2001 From: Stephen Benjamin Date: Fri, 1 Aug 2025 06:13:45 -0400 Subject: [PATCH] Revert "Merge pull request #5148 from RishabhSaini/unitDiff" This reverts commit 5510c60e185925a8b344c9b091e7a13e60d32554, reversing changes made to 3a09b61ed8cee97153f865970f93c39e0ec7d85f. --- pkg/controller/common/helpers.go | 7 ++----- pkg/daemon/on_disk_validation.go | 27 +-------------------------- pkg/daemon/update.go | 16 +++++++--------- 3 files changed, 10 insertions(+), 40 deletions(-) diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index 5f30124d3c..b0999cfec2 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -926,7 +926,7 @@ func CalculateConfigFileDiffs(oldIgnConfig, newIgnConfig *ign3types.Config) []st // that are different between them // //nolint:dupl -func CalculateConfigUnitDiffs(oldIgnConfig, newIgnConfig *ign3types.Config) ([]string, []ign3types.Unit) { +func CalculateConfigUnitDiffs(oldIgnConfig, newIgnConfig *ign3types.Config) []string { // Go through the units and see what is new or different oldUnitSet := make(map[string]ign3types.Unit) for _, u := range oldIgnConfig.Systemd.Units { @@ -946,19 +946,16 @@ func CalculateConfigUnitDiffs(oldIgnConfig, newIgnConfig *ign3types.Config) ([]s } } - addedOrChangedUnits := []ign3types.Unit{} // Now check if any units were added/changed for name, newUnit := range newUnitSet { oldUnit, ok := oldUnitSet[name] if !ok { diffUnitSet = append(diffUnitSet, name) - addedOrChangedUnits = append(addedOrChangedUnits, newUnit) } else if !reflect.DeepEqual(oldUnit, newUnit) { diffUnitSet = append(diffUnitSet, name) - addedOrChangedUnits = append(addedOrChangedUnits, newUnit) } } - return diffUnitSet, addedOrChangedUnits + return diffUnitSet } // NewIgnFile returns a simple ignition3 file from just path and file contents. diff --git a/pkg/daemon/on_disk_validation.go b/pkg/daemon/on_disk_validation.go index a30b62efc3..b6cabff62d 100644 --- a/pkg/daemon/on_disk_validation.go +++ b/pkg/daemon/on_disk_validation.go @@ -4,9 +4,7 @@ import ( "bytes" "fmt" "os" - "os/exec" "path/filepath" - "strings" ign2types "github.com/coreos/ignition/config/v2_2/types" ign3types "github.com/coreos/ignition/v2/config/v3_5/types" @@ -103,12 +101,7 @@ func checkV3Unit(unit ign3types.Unit, systemdPath string) error { return nil } - err := checkFileContentsAndMode(path, []byte(*unit.Contents), defaultFilePermissions) - if err != nil { - return err - } - - return checkUnitEnabled(unit.Name, unit.Enabled) + return checkFileContentsAndMode(path, []byte(*unit.Contents), defaultFilePermissions) } // checkV3Units validates the contents of all the units in the @@ -238,24 +231,6 @@ func checkV2Files(files []ign2types.File) error { return nil } -// checkUnitEnabled checks whether a systemd unit is enabled as expected. -func checkUnitEnabled(name string, expectedEnabled *bool) error { - if expectedEnabled == nil { - return nil - } - cmd := exec.Command("systemctl", "is-enabled", name) - outBytes, err := cmd.CombinedOutput() - out := strings.TrimSpace(string(outBytes)) - - // units types considered not enabled (linked, linked-runtime, masked, masked-runtime, disabled, bad) - isEnabled := err == nil - if isEnabled != *expectedEnabled { - return fmt.Errorf("unit %q expected enabled=%t, but systemd reports %q", name, *expectedEnabled, out) - } - - return nil -} - // checkFileContentsAndMode reads the file from the filepath and compares its // contents and mode with the expectedContent and mode parameters. It logs an // error in case of an error or mismatch and returns the status of the diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 0ad347cd59..0edc6a0adf 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -1005,7 +1005,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi logSystem("Starting update from %s to %s: %+v", oldConfigName, newConfigName, diff) diffFileSet := ctrlcommon.CalculateConfigFileDiffs(&oldIgnConfig, &newIgnConfig) - diffUnitSet, addedOrChangedUnits := ctrlcommon.CalculateConfigUnitDiffs(&oldIgnConfig, &newIgnConfig) + diffUnitSet := ctrlcommon.CalculateConfigUnitDiffs(&oldIgnConfig, &newIgnConfig) var nodeDisruptionActions []opv1.NodeDisruptionPolicyStatusAction var actions []string @@ -1117,13 +1117,13 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi } // update files on disk that need updating - if err := dn.updateFiles(oldIgnConfig, newIgnConfig, addedOrChangedUnits, skipCertificateWrite); err != nil { + if err := dn.updateFiles(oldIgnConfig, newIgnConfig, skipCertificateWrite); err != nil { return err } defer func() { if retErr != nil { - if err := dn.updateFiles(newIgnConfig, oldIgnConfig, addedOrChangedUnits, skipCertificateWrite); err != nil { + if err := dn.updateFiles(newIgnConfig, oldIgnConfig, skipCertificateWrite); err != nil { errs := kubeErrs.NewAggregate([]error{err, retErr}) retErr = fmt.Errorf("error rolling back files writes: %w", errs) return @@ -1258,17 +1258,15 @@ func (dn *Daemon) updateHypershift(oldConfig, newConfig *mcfgv1.MachineConfig, d return fmt.Errorf("parsing new Ignition config failed: %w", err) } - _, addedOrChangedUnits := ctrlcommon.CalculateConfigUnitDiffs(&oldIgnConfig, &newIgnConfig) - // update files on disk that need updating // We should't skip the certificate write in HyperShift since it does not run the extra daemon process - if err := dn.updateFiles(oldIgnConfig, newIgnConfig, addedOrChangedUnits, false); err != nil { + if err := dn.updateFiles(oldIgnConfig, newIgnConfig, false); err != nil { return err } defer func() { if retErr != nil { - if err := dn.updateFiles(newIgnConfig, oldIgnConfig, addedOrChangedUnits, false); err != nil { + if err := dn.updateFiles(newIgnConfig, oldIgnConfig, false); err != nil { errs := kubeErrs.NewAggregate([]error{err, retErr}) retErr = fmt.Errorf("error rolling back files writes: %w", errs) return @@ -1773,12 +1771,12 @@ func (dn *CoreOSDaemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) // whatever has been written is picked up by the appropriate daemons, if // required. in particular, a daemon-reload and restart for any unit files // touched. -func (dn *Daemon) updateFiles(oldIgnConfig, newIgnConfig ign3types.Config, addedOrChangedUnits []ign3types.Unit, skipCertificateWrite bool) error { +func (dn *Daemon) updateFiles(oldIgnConfig, newIgnConfig ign3types.Config, skipCertificateWrite bool) error { klog.Info("Updating files") if err := dn.writeFiles(newIgnConfig.Storage.Files, skipCertificateWrite); err != nil { return err } - if err := dn.writeUnits(addedOrChangedUnits); err != nil { + if err := dn.writeUnits(newIgnConfig.Systemd.Units); err != nil { return err } return dn.deleteStaleData(oldIgnConfig, newIgnConfig)