diff --git a/cmd/machine-config-daemon/pivot.go b/cmd/machine-config-daemon/pivot.go index 0994dd819d..4b40395a25 100644 --- a/cmd/machine-config-daemon/pivot.go +++ b/cmd/machine-config-daemon/pivot.go @@ -22,12 +22,12 @@ import ( // flag storage var keep bool -var reboot bool var fromEtcPullSpec bool const ( - etcPivotFile = "/etc/pivot/image-pullspec" - runPivotRebootFile = "/run/pivot/reboot-needed" + // etcPivotFile is used for 4.1 bootimages and is how the MCD + // currently communicated with this service. + etcPivotFile = "/etc/pivot/image-pullspec" // File containing kernel arg changes for tuning kernelTuningFile = "/etc/pivot/kernel-args" cmdLineFile = "/proc/cmdline" @@ -51,7 +51,6 @@ var pivotCmd = &cobra.Command{ func init() { rootCmd.AddCommand(pivotCmd) pivotCmd.PersistentFlags().BoolVarP(&keep, "keep", "k", false, "Do not remove container image") - pivotCmd.PersistentFlags().BoolVarP(&reboot, "reboot", "r", false, "Reboot if changed") pivotCmd.PersistentFlags().BoolVarP(&fromEtcPullSpec, "from-etc-pullspec", "P", false, "Parse /etc/pivot/image-pullspec") pflag.CommandLine.AddGoFlagSet(flag.CommandLine) } @@ -232,29 +231,8 @@ func run(_ *cobra.Command, args []string) error { if !changed { glog.Info("No changes; already at target oscontainer, no kernel args provided") - return nil } - if reboot { - glog.Infof("Rebooting as requested by cmdline flag") - } else { - // Otherwise see if it's specified by the file - _, err = os.Stat(runPivotRebootFile) - if err != nil && !os.IsNotExist(err) { - return errors.Wrapf(err, "Checking %s", runPivotRebootFile) - } - if err == nil { - glog.Infof("Rebooting due to %s", runPivotRebootFile) - reboot = true - } - } - if reboot { - // Reboot the machine if asked to do so - err := exec.Command("systemctl", "reboot").Run() - if err != nil { - return errors.Wrapf(err, "rebooting") - } - } return nil } diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 8c1d72ce3c..7ca5b22a29 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -971,7 +971,11 @@ func (dn *Daemon) checkStateOnFirstRun() error { if !osMatch { glog.Infof("Bootstrap pivot required to: %s", targetOSImageURL) // This only returns on error - return dn.updateOSAndReboot(state.currentConfig) + if err := dn.updateOS(state.currentConfig); err != nil { + return err + } + + return dn.finalizeAndReboot(state.currentConfig) } glog.Info("No bootstrap pivot required; unlinking bootstrap node annotations") diff --git a/pkg/daemon/rpm-ostree.go b/pkg/daemon/rpm-ostree.go index 7337322e53..508fcd927d 100644 --- a/pkg/daemon/rpm-ostree.go +++ b/pkg/daemon/rpm-ostree.go @@ -317,7 +317,8 @@ func (r *RpmOstreeClient) RunPivot(osImageURL string) error { defer close(journalStopCh) go followPivotJournalLogs(journalStopCh) - // This is written by code injected by the MCS, but we always want the MCD to be in control of reboots + // This is written by code injected by the MCS for compatibility with 4.1 bootimages, + // remove it to clean things up. if err := os.Remove("/run/pivot/reboot-needed"); err != nil && !os.IsNotExist(err) { return errors.Wrap(err, "deleting pivot reboot-needed file") } diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index ed4be2f3c0..f2951e5d44 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -109,13 +109,42 @@ func getNodeRef(node *corev1.Node) *corev1.ObjectReference { } } -// updateOSAndReboot is the last step in an update(), and it can also -// be called as a special case for the "bootstrap pivot". -func (dn *Daemon) updateOSAndReboot(newConfig *mcfgv1.MachineConfig) (retErr error) { +// Remove pending deployment on OSTree based system +func removePendingDeployment() error { + _, err := runGetOut("rpm-ostree", "cleanup", "-p") + return err +} + +func (dn *Daemon) applyOSChanges(oldConfig, newConfig *mcfgv1.MachineConfig) (retErr error) { if err := dn.updateOS(newConfig); err != nil { return err } - return dn.finalizeAndReboot(newConfig) + + defer func() { + // Operations performed by rpm-ostree on the booted system are available + // as staged deployment. It gets applied only when we reboot the system. + // In case of an error during any rpm-ostree transaction, removing pending deployment + // should be sufficient to discard any applied changes. + if retErr != nil { + glog.Infof("Rolling back applied changes to OS") + if err := removePendingDeployment(); err != nil { + retErr = errors.Wrapf(retErr, "error removing staged deployment: %v", err) + return + } + } + }() + + // kargs + if err := dn.updateKernelArguments(oldConfig, newConfig); err != nil { + return err + } + + // Switch to real time kernel + if err := dn.switchKernel(oldConfig, newConfig); err != nil { + return err + } + + return nil } func (dn *Daemon) finalizeAndReboot(newConfig *mcfgv1.MachineConfig) (retErr error) { @@ -353,34 +382,20 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig) (retErr err } }() - // kargs - if err := dn.updateKernelArguments(oldConfig, newConfig); err != nil { - return err - } - defer func() { - if retErr != nil { - if err := dn.updateKernelArguments(newConfig, oldConfig); err != nil { - retErr = errors.Wrapf(retErr, "error rolling back kernel arguments %v", err) - return - } - } - }() - - // Switch to real time kernel - if err := dn.switchKernel(oldConfig, newConfig); err != nil { + if err := dn.applyOSChanges(oldConfig, newConfig); err != nil { return err } defer func() { if retErr != nil { - if err := dn.switchKernel(newConfig, oldConfig); err != nil { - retErr = errors.Wrapf(retErr, "error rolling back Real time Kernel %v", err) + if err := dn.applyOSChanges(newConfig, oldConfig); err != nil { + retErr = errors.Wrapf(retErr, "error rolling back changes to OS %v", err) return } } }() - return dn.updateOSAndReboot(newConfig) + return dn.finalizeAndReboot(newConfig) } // MachineConfigDiff represents an ad-hoc difference between two MachineConfig objects.