MCO-2117: Allow default OSImageStream overrides

manifests/machineconfigcontroller/osimagestream-deletion-guard-validatingadmissionpolicy.yaml

@@ -0,0 +1,19 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicy
+metadata:
+  name: "osimagestream-prevent-deletion"
+spec:
+  failurePolicy: Fail
+  matchConstraints:
+    matchPolicy: Equivalent
+    namespaceSelector: {}
+    objectSelector: {}
+    resourceRules:
+    - apiGroups:   ["machineconfiguration.openshift.io"]
+      apiVersions: ["v1alpha1"]
+      operations:  ["DELETE"]
+      resources:   ["osimagestreams"]
+      scope: "*"
+  validations:
+    - expression: "false"
+      message: "Deletion of OSImageStream resources is not allowed"

manifests/machineconfigcontroller/osimagestream-deletion-guard-validatingadmissionpolicybinding.yaml

@@ -0,0 +1,7 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicyBinding
+metadata:
+  name: "osimagestream-prevent-deletion-binding"
+spec:
+  policyName: "osimagestream-prevent-deletion"
+  validationActions: [Deny]

pkg/controller/bootstrap/bootstrap.go

@@ -115,6 +115,7 @@ func (b *Bootstrap) Run(destDir string) error {
 		imageStream          *imagev1.ImageStream
 		iri                  *mcfgv1alpha1.InternalReleaseImage
 		iriTLSCert           *corev1.Secret
+		osImageStream        *mcfgv1alpha1.OSImageStream
 	)
 	for _, info := range infos {
 		if info.IsDir() {
@@ -199,6 +200,9 @@ func (b *Bootstrap) Run(destDir string) error {
 				if obj.GetName() == ctrlcommon.InternalReleaseImageTLSSecretName {
 					iriTLSCert = obj
 				}
+			case *mcfgv1alpha1.OSImageStream:
+				// If given, it's treated as user input with config such as the default stream
+				osImageStream = obj
 			default:
 				klog.Infof("skipping %q [%d] manifest because of unhandled %T", file.Name(), idx+1, obji)
 			}
@@ -224,10 +228,17 @@ func (b *Bootstrap) Run(destDir string) error {
 		return fmt.Errorf("error filtering pools: %w", err)
 	}
 
-	var osImageStream *mcfgv1alpha1.OSImageStream
 	// Enable OSImageStreams if the FeatureGate is active and the deployment is not OKD
 	if osimagestream.IsFeatureEnabled(fgHandler) {
-		osImageStream, err = b.fetchOSImageStream(imageStream, cconfig, icspRules, idmsRules, itmsRules, imgCfg, pullSecret)
+		osImageStream, err = b.fetchOSImageStream(
+			imageStream,
+			cconfig,
+			icspRules,
+			idmsRules,
+			itmsRules,
+			imgCfg,
+			pullSecret,
+			osImageStream)
 		if err != nil {
 			return err
 		}
@@ -240,6 +251,9 @@ func (b *Bootstrap) Run(destDir string) error {
 		}
 		cconfig.Spec.BaseOSContainerImage = string(defaultStreamSet.OSImage)
 		cconfig.Spec.BaseOSExtensionsContainerImage = string(defaultStreamSet.OSExtensionsImage)
+	} else {
+		// Just ensuring the osImageStream is nil if the FG is disabled
+		osImageStream = nil
 	}
 
 	pullSecretBytes := pullSecret.Data[corev1.DockerConfigJsonKey]
@@ -462,6 +476,7 @@ func (b *Bootstrap) fetchOSImageStream(
 	itmsRules []*apicfgv1.ImageTagMirrorSet,
 	imgCfg *apicfgv1.Image,
 	pullSecret *corev1.Secret,
+	existingOSImageStream *mcfgv1alpha1.OSImageStream,
 ) (*mcfgv1alpha1.OSImageStream, error) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
@@ -479,15 +494,25 @@ func (b *Bootstrap) fetchOSImageStream(
 		sysCtxBuilder.WithRegistriesConfig(registriesConfig)
 	}
 
-	osImageStream, err := osimagestream.BuildOsImageStreamBootstrap(ctx,
-		sysCtxBuilder,
-		imageStream,
-		&osimagestream.OSImageTuple{
+	sysCtx, err := sysCtxBuilder.Build()
+	if err != nil {
+		return nil, fmt.Errorf("could not prepare for OSImageStream inspection: %w", err)
+	}
+	defer func() {
+		if err := sysCtx.Cleanup(); err != nil {
+			klog.Warningf("Unable to clean resources after OSImageStream inspection: %s", err)
+		}
+	}()
+
+	factory := osimagestream.NewDefaultStreamSourceFactory(&osimagestream.DefaultImagesInspectorFactory{})
+	osImageStream, err := factory.Create(ctx, sysCtx.SysContext, osimagestream.CreateOptions{
+		ExistingOSImageStream: existingOSImageStream,
+		ReleaseImageStream:    imageStream,
+		CliImages: &osimagestream.OSImageTuple{
 			OSImage:           cconfig.Spec.BaseOSContainerImage,
 			OSExtensionsImage: cconfig.Spec.BaseOSExtensionsContainerImage,
 		},
-		osimagestream.NewDefaultStreamSourceFactory(nil, &osimagestream.DefaultImagesInspectorFactory{}),
-	)
+	})
 	if err != nil {
 		return nil, fmt.Errorf("error inspecting available OSImageStreams: %w", err)
 	}

pkg/controller/internalreleaseimage/internalreleaseimage_controller.go

@@ -389,7 +389,11 @@ func (ctrl *Controller) initializeInternalReleaseImageStatus(iri *mcfgv1alpha1.I
 	klog.V(4).Infof("Initializing status for InternalReleaseImage %s", iri.Name)
 
 	// Get the release payload image from ClusterVersion
-	releaseImage, err := osimagestream.GetReleasePayloadImage(ctrl.clusterVersionLister)
+	clusterVersion, err := osimagestream.GetClusterVersion(ctrl.clusterVersionLister)
+	if err != nil {
+		return fmt.Errorf("error getting ClusterVersion for InternalReleaseImage status initialization: %w", err)
+	}
+	releaseImage, err := osimagestream.GetReleasePayloadImage(clusterVersion)
 	if err != nil {
 		return fmt.Errorf("error getting Release Image from ClusterVersion for InternalReleaseImage status initialization: %w", err)
 	}

pkg/controller/render/render_controller.go

@@ -150,6 +150,9 @@ func New(
 	if osImageStreamInformer != nil && osimagestream.IsFeatureEnabled(ctrl.fgHandler) {
 		ctrl.osImageStreamLister = osImageStreamInformer.Lister()
 		ctrl.osImageStreamListerSynced = osImageStreamInformer.Informer().HasSynced
+		osImageStreamInformer.Informer().AddEventHandler(
+			cache.ResourceEventHandlerFuncs{UpdateFunc: ctrl.updateOSImageStream},
+		)
 	}
 	return ctrl
 }
@@ -283,6 +286,28 @@ func (ctrl *Controller) updateMachineConfig(old, cur interface{}) {
 	}
 }
 
+func (ctrl *Controller) updateOSImageStream(old, cur interface{}) {
+	oldOSImageStream := old.(*v1alpha1.OSImageStream)
+	curOSImageStream := cur.(*v1alpha1.OSImageStream)
+
+	// return if status hasn't changed
+	// The operator components only reacts to what the OSImageStream
+	// publish in its status subresource.
+	if reflect.DeepEqual(oldOSImageStream.Status, curOSImageStream.Status) {
+		return
+	}
+	klog.V(4).Info("OSImageStream updated")
+
+	pList, err := ctrl.mcpLister.List(labels.Everything())
+	if err != nil {
+		klog.Errorf("error listing MCPs for OSImageStream update: %v", err)
+	}
+
+	for _, p := range pList {
+		ctrl.enqueueMachineConfigPool(p)
+	}
+}
+
 func (ctrl *Controller) deleteMachineConfig(obj interface{}) {
 	mc, ok := obj.(*mcfgv1.MachineConfig)
 

pkg/operator/operator.go

@@ -372,6 +372,8 @@ func New(
 	if osImageStreamInformer != nil && osimagestream.IsFeatureEnabled(optr.fgHandler) {
 		optr.osImageStreamLister = osImageStreamInformer.Lister()
 		optr.osImageStreamListerSynced = osImageStreamInformer.Informer().HasSynced
+		// TODO: Move this AddEventHandler to the main loop when the FG is removed
+		osImageStreamInformer.Informer().AddEventHandler(optr.eventHandler())
 	}
 	if iriInformer != nil {
 		optr.iriLister = iriInformer.Lister()

pkg/operator/osimagestream_ocp.go

@@ -32,27 +32,70 @@ func (optr *Operator) syncOSImageStream(_ *renderConfig, _ *configv1.ClusterOper
 	// This sync runs once per version. Before performing the streams fetching
 	// process, that takes time as it requires inspecting images, ensure this function
 	// needs to build the stream.
-	existingOSImageStream, updateRequired, err := optr.isOSImageStreamBuildRequired()
-	if !updateRequired || err != nil {
+	existingOSImageStream, rebuildRequired, err := optr.isOSImageStreamBuildRequired()
+	if err != nil {
 		return err
 	}
 
-	// If the code reaches this point the OSImageStream CR is not
-	// present (new cluster) or it's out-dated (cluster update).
-	// Build the new OSImageStream and push it.
-	return optr.buildOSImageStream(existingOSImageStream)
+	if rebuildRequired {
+		// If the code reaches this point the OSImageStream status containing
+		// the available OS Image Streams is outdated or doesn't exist.
+		return optr.buildOSImageStream(existingOSImageStream)
+	} else if existingOSImageStream != nil {
+		// We didn't require a rebuild, but check if we need to update the default
+		return optr.updateOSImageStream(existingOSImageStream)
+	}
+
+	return err
+}
+
+func (optr *Operator) updateOSImageStream(existingOSImageStream *v1alpha1.OSImageStream) error {
+	requestedDefault := osimagestream.GetOSImageStreamSpecDefault(existingOSImageStream)
+	if requestedDefault == "" {
+		// Nothing to do. Empty requests are ignored
+		return nil
+	}
+
+	currentDefault := existingOSImageStream.Status.DefaultStream
+	if currentDefault != requestedDefault {
+		if _, err := osimagestream.GetOSImageStreamSetByName(existingOSImageStream, requestedDefault); err != nil {
+			return fmt.Errorf("error syncing default OSImageStream with OSImageStream %s: %v", requestedDefault, err)
+		}
+
+		// DeepCopy to avoid mutating the shared informer cache
+		osImageStream := existingOSImageStream.DeepCopy()
+		osImageStream.Status.DefaultStream = requestedDefault
+		if _, err := optr.client.
+			MachineconfigurationV1alpha1().
+			OSImageStreams().
+			UpdateStatus(context.TODO(), osImageStream, metav1.UpdateOptions{}); err != nil {
+			return fmt.Errorf("error updating the default OSImageStream status: %w", err)
+		}
 
+		klog.Infof("OSImageStream default has changed from %s to %s", currentDefault, requestedDefault)
+	}
+	return nil
 }
 
 func (optr *Operator) buildOSImageStream(existingOSImageStream *v1alpha1.OSImageStream) error {
 	klog.Info("Starting building of the OSImageStream instance")
 
 	// Get the release payload image from ClusterVersion
-	image, err := osimagestream.GetReleasePayloadImage(optr.clusterVersionLister)
+	clusterVersion, err := osimagestream.GetClusterVersion(optr.clusterVersionLister)
+	if err != nil {
+		return fmt.Errorf("error getting cluster version for OSImageStream inspection: %w", err)
+	}
+	image, err := osimagestream.GetReleasePayloadImage(clusterVersion)
 	if err != nil {
 		return fmt.Errorf("error getting the Release Image digest from the ClusterVersion for OSImageStream sync: %w", err)
 	}
 
+	// The original cluster version is used as a fallback to infer the default stream
+	installVersion, err := osimagestream.GetInstallVersion(clusterVersion)
+	if err != nil {
+		klog.Warningf("Unable to get install version for OSImageStream build: %s", err)
+	}
+
 	// Get the cluster pull secret from well-known location
 	clusterPullSecret, err := optr.kubeClient.CoreV1().Secrets("openshift-config").Get(context.TODO(), "pull-secret", metav1.GetOptions{})
 	if err != nil {
@@ -76,8 +119,26 @@ func (optr *Operator) buildOSImageStream(existingOSImageStream *v1alpha1.OSImage
 		return fmt.Errorf("could not build SysContext for OSImageStream build: %w", err)
 	}
 
-	imageStreamFactory := osimagestream.NewDefaultStreamSourceFactory(optr.mcoCmLister, &osimagestream.DefaultImagesInspectorFactory{})
-	osImageStream, err := osimagestream.BuildOsImageStreamRuntime(buildCtx, sysCtxBuilder, image, imageStreamFactory)
+	sysCtx, err := sysCtxBuilder.Build()
+	if err != nil {
+		return fmt.Errorf("could not prepare for OSImageStream inspection: %w", err)
+	}
+	defer func() {
+		if err := sysCtx.Cleanup(); err != nil {
+			klog.Warningf("Unable to clean resources after OSImageStream inspection: %s", err)
+		}
+	}()
+
+	factory := osimagestream.NewDefaultStreamSourceFactory(&osimagestream.DefaultImagesInspectorFactory{})
+	osImageStream, err := factory.Create(
+		buildCtx,
+		sysCtx.SysContext,
+		osimagestream.CreateOptions{
+			ReleaseImage:          image,
+			ConfigMapLister:       optr.mcoCmLister,
+			ExistingOSImageStream: existingOSImageStream,
+			InstallVersion:        installVersion,
+		})
 	if err != nil {
 		return fmt.Errorf("error building the OSImageStream: %w", err)
 	}
@@ -96,8 +157,10 @@ func (optr *Operator) buildOSImageStream(existingOSImageStream *v1alpha1.OSImage
 		oldVersion := existingOSImageStream.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey]
 		klog.V(4).Infof("Updating OSImageStream (previous version: %s, new version: %s)", oldVersion, version.Hash)
 		// Update metadata/spec first (mainly for annotations)
-		existingOSImageStream.ObjectMeta.Annotations = osImageStream.ObjectMeta.Annotations
-		updateOSImageStream, err = optr.client.MachineconfigurationV1alpha1().OSImageStreams().Update(context.TODO(), existingOSImageStream, metav1.UpdateOptions{})
+		// DeepCopy to avoid mutating the shared informer cache
+		desired := existingOSImageStream.DeepCopy()
+		desired.ObjectMeta.Annotations = osImageStream.ObjectMeta.Annotations
+		updateOSImageStream, err = optr.client.MachineconfigurationV1alpha1().OSImageStreams().Update(context.TODO(), desired, metav1.UpdateOptions{})
 		if err != nil {
 			return fmt.Errorf("error updating the OSImageStream: %w", err)
 		}
@@ -179,9 +242,9 @@ func (optr *Operator) isOSImageStreamBuildRequired() (*v1alpha1.OSImageStream, b
 	}
 
 	// Check if an update is needed
-	if !osImageStreamRequiresUpdate(existingOSImageStream) {
+	if !osImageStreamRequiresRebuild(existingOSImageStream) {
 		klog.V(4).Info("OSImageStream is already up-to-date, skipping sync")
-		return nil, false, nil
+		return existingOSImageStream, false, nil
 	}
 	return existingOSImageStream, true, nil
 }
@@ -235,9 +298,9 @@ func (optr *Operator) getExistingOSImageStream() (*v1alpha1.OSImageStream, error
 	return osImageStream, nil
 }
 
-// osImageStreamRequiresUpdate checks if the OSImageStream needs to be created or updated.
-// Returns true if osImageStream is nil or if its version annotation doesn't match the current version.
-func osImageStreamRequiresUpdate(osImageStream *v1alpha1.OSImageStream) bool {
+// osImageStreamRequiresRebuild checks if the OSImageStream needs to be created or updated.
+// Returns true if osImageStream is nil, if its version annotation doesn't match the current version.
+func osImageStreamRequiresRebuild(osImageStream *v1alpha1.OSImageStream) bool {
 	if osImageStream == nil {
 		return true
 	}

pkg/operator/sync.go

@@ -106,6 +106,8 @@ const (
 	mccUpdateBootImagesValidatingAdmissionPolicyBindingPath               = "manifests/machineconfigcontroller/update-bootimages-validatingadmissionpolicybinding.yaml"
 	mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyPath        = "manifests/machineconfigcontroller/machineconfigpool-osimagestream-reference-validatingadmissionpolicy.yaml"
 	mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyBindingPath = "manifests/machineconfigcontroller/machineconfigpool-osimagestream-reference-validatingadmissionpolicybinding.yaml"
+	mccOSImageStreamDeletionGuardValidatingAdmissionPolicyPath            = "manifests/machineconfigcontroller/osimagestream-deletion-guard-validatingadmissionpolicy.yaml"
+	mccOSImageStreamDeletionGuardValidatingAdmissionPolicyBindingPath     = "manifests/machineconfigcontroller/osimagestream-deletion-guard-validatingadmissionpolicybinding.yaml"
 	mccIRIDeletionGuardValidatingAdmissionPolicyPath                      = "manifests/machineconfigcontroller/internalreleaseimage-deletion-guard-validatingadmissionpolicy.yaml"
 	mccIRIDeletionGuardValidatingAdmissionPolicyBindingPath               = "manifests/machineconfigcontroller/internalreleaseimage-deletion-guard-validatingadmissionpolicybinding.yaml"
 	mccUpdateBootImagesCPMSValidatingAdmissionPolicyPath                  = "manifests/machineconfigcontroller/update-bootimages-cpms-validatingadmissionpolicy.yaml"
@@ -1207,8 +1209,14 @@ func (optr *Operator) syncMachineConfigController(config *renderConfig, _ *confi
 
 	// Only deploy OS image stream validating admission policies when the feature is enabled and the cluster isn't OKD
 	if osimagestream.IsFeatureEnabled(optr.fgHandler) {
-		paths.validatingAdmissionPolicies = append(paths.validatingAdmissionPolicies, mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyPath)
-		paths.validatingAdmissionPolicyBindings = append(paths.validatingAdmissionPolicyBindings, mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyBindingPath)
+		paths.validatingAdmissionPolicies = append(paths.validatingAdmissionPolicies,
+			mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyPath,
+			mccOSImageStreamDeletionGuardValidatingAdmissionPolicyPath,
+		)
+		paths.validatingAdmissionPolicyBindings = append(paths.validatingAdmissionPolicyBindings,
+			mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyBindingPath,
+			mccOSImageStreamDeletionGuardValidatingAdmissionPolicyBindingPath,
+		)
 	}
 	if optr.fgHandler.Enabled(features.FeatureGateManagedBootImagesCPMS) {
 		paths.validatingAdmissionPolicies = append(paths.validatingAdmissionPolicies, mccUpdateBootImagesCPMSValidatingAdmissionPolicyPath)