OCPBUGS-72553: CPMS boot image config should not override standard MachineSet configuration

pkg/apihelpers/apihelpers.go

@@ -487,17 +487,70 @@ func CheckNodeDisruptionActionsForTargetActions(actions []opv1.NodeDisruptionPol
 	return currentActions.HasAny(targetActions...)
 }
 
-// Returns a MachineConfiguration object with the cluster opted into boot image updates.
-func GetManagedBootImagesWithUpdateEnabled() opv1.ManagedBootImages {
-	return opv1.ManagedBootImages{MachineManagers: []opv1.MachineManager{{Resource: opv1.MachineSets, APIGroup: opv1.MachineAPI, Selection: opv1.MachineManagerSelector{Mode: opv1.All}}}}
+// HasMAPIMachineSetManager checks if a MachineManager entry for a target resource exists.
+func HasMAPIMachineSetManager(machineManagers []opv1.MachineManager, resource opv1.MachineManagerMachineSetsResourceType) bool {
+	for _, manager := range machineManagers {
+		if manager.Resource == resource {
+			return true
+		}
+	}
+	return false
 }
 
-// Returns a MachineConfiguration object with the cluster opted out of boot image updates.
-func GetManagedBootImagesWithUpdateDisabled() opv1.ManagedBootImages {
-	return opv1.ManagedBootImages{MachineManagers: []opv1.MachineManager{{Resource: opv1.MachineSets, APIGroup: opv1.MachineAPI, Selection: opv1.MachineManagerSelector{Mode: opv1.None}}}}
+// GetMAPIMachineSetManager returns a target machine resource's machine manager. This should ideally
+// only be called if the machine manager exists in the list, returns None if not found.
+func GetMAPIMachineSetManager(machineManagers []opv1.MachineManager, resource opv1.MachineManagerMachineSetsResourceType) opv1.MachineManager {
+	for _, manager := range machineManagers {
+		if manager.Resource == resource {
+			return manager
+		}
+	}
+	// Return a None manager if no match is found
+	return opv1.MachineManager{Resource: resource, APIGroup: opv1.MachineAPI, Selection: opv1.MachineManagerSelector{Mode: opv1.None}}
+}
+
+// HasMAPIMachineSetManagerWithMode checks if a MachineManager entry for a target resource exists with the specified mode.
+func HasMAPIMachineSetManagerWithMode(machineManagers []opv1.MachineManager, resource opv1.MachineManagerMachineSetsResourceType, mode opv1.MachineManagerSelectorMode) bool {
+	for _, manager := range machineManagers {
+		if manager.Resource == resource {
+			return manager.Selection.Mode == mode
+		}
+	}
+	return false
 }
 
-// Returns a MachineConfiguration object with an empty configuration; to be used in testing a situation where admin has no opinion.
-func GetManagedBootImagesWithNoConfiguration() opv1.ManagedBootImages {
-	return opv1.ManagedBootImages{}
+// MergeMachineManager updates or adds a MachineManager entry for the target MachineSets resource
+// with the specified manager, preserving other MachineManager entries.
+func MergeMachineManager(status *opv1.MachineConfigurationStatus, manager opv1.MachineManager) {
+	var result opv1.ManagedBootImages
+	// Grab existing machine managers from status
+	existing := status.ManagedBootImagesStatus
+	// Handle nil case by initializing
+	if existing.MachineManagers != nil {
+		result = *existing.DeepCopy()
+	} else {
+		result = opv1.ManagedBootImages{MachineManagers: []opv1.MachineManager{}}
+	}
+
+	// Look for existing MachineSet MachineManager
+	found := false
+	for i := range result.MachineManagers {
+		if result.MachineManagers[i].Resource == manager.Resource &&
+			result.MachineManagers[i].APIGroup == manager.APIGroup {
+			result.MachineManagers[i].Selection = manager.Selection
+			found = true
+			break
+		}
+	}
+
+	// If not found, append a new MachineManager for regular MachineSets
+	if !found {
+		result.MachineManagers = append(result.MachineManagers, opv1.MachineManager{
+			Resource:  manager.Resource,
+			APIGroup:  manager.APIGroup,
+			Selection: manager.Selection,
+		})
+	}
+
+	status.ManagedBootImagesStatus = result
 }

pkg/controller/common/featuregates.go

@@ -148,30 +148,38 @@ func IsBootImageControllerRequired(ctx *ControllerContext) bool {
 		klog.Errorf("unable to get infra.Status.PlatformStatus for boot image controller startup: %v", err)
 		return false
 	}
-	return CheckBootImagePlatform(infra, ctx.FeatureGatesHandler)
+	supported, _, _ := CheckBootImagePlatform(infra, ctx.FeatureGatesHandler)
+	return supported
 }
 
-// Current valid feature gate and platform combinations:
-// GCP -> FeatureGateManagedBootImages
-// AWS -> FeatureGateManagedBootImagesAWS
-// vSphere -> FeatureGateManagedBootImagesvSphere
-// Azure -> FeatureGateManagedBootImagesAzure
-func CheckBootImagePlatform(infra *configv1.Infrastructure, fgHandler FeatureGatesHandler) bool {
+// CheckBootImagePlatform determines boot image update support for the cluster platform.
+//
+// Current platform support:
+// - GCP: FeatureGateManagedBootImages (opt-out), CPMS supported
+// - AWS: FeatureGateManagedBootImagesAWS (opt-out), CPMS supported
+// - vSphere: FeatureGateManagedBootImagesvSphere (opt-in), CPMS not supported
+// - Azure: FeatureGateManagedBootImagesAzure (opt-in), CPMS supported (except AzureStackCloud)
+//
+// Returns:
+// - supported: whether the platform supports boot image updates on machinesets
+// - cpmsSupported: whether the platform supports boot image updates on controlplanemachinesets
+// - enabledByDefault: true if opt-out (enabled by default), false if opt-in (disabled by default)
+func CheckBootImagePlatform(infra *configv1.Infrastructure, fgHandler FeatureGatesHandler) (supported, cpmsSupported, enabledByDefault bool) {
 	switch infra.Status.PlatformStatus.Type {
 	case configv1.AWSPlatformType:
-		return fgHandler.Enabled(features.FeatureGateManagedBootImagesAWS)
+		return fgHandler.Enabled(features.FeatureGateManagedBootImagesAWS), true, true
 	case configv1.GCPPlatformType:
-		return fgHandler.Enabled(features.FeatureGateManagedBootImages)
+		return fgHandler.Enabled(features.FeatureGateManagedBootImages), true, true
 	case configv1.VSpherePlatformType:
-		return fgHandler.Enabled(features.FeatureGateManagedBootImagesvSphere)
+		return fgHandler.Enabled(features.FeatureGateManagedBootImagesvSphere), false, false
 	case configv1.AzurePlatformType:
 		// Special variant check for Azure platforms; AzureStackCloud boot images are defined at install time
 		// See: https://github.com/openshift/installer/blob/bc941c822f06c10a95ddd080ae6345c25968baf4/pkg/asset/installconfig/azure/validation.go#L743-L749
 		if infra.Status.PlatformStatus.Azure != nil && infra.Status.PlatformStatus.Azure.CloudName == configv1.AzureStackCloud {
 			klog.Infof(" %s is not supported for boot image updates; disabling boot image controller", configv1.AzureStackCloud)
-			return false
+			return false, false, false
 		}
-		return fgHandler.Enabled(features.FeatureGateManagedBootImagesAzure)
+		return fgHandler.Enabled(features.FeatureGateManagedBootImagesAzure), true, false
 	}
-	return false
+	return false, false, false
 }

pkg/operator/sync.go

@@ -2310,29 +2310,11 @@ func (optr *Operator) syncMachineConfiguration(_ *renderConfig, _ *configv1.Clus
 	}
 
 	defaultOptInEvent := false
+	supportsBootImageUpdates, supportCPMSBootImageUpdates, isDefaultOnPlatform := ctrlcommon.CheckBootImagePlatform(infra, optr.fgHandler)
 	// Populate the default boot images configuration in the status, if the cluster is on a
 	// boot image updates supported platform
-	if ctrlcommon.CheckBootImagePlatform(infra, optr.fgHandler) {
-		// Mirror the spec if it is defined, i.e. admin has an opinion
-		if mcop.Spec.ManagedBootImages.MachineManagers != nil {
-			newMachineConfigurationStatus.ManagedBootImagesStatus = *mcop.Spec.ManagedBootImages.DeepCopy()
-		} else {
-			// Admin has no opinion, so use the MCO defined default, which is platform dependant
-			isDefaultOnPlatform, err := optr.isDefaultOnBootImageUpdatePlatform()
-			if err != nil {
-				klog.Errorf("failed to determine platform type, cluster will not be opted in for boot image updates by default")
-			}
-			if isDefaultOnPlatform {
-				// A default opt-in can happen in two ways:
-				// - Cluster is being installed on a release that opts the cluster in (ManagedBootImagesStatus is not defined)
-				// - Cluster is opted out and is being upgraded to a release that opts the cluster in (ManagedBootImagesStatus is currently disabled)
-				defaultOptInEvent = (mcop.Status.ManagedBootImagesStatus.MachineManagers == nil) ||
-					reflect.DeepEqual(mcop.Status.ManagedBootImagesStatus, apihelpers.GetManagedBootImagesWithUpdateDisabled())
-				newMachineConfigurationStatus.ManagedBootImagesStatus = apihelpers.GetManagedBootImagesWithUpdateEnabled()
-			} else {
-				newMachineConfigurationStatus.ManagedBootImagesStatus = apihelpers.GetManagedBootImagesWithUpdateDisabled()
-			}
-		}
+	if supportsBootImageUpdates {
+		defaultOptInEvent = optr.syncManagedBootImagesStatus(mcop, newMachineConfigurationStatus, isDefaultOnPlatform, supportCPMSBootImageUpdates)
 	}
 
 	newMachineConfigurationStatus.ObservedGeneration = mcop.GetGeneration()
@@ -2374,15 +2356,50 @@ func (optr *Operator) syncMachineConfiguration(_ *renderConfig, _ *configv1.Clus
 	return nil
 }
 
-// Determines if this cluster is on a platform that opts in for boot images by default
-func (optr *Operator) isDefaultOnBootImageUpdatePlatform() (bool, error) {
-	infra, err := optr.infraLister.Get("cluster")
-	if err != nil {
-		klog.Errorf("Could not get infra: %v", err)
-		return false, err
+// syncManagedBootImagesStatus populates the ManagedBootImagesStatus in the MachineConfiguration status
+// based on admin spec, platform defaults, and existing status. Returns true if a default opt-in event occurred.
+func (optr *Operator) syncManagedBootImagesStatus(mcop *opv1.MachineConfiguration, status *opv1.MachineConfigurationStatus, isDefaultOnPlatform, supportCPMSBootImageUpdates bool) bool {
+	defaultOptInEvent := false
+
+	// An empty list is a legacy method of opting out of boot image updates completely
+	if mcop.Spec.ManagedBootImages.MachineManagers != nil && len(mcop.Spec.ManagedBootImages.MachineManagers) == 0 {
+		status.ManagedBootImagesStatus = *mcop.Spec.ManagedBootImages.DeepCopy()
+		return false
 	}
-	defaultOnPlatforms := sets.New(configv1.GCPPlatformType, configv1.AWSPlatformType)
-	return defaultOnPlatforms.Has(infra.Status.PlatformStatus.Type), nil
+
+	// Populate/Reflect opinion for MAPI MachineSets
+	//
+	// Default to None unless overridden via admin opinion or opt-in mechanism
+	mapiMachineSetManager := opv1.MachineManager{Resource: opv1.MachineSets, APIGroup: opv1.MachineAPI, Selection: opv1.MachineManagerSelector{Mode: opv1.None}}
+	if mcop.Spec.ManagedBootImages.MachineManagers != nil && apihelpers.HasMAPIMachineSetManager(mcop.Spec.ManagedBootImages.MachineManagers, opv1.MachineSets) {
+		// An admin-defined opinion for MAPI MachineSets exist, so reflect that to the status
+		mapiMachineSetManager = apihelpers.GetMAPIMachineSetManager(mcop.Spec.ManagedBootImages.MachineManagers, opv1.MachineSets)
+	} else if isDefaultOnPlatform {
+		// This implies that the MachineManagers list is nil or it does and no opinion for MAPI MachineSet exists
+		// A default opt-in can happen in two ways:
+		// - Cluster is being installed on a release that opts the cluster in (ManagedBootImagesStatus is not defined)
+		// - Cluster is opted out and is being upgraded to a release that opts the cluster in (MachineSet manager has mode=None)
+		defaultOptInEvent = (mcop.Status.ManagedBootImagesStatus.MachineManagers == nil) ||
+			apihelpers.HasMAPIMachineSetManagerWithMode(mcop.Status.ManagedBootImagesStatus.MachineManagers, opv1.MachineSets, opv1.None)
+		// Merge the MachineSet manager with mode=All, preserving other MachineManagers
+		mapiMachineSetManager.Selection.Mode = opv1.All
+	}
+	// Merge the determined MAPI MachineSet Manager mode to status
+	apihelpers.MergeMachineManager(status, mapiMachineSetManager)
+
+	// Populate/Reflect opinion for ControlPlaneMachineSets, if CPMS feature gate is enabled
+	if optr.fgHandler.Enabled(features.FeatureGateManagedBootImagesCPMS) && supportCPMSBootImageUpdates {
+		// Default to None unless overridden via admin opinion.
+		cpmsManager := opv1.MachineManager{Resource: opv1.ControlPlaneMachineSets, APIGroup: opv1.MachineAPI, Selection: opv1.MachineManagerSelector{Mode: opv1.None}}
+		if mcop.Spec.ManagedBootImages.MachineManagers != nil && apihelpers.HasMAPIMachineSetManager(mcop.Spec.ManagedBootImages.MachineManagers, opv1.ControlPlaneMachineSets) {
+			// An admin-defined opinion for ControlMachineSets exist, so reflect that to the status
+			cpmsManager = apihelpers.GetMAPIMachineSetManager(mcop.Spec.ManagedBootImages.MachineManagers, opv1.ControlPlaneMachineSets)
+		}
+		// Merge the determined ControlPlaneMachineSet Manager
+		apihelpers.MergeMachineManager(status, cpmsManager)
+	}
+
+	return defaultOptInEvent
 }
 
 // syncPreBuiltImageMachineConfigs creates/updates/deletes component MachineConfigs for pools with pre-built images.