controller, daemon, and upgrade monitor: update MachineConfigNode update flow to include image mode status reporting fields

Author: isabella-janssen

pkg/controller/common/layered_node_state.go

@@ -321,3 +321,19 @@ func (l *LayeredNodeState) CheckNodeCandidacyForUpdate(layered bool, pool *mcfgv
 
 	return true
 }
+
+// GetDesiredAnnotationsFromMachineOSConfig gets the desired config version and desired image values from the associated MOSC and MOSB
+func (l *LayeredNodeState) GetDesiredAnnotationsFromMachineConfigPool(mcp *mcfgv1.MachineConfigPool) (desriedConfig string) {
+	return mcp.Spec.Configuration.Name
+}
+
+// GetDesiredAnnotationsFromMachineOSConfig gets the desired config version and desired image values from the associated MOSC and MOSB
+func (l *LayeredNodeState) GetDesiredAnnotationsFromMachineOSConfig(mosc *mcfgv1.MachineOSConfig, mosb *mcfgv1.MachineOSBuild) (desriedConfig, desiredImage string) {
+	desiredImage = ""
+	moscs := NewMachineOSConfigState(mosc)
+	if moscs.HasOSImage() {
+		desiredImage = moscs.GetOSImage()
+	}
+
+	return mosb.Spec.MachineConfig.Name, desiredImage
+}

pkg/controller/node/node_controller.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	helpers "github.com/openshift/machine-config-operator/pkg/helpers"
+	"github.com/openshift/machine-config-operator/pkg/upgrademonitor"
 
 	configv1 "github.com/openshift/api/config/v1"
 	features "github.com/openshift/api/features"
@@ -1288,11 +1289,19 @@ func (ctrl *Controller) updateCandidateNode(mosc *mcfgv1.MachineOSConfig, mosb *
 		}
 
 		lns := ctrlcommon.NewLayeredNodeState(oldNode)
+		desiredConfig := ""
+		desiredImage := ""
 		if !layered {
 			lns.SetDesiredStateFromPool(pool)
+			// If pool is not layered, the desired image annotation is removed (see the `delete`
+			// call in `SetDesiredStateFromPool`), so only the desired config version must be set.
+			desiredConfig = lns.GetDesiredAnnotationsFromMachineConfigPool(pool)
 		} else {
 			lns.SetDesiredStateFromMachineOSConfig(mosc, mosb)
+			desiredConfig, desiredImage = lns.GetDesiredAnnotationsFromMachineOSConfig(mosc, mosb)
 		}
+		// TODO: maybe add a try/catch for the error handling if the MCN does not yest exist
+		upgrademonitor.UpdateMachineConfigNodeSpecDesiredAnnotations(ctrl.fgHandler, ctrl.client, nodeName, desiredConfig, desiredImage)
 
 		// Set the desired state to match the pool.
 

pkg/daemon/update.go

@@ -2567,11 +2567,37 @@ func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error {
 		}
 	}
 
+	// For image mode status reporting we need the node's MCP association to populate it's MCN
+	pool := ""
+	if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+		pool, err = helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node)
+		if err != nil {
+			return err
+		}
+	}
+
 	if isOsImagePresent {
 		if err := dn.NodeUpdaterClient.RebaseLayeredFromContainerStorage(newURL); err != nil {
 			return fmt.Errorf("failed to update OS from local storage: %s: %w", newURL, err)
 		}
 	} else {
+		// Report ImagePulledFromRegistry condition as unknown (pulling)
+		if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+			err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
+				&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Pulling OS image %s from registry", newURL)},
+				nil,
+				metav1.ConditionUnknown,
+				metav1.ConditionFalse,
+				dn.node,
+				dn.mcfgClient,
+				dn.fgHandler,
+				pool,
+			)
+			if err != nil {
+				klog.Errorf("Error setting ImagePulledFromRegistry condition to unknown: %v", err)
+			}
+		}
+
 		// Workaround for OCPBUGS-43406, retry the remote rebase with backoff,
 		// such that if we happen to update while the CoreDNS pod is being restarted,
 		// the next retry should succeed if no other issues are present.
@@ -2588,8 +2614,41 @@ func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error {
 			}
 			return true, nil
 		}); err != nil {
+			// Report ImagePulledFromRegistry condition as false (failed)
+			if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+				err = upgrademonitor.GenerateAndApplyMachineConfigNodes(
+					&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Failed to pull OS image %s from registry: %v", newURL, err)},
+					nil,
+					metav1.ConditionFalse,
+					metav1.ConditionFalse,
+					dn.node,
+					dn.mcfgClient,
+					dn.fgHandler,
+					pool,
+				)
+				if err != nil {
+					klog.Errorf("Error setting ImagePulledFromRegistry condition to false: %v", err)
+				}
+			}
 			return fmt.Errorf("Failed to update OS to %s after retries: %w", newURL, err)
 		}
