pkg/daemon: do not delete host files when deleting an MC

pkg/daemon/update.go

@@ -485,6 +485,13 @@ func (dn *Daemon) deleteStaleData(oldConfig, newConfig *mcfgv1.MachineConfig) er
 
 	for _, f := range oldConfig.Spec.Config.Storage.Files {
 		if _, ok := newFileSet[f.Path]; !ok {
+			if _, err := os.Stat(origFileName(f.Path)); err == nil {
+				if err := os.Rename(origFileName(f.Path), f.Path); err != nil {
+					return err
+				}
+				glog.V(2).Infof("Restored file %q", f.Path)
+				continue
+			}
 			glog.V(2).Infof("Deleting stale config file: %s", f.Path)
 			if err := os.Remove(f.Path); err != nil {
 				newErr := fmt.Errorf("unable to delete %s: %s", f.Path, err)
@@ -680,13 +687,36 @@ func (dn *Daemon) writeFiles(files []igntypes.File) error {
 				return fmt.Errorf("failed to retrieve file ownership for file %q: %v", file.Path, err)
 			}
 		}
+		if err := createOrigFile(file.Path); err != nil {
+			return err
+		}
 		if err := writeFileAtomically(file.Path, contents.Data, defaultDirectoryPermissions, mode, uid, gid); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
+func origFileName(fpath string) string {
+	return fpath + ".mcdorig"
+}
+
+func createOrigFile(fpath string) error {
+	if _, err := os.Stat(fpath); err != nil {
+		// the file isn't there, no need to back it up
+		// we could check ENOENT only maybe?
+		return nil
+	}
+	if _, err := os.Stat(origFileName(fpath)); err == nil {
+		// the orig file is already there and we avoid creating a new one to preserve the real default
+		return nil
+	}
+	if out, err := exec.Command("cp", "-a", "--reflink=auto", fpath, origFileName(fpath)).CombinedOutput(); err != nil {
+		return errors.Wrapf(err, "creating orig file for %q: %s", fpath, string(out))
+	}
+	return nil
+}
+
 // This is essentially ResolveNodeUidAndGid() from Ignition; XXX should dedupe
 func getFileOwnership(file igntypes.File) (int, int, error) {
 	uid, gid := 0, 0 // default to root
@@ -781,12 +811,12 @@ func (dn *Daemon) updateOS(config *mcfgv1.MachineConfig) error {
 
 // RHEL 7.6 logger (util-linux) doesn't have the --journald flag
 func (dn *Daemon) isLoggingToJournalSupported() bool {
+	if dn.OperatingSystem == machineConfigDaemonOSRHEL {
+		return true
+	}
 	loggerOutput, err := exec.Command("logger", "--help").CombinedOutput()
 	if err != nil {
 		dn.logSystem("error running logger --help: %v", err)
-		if dn.OperatingSystem == machineConfigDaemonOSRHCOS {
-			return true
-		}
 		return false
 	}
 	return strings.Contains(string(loggerOutput), "--journald")

test/e2e/mcd_test.go

@@ -482,3 +482,68 @@ func TestReconcileAfterBadMC(t *testing.T) {
 		t.Errorf("machine config didn't roll back on any worker: %v", err)
 	}
 }
+
+func TestDontDeleteRPMFiles(t *testing.T) {
+	cs := framework.NewClientSet("")
+	bumpPoolMaxUnavailableTo(t, cs, 3)
+
+	mcHostFile := createMCToAddFile("modify-host-file", "/etc/motd", "mco-test", "root")
+
+	// grab the initial machineconfig used by the worker pool
+	// this MC is gonna be the one which is going to be reapplied once the previous MC is deleted
+	mcp, err := cs.MachineConfigPools().Get("worker", metav1.GetOptions{})
+	if err != nil {
+		t.Error(err)
+	}
+	workerOldMc := mcp.Status.Configuration.Name
+
+	// create the dummy MC now
+	_, err = cs.MachineConfigs().Create(mcHostFile)
+	if err != nil {
+		t.Errorf("failed to create machine config %v", err)
+	}
+
+	renderedConfig, err := waitForRenderedConfig(t, cs, "worker", mcHostFile.Name)
+	if err != nil {
+		t.Errorf("%v", err)
+	}
+
+	// wait for the mcp to go back to previous config
+	if err := waitForPoolComplete(t, cs, "worker", renderedConfig); err != nil {
+		t.Fatal(err)
+	}
+
+	// now delete the bad MC and watch the nodes reconciling as expected
+	if err := cs.MachineConfigs().Delete(mcHostFile.Name, &metav1.DeleteOptions{}); err != nil {
+		t.Error(err)
+	}
+
+	// wait for the mcp to go back to previous config
+	if err := waitForPoolComplete(t, cs, "worker", workerOldMc); err != nil {
+		t.Fatal(err)
+	}
+
+	nodes, err := getNodesByRole(cs, "worker")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, node := range nodes {
+		assert.Equal(t, node.Annotations[constants.CurrentMachineConfigAnnotationKey], workerOldMc)
+		assert.Equal(t, node.Annotations[constants.MachineConfigDaemonStateAnnotationKey], constants.MachineConfigDaemonStateDone)
+		mcd, err := mcdForNode(cs, &node)
+		if err != nil {
+			t.Fatal(err)
+		}
+		mcdName := mcd.ObjectMeta.Name
+
+		found, err := exec.Command("oc", "rsh", "-n", "openshift-machine-config-operator", mcdName,
+			"cat", "/rootfs/etc/motd").CombinedOutput()
+		if err != nil {
+			t.Fatalf("unable to read test file on daemon: %s got: %s got err: %v", mcdName, found, err)
+		}
+		if strings.Contains(string(found), "mco-test") {
+			t.Fatalf("updated file doesn't contain expected data, got %s", found)
+		}
+	}
+}