Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase

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
 }
 

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")
 

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")
 	}

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.