+
+		// Report ImagePulledFromRegistry condition as true (success)
+		if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+			err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
+				&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Successfully pulled OS image %s from registry", newURL)},
+				nil,
+				metav1.ConditionTrue,
+				metav1.ConditionFalse,
+				dn.node,
+				dn.mcfgClient,
+				dn.fgHandler,
+				pool,
+			)
+			if err != nil {
+				klog.Errorf("Error setting ImagePulledFromRegistry condition to true: %v", err)
+			}
+		}
 	}
 
 	return nil
@@ -2738,13 +2797,74 @@ func (dn *CoreOSDaemon) applyLayeredOSChanges(mcDiff machineConfigDiff, oldConfi
 	var osExtensionsContentDir string
 	var err error
 
+	// For image mode status reporting we need the node's MCP association to populate it's MCN
+	pool := ""
+	if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+		pool, err = helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node)
+		if err != nil {
+			return err
+		}
+	}
+
 	if newConfig.Spec.BaseOSExtensionsContainerImage != "" && (mcDiff.osUpdate || mcDiff.extensions || mcDiff.kernelType) && !mcDiff.oclEnabled {
 		// TODO(jkyros): the original intent was that we use the extensions container as a service, but that currently results
 		// in a lot of complexity due to boostrap and firstboot where the service isn't easily available, so for now we are going
 		// to extract them to disk like we did previously.
+
+		// Report ImagePulledFromRegistry condition as unknown (pulling)
+		if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+			err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
+				&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Pulling image %s from registry", newConfig.Spec.BaseOSExtensionsContainerImage)},
+				nil,
+				metav1.ConditionUnknown,
+				metav1.ConditionFalse,
+				dn.node,
+				dn.mcfgClient,
+				dn.fgHandler,
+				pool,
+			)
+			if err != nil {
+				klog.Errorf("Error setting ImagePulledFromRegistry condition to unknown: %v", err)
+			}
+		}
+
 		if osExtensionsContentDir, err = ExtractExtensionsImage(newConfig.Spec.BaseOSExtensionsContainerImage); err != nil {
+			// Report ImagePulledFromRegistry condition as false (failed)
+			if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+				err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
+					&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Failed to pull image %s from registry: %v", newConfig.Spec.BaseOSExtensionsContainerImage, err)},
+					nil,
+					metav1.ConditionFalse,
+					metav1.ConditionFalse,
+					dn.node,
+					dn.mcfgClient,
+					dn.fgHandler,
+					pool,
+				)
+				if err != nil {
+					klog.Errorf("Error setting ImagePulledFromRegistry condition to false: %v", err)
+				}
+			}
 			return err
 		}
+
+		// Report ImagePulledFromRegistry condition as true (success)
+		if dn.fgHandler != nil && dn.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+			err := upgrademonitor.GenerateAndApplyMachineConfigNodes(
+				&upgrademonitor.Condition{State: mcfgv1.MachineConfigNodeImagePulledFromRegistry, Reason: string(mcfgv1.MachineConfigNodeImagePulledFromRegistry), Message: fmt.Sprintf("Successfully pulled image %s from registry", newConfig.Spec.BaseOSExtensionsContainerImage)},
+				nil,
+				metav1.ConditionTrue,
+				metav1.ConditionFalse,
+				dn.node,
+				dn.mcfgClient,
+				dn.fgHandler,
+				pool,
+			)
+			if err != nil {
+				klog.Errorf("Error setting ImagePulledFromRegistry condition to true: %v", err)
+			}
+		}
+
 		// Delete extracted OS image once we are done.
 		defer os.RemoveAll(osExtensionsContentDir)
 

pkg/upgrademonitor/upgrade_monitor.go

