From d4a2e2a203c54e8b6fd017819abc075000033497 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Tue, 28 May 2019 20:10:26 +0200 Subject: [PATCH] pkg/daemon: do not delete host files when deleting an MC Deploying an MC changing a file and later deleting it causes the file to be deleted (it could have been already on disk i.e. shipped by an RPM). Following Colin's suggestion, this patch adds a backup mechanism which, when deleting an MC, causes the old file to be restored. Added an e2e test as well. Signed-off-by: Antonio Murdaca --- pkg/daemon/update.go | 36 ++++++++++++++++++++++-- test/e2e/mcd_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index fa956adf0d..6e98e1a9f7 100644 --- a/pkg/daemon/update.go +++ b/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,6 +687,9 @@ 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 } @@ -687,6 +697,26 @@ func (dn *Daemon) writeFiles(files []igntypes.File) error { 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") diff --git a/test/e2e/mcd_test.go b/test/e2e/mcd_test.go index f3aaf74dc0..9f99fb158d 100644 --- a/test/e2e/mcd_test.go +++ b/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) + } + } +}