From fc0638e90527fa7a5f54560df75f1f868e925b16 Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Tue, 3 Nov 2020 12:30:28 +0100 Subject: [PATCH 1/2] pkg/daemon: don't delete a file if its replaced with a dropin Some systemd service settings may be defined in .Storage.Files and in .Systemd.Units.Dropins. Dropins is preferable, so users may want to migrate to the way. However if a file representing a dropin was defined in .Storage.Files and converted into .Systemd.Units.Dropin in new version of MC, it would be placed by Ignition's dropin implementation and then garbage-collected by MCD's deleteStaleData. This change ensures we're checking Systemd.Units.Dropins before attempting to remove this file. This is crucial for OKD 4.5 -> 4.6 upgrades - kubelet MCO config was defined as a file in 4.5 and converted into a dropin in 4.6 --- pkg/daemon/update.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 3a35a9cc33..2906c13a19 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -840,6 +840,25 @@ func restorePath(path string) error { return nil } +// parse path to find out if its a systemd dropin +// Returns is dropin (true/false), service name, dropin name +func isSystemdDropin(path string) (bool, string, string) { + if !strings.HasPrefix(path, "/etc/systemd/system") { + return false, "", "" + } + pathSegments := strings.Split(path, "/") + if len(pathSegments) != 5 { + return false, "", "" + } + dropinName := pathSegments[len(pathSegments)-1] + segmentWithService := pathSegments[len(pathSegments)-2] + sectionSegments := strings.SplitN(segmentWithService, ".", 1) + if sectionSegments[len(sectionSegments)-1] != "service.d" { + return false, "", "" + } + return true, string(segmentWithService[0]), dropinName +} + // deleteStaleData performs a diff of the new and the old Ignition config. It then deletes // all the files, units that are present in the old config but not in the new one. // this function will error out if it fails to delete a file (with the exception @@ -892,6 +911,29 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *igntypes.Config) e continue } + // Check Systemd.Units.Dropins - don't remove the file if configuration has been converted into a dropin + fileReplacedWithDropin := false + if ok, service, dropin := isSystemdDropin(f.Path); ok { + for _, u := range newIgnConfig.Systemd.Units { + if u.Name == service { + for _, j := range u.Dropins { + if j.Name == dropin { + fileReplacedWithDropin = true + break + } + } + if fileReplacedWithDropin { + break + } + } + } + } + + if fileReplacedWithDropin { + glog.V(2).Infof("Not removing file %q: replaced with systemd dropin", f.Path) + continue + } + if err := os.Remove(origFileName(f.Path)); err != nil { return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) } From 59ac825513f187ea486a1408ffb9ff5b60c75a7e Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Tue, 3 Nov 2020 16:21:32 +0100 Subject: [PATCH 2/2] DEBUG --- pkg/daemon/update.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 2906c13a19..e5a1b8e18a 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -843,20 +843,27 @@ func restorePath(path string) error { // parse path to find out if its a systemd dropin // Returns is dropin (true/false), service name, dropin name func isSystemdDropin(path string) (bool, string, string) { + glog.V(2).Infof("DEBUG isSystemdDropin path=%v", path) if !strings.HasPrefix(path, "/etc/systemd/system") { return false, "", "" } pathSegments := strings.Split(path, "/") + glog.V(2).Infof("DEBUG isSystemdDropin pathSegments=%v", pathSegments) if len(pathSegments) != 5 { return false, "", "" } dropinName := pathSegments[len(pathSegments)-1] - segmentWithService := pathSegments[len(pathSegments)-2] - sectionSegments := strings.SplitN(segmentWithService, ".", 1) - if sectionSegments[len(sectionSegments)-1] != "service.d" { + glog.V(2).Infof("DEBUG isSystemdDropin dropinName=%v", dropinName) + servicePart := pathSegments[len(pathSegments)-2] + glog.V(2).Infof("DEBUG isSystemdDropin servicePart=%v", servicePart) + allServiceSegments := strings.Split(servicePart, ".") + glog.V(2).Infof("DEBUG isSystemdDropin allServiceSegments=%v", allServiceSegments) + if allServiceSegments[len(allServiceSegments)-1] != "d" { return false, "", "" } - return true, string(segmentWithService[0]), dropinName + serviceName := strings.Join(allServiceSegments[:len(allServiceSegments)-1], ".") + glog.V(2).Infof("DEBUG isSystemdDropin serviceName=%v", serviceName) + return true, serviceName, dropinName } // deleteStaleData performs a diff of the new and the old Ignition config. It then deletes @@ -914,10 +921,15 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *igntypes.Config) e // Check Systemd.Units.Dropins - don't remove the file if configuration has been converted into a dropin fileReplacedWithDropin := false if ok, service, dropin := isSystemdDropin(f.Path); ok { + glog.V(2).Infof("DEBUG isSystemdDropin ok=%v service=%v dropin=%v", ok, service, dropin) for _, u := range newIgnConfig.Systemd.Units { + glog.V(2).Infof("DEBUG isSystemdDropin service=%v", u.Name) if u.Name == service { + glog.V(2).Infof("DEBUG isSystemdDropin found matching service") for _, j := range u.Dropins { + glog.V(2).Infof("DEBUG isSystemdDropin dropin=%v", j.Name) if j.Name == dropin { + glog.V(2).Infof("DEBUG isSystemdDropin found matching dropin") fileReplacedWithDropin = true break }