Merge pull request #5612 from isabella-janssen/ocpbugs-74692

OCPBUGS-74692: Write all units when forcefile exists to recover from systemd unit config drift

Author: openshift-merge-bot[bot]

pkg/daemon/update.go

@@ -1026,6 +1026,9 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi
 
 	var nodeDisruptionActions []opv1.NodeDisruptionPolicyStatusAction
 	var actions []string
+	// Check for forcefile before calculatePostConfigChange* functions delete it.
+	// This is needed for updateFiles to know whether to write all units (OCPBUGS-74692).
+	forceFilePresent := forceFileExists()
 	// Node Disruption Policies cannot be used during firstboot as API is not accessible.
 	if !firstBoot {
 		nodeDisruptionActions, err = dn.calculatePostConfigChangeNodeDisruptionAction(diff, diffFileSet, allChangedUnitNames)
@@ -1134,13 +1137,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, addedOrChangedUnits, skipCertificateWrite, forceFilePresent); 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, addedOrChangedUnits, skipCertificateWrite, false); err != nil {
 				errs := kubeErrs.NewAggregate([]error{err, retErr})
 				retErr = fmt.Errorf("error rolling back files writes: %w", errs)
 				return
@@ -1278,15 +1281,18 @@ func (dn *Daemon) updateHypershift(oldConfig, newConfig *mcfgv1.MachineConfig, d
 	unitDiff := ctrlcommon.GetChangedConfigUnitsByType(&oldIgnConfig, &newIgnConfig)
 	addedOrChangedUnits := slices.Concat(unitDiff.Added, unitDiff.Updated)
 
+	// Check for forcefile to support config drift recovery (OCPBUGS-74692)
+	forceFilePresent := forceFileExists()
+
 	// 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, addedOrChangedUnits, false, forceFilePresent); 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, addedOrChangedUnits, false, false); err != nil {
 				errs := kubeErrs.NewAggregate([]error{err, retErr})
 				retErr = fmt.Errorf("error rolling back files writes: %w", errs)
 				return
@@ -1811,12 +1817,26 @@ func (dn *CoreOSDaemon) getKernelPackagesForTargetRelease() (releaseKernelPackag
 // 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, addedOrChangedUnits []ign3types.Unit, skipCertificateWrite, forceFilePresent 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 {
+
+	// With OCPBUGS-58023, we updated this flow to only write units that were either added or
+	// updated. As can be seen in OCPBUGS-74692, this impacted the traditional method to recover
+	// from config drifts with systemd units. It makes the `touch /run/machine-config-daemon-force`
+	// command useless since the new flow does not rewrite all files, only the ones that have been
+	// added or changed with the latest MC. To keep the fix for OCPBUGS-58023 and allow continue
+	// supporting the traditional config drift recovery for systemd units, all units should be
+	// written when a forcefile exists.
+	unitsToWrite := addedOrChangedUnits
+	if forceFilePresent {
+		klog.Info("Forcefile exists, writing all units")
+		unitsToWrite = newIgnConfig.Systemd.Units
+	}
+
+	if err := dn.writeUnits(unitsToWrite); err != nil {
 		return err
 	}
 	return dn.deleteStaleData(oldIgnConfig, newIgnConfig)