@@ -135,6 +135,7 @@ func generateAndApplyMachineConfigNodes(
 
 	// we use this array to see if the MCN has all of its conditions set
 	// if not we set a sane default
+	// TODO: decide if the image reg stuff should be added here too
 	allConditionTypes := []mcfgv1.StateProgress{
 		mcfgv1.MachineConfigNodeUpdatePrepared,
 		mcfgv1.MachineConfigNodeUpdateExecuted,
@@ -217,8 +218,20 @@ func generateAndApplyMachineConfigNodes(
 
 			case condition.Status != metav1.ConditionFalse && reset:
 				condition.Status = metav1.ConditionFalse
-				condition.Message = fmt.Sprintf("Action during update to %s: %s", newMCNode.Spec.ConfigVersion.Desired, condition.Message)
 				condition.LastTransitionTime = metav1.Now()
+
+				// TODO: test this in the enable and disable case
+				// Set the update to annotation to the desired rendered MC version by default for the condition message
+				updateToMessage := fmt.Sprintf("Action during update to %s: %s", newMCNode.Spec.ConfigVersion.Desired, condition.Message)
+				// Handle OCL update cases differently
+				if newMCNode.Spec.ConfigImage.DesiredImage != newMCNode.Status.ConfigImage.CurrentImage {
+					if newMCNode.Spec.ConfigImage.DesiredImage != "" { // Handle case when desired image exists
+						updateToMessage = fmt.Sprintf("Action during update to %s: %s", newMCNode.Spec.ConfigImage.DesiredImage, condition.Message)
+					} else { // When the desired image is empty, it means OCL is being disabled; provide a more useful message in this case
+						updateToMessage = fmt.Sprintf("Action during update to disable image mode: %s", condition.Message)
+					}
+				}
+				condition.Message = updateToMessage
 			}
 			condition.DeepCopyInto(&newMCNode.Status.Conditions[i])
 		}
@@ -250,6 +263,27 @@ func generateAndApplyMachineConfigNodes(
 		newMCNode.Status.ConfigVersion.Current = node.Annotations[daemonconsts.CurrentMachineConfigAnnotationKey]
 	}
 
+	// Set current and desired image values in MCN.Status.ConfigImage
+	// This is only done when the ImageModeStatusReporting feature gate is enabled
+	if fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+		// TODO: test if this flow accurately updates the current image annotaiton on OCL disable
+		newMCNStatusConfigImage := mcfgv1.MachineConfigNodeStatusConfigImage{}
+		currentImageAnnotation := node.Annotations[daemonconsts.CurrentImageAnnotationKey]
+		desiredImageAnnotation := node.Annotations[daemonconsts.DesiredImageAnnotationKey]
+
+		// Set current image if annotation exists
+		if currentImageAnnotation != "" {
+			newMCNStatusConfigImage.CurrentImage = mcfgv1.ImageDigestFormat(currentImageAnnotation)
+		}
+
+		// Set desired image if annotation exists
+		if desiredImageAnnotation != "" {
+			newMCNStatusConfigImage.DesiredImage = mcfgv1.ImageDigestFormat(desiredImageAnnotation)
+		}
+
+		newMCNode.Status.ConfigImage = newMCNStatusConfigImage
+	}
+
 	// if we do not need a new MCN, generate the apply configurations for this object
 	if !needNewMCNode {
 		statusconfigVersionApplyConfig := machineconfigurationv1.MachineConfigNodeStatusMachineConfigVersion().WithDesired(newMCNode.Status.ConfigVersion.Desired)
@@ -262,6 +296,23 @@ func generateAndApplyMachineConfigNodes(
 			WithObservedGeneration(newMCNode.Generation + 1).
 			WithConfigVersion(statusconfigVersionApplyConfig)
 
+		// Add ConfigImage to apply configuration if feature gate is enabled and image annotations exist
+		if fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) && (newMCNode.Status.ConfigImage.CurrentImage != "" || newMCNode.Status.ConfigImage.DesiredImage != "") {
+			configImageApplyConfig := machineconfigurationv1.MachineConfigNodeStatusConfigImage()
+
+			// Set current image if it exists
+			if newMCNode.Status.ConfigImage.CurrentImage != "" {
+				configImageApplyConfig = configImageApplyConfig.WithCurrentImage(newMCNode.Status.ConfigImage.CurrentImage)
+			}
+
+			// Set desired image if it exists
+			if newMCNode.Status.ConfigImage.DesiredImage != "" {
+				configImageApplyConfig = configImageApplyConfig.WithDesiredImage(newMCNode.Status.ConfigImage.DesiredImage)
+			}
+
+			statusApplyConfig = statusApplyConfig.WithConfigImage(configImageApplyConfig)
+		}
+
 		if fgHandler.Enabled(features.FeatureGatePinnedImages) {
 			if imageSetApplyConfig == nil {
 				for _, imageSet := range newMCNode.Status.PinnedImageSets {
@@ -335,6 +386,51 @@ func isSingletonCondition(singletonConditionTypes []mcfgv1.StateProgress, condit
 	return false
 }
 
+// UpdateMachineConfigNodeSpecDesiredAnnotations sets the desired config version and image
+// annotation values in the `Spec` of an existing MachineConfigNode resource
+func UpdateMachineConfigNodeSpecDesiredAnnotations(fgHandler ctrlcommon.FeatureGatesHandler, mcfgClient mcfgclientset.Interface, nodeName string, desiredConfig string, desiredImage string) error {
+	if fgHandler == nil {
+		return nil
+	}
+
+	// Check that the MachineConfigNode and ImageModeStatusReporting feature gates are enabled
+	if !fgHandler.Enabled(features.FeatureGateMachineConfigNodes) || !fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+		klog.Infof("MachineConfigNode or ImageModeStatusReporting FeatureGate is not enabled.  Please enable the TechPreviewNoUpgrade FeatureSet to use ImageModeStatusReporting.")
+		return nil
+	}
+
+	// Get the existing MCN
+	mcn, mcnErr := mcfgClient.MachineconfigurationV1().MachineConfigNodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
+	// TODO: check if we need to make an MCN at this point instead then or if we get to this point that we should have an MCN
+	if mcnErr != nil {
+		// no existing MCN found since no resource found
+		if apierrors.IsNotFound(mcnErr) {
+			return fmt.Errorf("MCN for %s node does not exits. Skipping MCN spec update.", nodeName)
+		}
+		return mcnErr
+	}
+
+	// Set the desired config annotation
+	mcn.Spec.ConfigVersion.Desired = NotYetSet
+	if desiredConfig != "" {
+		mcn.Spec.ConfigVersion.Desired = desiredConfig
+	}
+
+	// Set the desired image annotation
+	mcn.Spec.ConfigImage = mcfgv1.MachineConfigNodeSpecConfigImage{}
+	if desiredImage != "" {
+		mcn.Spec.ConfigImage = mcfgv1.MachineConfigNodeSpecConfigImage{
+			DesiredImage: mcfgv1.ImageDigestFormat(desiredImage),
+		}
+	}
+
+	// Update the MCN resource
+	if _, err := mcfgClient.MachineconfigurationV1().MachineConfigNodes().Update(context.TODO(), mcn, metav1.UpdateOptions{FieldManager: "machine-config-operator"}); err != nil {
+		return fmt.Errorf("failed to update the %s mcn spec with the new desired config and image value: %w", nodeName, err)
+	}
+	return nil
+}
+
 // GenerateAndApplyMachineConfigNodeSpec generates and applies a new MCN spec based off the node state
 func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHandler, pool string, node *corev1.Node, mcfgClient mcfgclientset.Interface) error {
 	if fgHandler == nil || node == nil {
@@ -345,10 +441,11 @@ func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHand
 		klog.Infof("MCN Featuregate is not enabled. Please enable the TechPreviewNoUpgrade featureset to use MachineConfigNodes")
 		return nil
 	}
+
 	// get the existing MCN, or if it DNE create one below
 	mcNode, needNewMCNode := createOrGetMachineConfigNode(mcfgClient, node)
 	newMCNode := mcNode.DeepCopy()
-	// set the spec config version
+	// Set the MCN owner references
 	newMCNode.ObjectMeta.OwnerReferences = []metav1.OwnerReference{
 		{
 			APIVersion: "v1",
@@ -358,14 +455,25 @@ func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHand
 		},
 	}
 
+	// Set the desired config version in the MCN
 	newMCNode.Spec.ConfigVersion = mcfgv1.MachineConfigNodeSpecMachineConfigVersion{
 		Desired: node.Annotations[daemonconsts.DesiredMachineConfigAnnotationKey],
 	}
-	// Set desired config to NotYetSet if the annotation is empty to satisfy API validation
+	// If the desired config does not yet exist for the node, the desired config should be set to NotYetSet
 	if newMCNode.Spec.ConfigVersion.Desired == "" {
 		newMCNode.Spec.ConfigVersion.Desired = NotYetSet
 	}
 
+	// Check that the ImageModeStatusReporting feature gate is enabled
+	if fgHandler.Enabled(features.FeatureGateImageModeStatusReporting) {
+		// Set the desired image in the MCN if it exists
+		newMCNode.Spec.ConfigImage = mcfgv1.MachineConfigNodeSpecConfigImage{}
+		if node.Annotations[daemonconsts.DesiredImageAnnotationKey] != "" {
+			newMCNode.Spec.ConfigImage.DesiredImage = mcfgv1.ImageDigestFormat(node.Annotations[daemonconsts.DesiredImageAnnotationKey])
+		}
+	}
+
+	// Set the MCN pool and node names
 	newMCNode.Spec.Pool = mcfgv1.MCOObjectReference{
 		Name: pool,
 	}