[DO NOT MERGE] - expands skip-ignition-apply work in MCD

pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template

@@ -13,11 +13,6 @@ RUN mkdir -p /etc/machine-config-daemon && \
 FROM {{.BaseOSImage}} AS configs
 # Copy the extracted MachineConfig into the expected place in the image.
 COPY --from=extract /etc/machine-config-daemon/currentconfig /etc/machine-config-daemon/currentconfig
-# Do the ignition live-apply, extracting the Ignition config from the MachineConfig.
-# Not sure why Ignition explicitly requires the container env var to be set
-# since it should be set by the container runtime / builder.
-RUN container="oci" exec -a ignition-apply /usr/lib/dracut/modules.d/30ignition/ignition --ignore-unsupported <(cat /etc/machine-config-daemon/currentconfig | jq '.spec.config') && \
-	ostree container commit
 # Install any extensions specified
 {{if and .ExtensionsImage .ExtensionsPackages}}
 # Mount the extensions image to use the content from it

pkg/controller/build/clients.go

@@ -23,6 +23,7 @@ import (
 type informers struct {
 	controllerConfigInformer  mcfginformersv1.ControllerConfigInformer
 	machineConfigPoolInformer mcfginformersv1.MachineConfigPoolInformer
+	machineConfigInformer     mcfginformersv1.MachineConfigInformer
 	jobInformer               batchinformersv1.JobInformer
 	machineOSBuildInformer    mcfginformersv1.MachineOSBuildInformer
 	machineOSConfigInformer   mcfginformersv1.MachineOSConfigInformer
@@ -43,6 +44,7 @@ func (i *informers) listers() *listers {
 		machineOSBuildLister:    i.machineOSBuildInformer.Lister(),
 		machineOSConfigLister:   i.machineOSConfigInformer.Lister(),
 		machineConfigPoolLister: i.machineConfigPoolInformer.Lister(),
+		machineConfigLister:     i.machineConfigInformer.Lister(),
 		jobLister:               i.jobInformer.Lister(),
 		controllerConfigLister:  i.controllerConfigInformer.Lister(),
 	}
@@ -53,6 +55,7 @@ type listers struct {
 	machineOSBuildLister    mcfglistersv1.MachineOSBuildLister
 	machineOSConfigLister   mcfglistersv1.MachineOSConfigLister
 	machineConfigPoolLister mcfglistersv1.MachineConfigPoolLister
+	machineConfigLister     mcfglistersv1.MachineConfigLister
 	jobLister               batchlisterv1.JobLister
 	controllerConfigLister  mcfglistersv1.ControllerConfigLister
 }
@@ -84,6 +87,7 @@ func newInformers(mcfgclient mcfgclientset.Interface, kubeclient clientset.Inter
 	controllerConfigInformer := mcoInformerFactory.Machineconfiguration().V1().ControllerConfigs()
 	machineConfigPoolInformer := mcoInformerFactory.Machineconfiguration().V1().MachineConfigPools()
 	machineOSBuildInformer := mcoInformerFactory.Machineconfiguration().V1().MachineOSBuilds()
+	machineConfigInformer := mcoInformerFactory.Machineconfiguration().V1().MachineConfigs()
 	machineOSConfigInformer := mcoInformerFactory.Machineconfiguration().V1().MachineOSConfigs()
 	jobInformer := coreInformerFactory.Batch().V1().Jobs()
 
@@ -92,6 +96,7 @@ func newInformers(mcfgclient mcfgclientset.Interface, kubeclient clientset.Inter
 		machineConfigPoolInformer: machineConfigPoolInformer,
 		machineOSBuildInformer:    machineOSBuildInformer,
 		machineOSConfigInformer:   machineOSConfigInformer,
+		machineConfigInformer:     machineConfigInformer,
 		jobInformer:               jobInformer,
 		toStart: []interface{ Start(<-chan struct{}) }{
 			mcoInformerFactory,
@@ -103,6 +108,7 @@ func newInformers(mcfgclient mcfgclientset.Interface, kubeclient clientset.Inter
 			jobInformer.Informer().HasSynced,
 			machineOSBuildInformer.Informer().HasSynced,
 			machineOSConfigInformer.Informer().HasSynced,
+			machineConfigInformer.Informer().HasSynced,
 		},
 	}
 }

pkg/controller/build/reconciler.go

@@ -20,6 +20,7 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 
+	"github.com/openshift/machine-config-operator/pkg/version"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -103,7 +104,16 @@ func (b *buildReconciler) updateMachineOSConfig(ctx context.Context, old, cur *m
 	// Whenever the MachineOSConfig spec has changed, create a new MachineOSBuild.
 	if !equality.Semantic.DeepEqual(old.Spec, cur.Spec) {
 		klog.Infof("Detected MachineOSConfig change for %s", cur.Name)
-		return b.createNewMachineOSBuildOrReuseExisting(ctx, cur)
+
+		// This could be a potential problem for future Day 0 operations, but it's fine here for now.
+		if cur.Spec.MachineConfigPool.Name == "" {
+			return fmt.Errorf("MachineOSConfig %s has empty MachineConfigPool.Name", cur.Name)
+		}
+
+		if err := b.createNewMachineOSBuildOrReuseExisting(ctx, cur); err != nil {
+			return fmt.Errorf("failed to create/reuse build for MOSC %q: %w", cur.Name, err)
+		}
+		return nil
 	}
 
 	return b.syncMachineOSConfigs(ctx)
@@ -356,18 +366,92 @@ func (b *buildReconciler) UpdateMachineConfigPool(ctx context.Context, oldMCP, c
 // Sepcifically, whenever a new rendered MachineConfig is applied, it will
 // create a new MachineOSBuild in response.
 func (b *buildReconciler) updateMachineConfigPool(ctx context.Context, oldMCP, curMCP *mcfgv1.MachineConfigPool) error {
-	if oldMCP.Spec.Configuration.Name != curMCP.Spec.Configuration.Name {
-		klog.Infof("Rendered config for pool %s changed from %s to %s", curMCP.Name, oldMCP.Spec.Configuration.Name, curMCP.Spec.Configuration.Name)
-		if err := b.createNewMachineOSBuildOrReuseExistingForPoolChange(ctx, curMCP); err != nil {
-			return fmt.Errorf("could not create or reuse existing MachineOSBuild for MachineConfigPool %q change: %w", curMCP.Name, err)
-		}
+	// Only proceed if the rendered configuration changed
+	if oldMCP.Spec.Configuration.Name == curMCP.Spec.Configuration.Name {
+		return nil
+	}
+
+	// Get current and desired MachineConfigs
+	currMC, err := b.machineConfigLister.Get(oldMCP.Spec.Configuration.Name)
+	if err != nil {
+		return fmt.Errorf("error getting current MachineConfig %s: %w", oldMCP.Spec.Configuration.Name, err)
 	}
 
-	// Not sure if we need to do this here yet or not.
-	// TODO: Determine if we should call b.syncMachineConfigPools() here or not.
+	desMC, err := b.machineConfigLister.Get(curMCP.Spec.Configuration.Name)
+	if err != nil {
+		return fmt.Errorf("error getting desired MachineConfig %s: %w", curMCP.Spec.Configuration.Name, err)
+	}
+
+	// Determine rebuild requirements using shared logic
+	needsBuild := ctrlcommon.RequiresRebuild(currMC, desMC)
+
+	if needsBuild {
+		// Get the MachineOSConfig for this MCP
+		if _, err := b.machineOSConfigLister.Get(curMCP.Name); err != nil {
+			if k8serrors.IsNotFound(err) {
+				klog.Warningf("No MachineOSConfig found for pool %s, skipping OCL build", curMCP.Name)
+				return nil
+			}
+			return fmt.Errorf("error checking for MachineOSConfig: %w", err)
+		}
+
+		// Proceed with build creation
+		return b.createNewMachineOSBuildOrReuseExistingForPoolChange(ctx, curMCP)
+	}
+	// Otherwise, no build is required and we skip
+	klog.Infof("MachineConfig %s to %s changes do not require a build, skipping", oldMCP.Name, curMCP.Name)
 	return b.syncAll(ctx)
 }
 
+// Helper to generate and manage container configs for both MCP and MOSC updates
+func (b *buildReconciler) prepareContainerConfig(ctx context.Context, desiredMC *mcfgv1.MachineConfig, owner *mcfgv1.MachineOSConfig, mcpName string) (string, error) {
+	// Get controller config (common requirement)
+	cc, err := b.controllerConfigLister.Get(ctrlcommon.ControllerConfigName)
+	if err != nil {
+		return "", fmt.Errorf("error getting controller config: %w", err)
+	}
+
+	// Generate container config (shared logic)
+	containerMC, err := ctrlcommon.GenerateContainerConfig(desiredMC, cc, mcpName)
+	if err != nil {
+		return "", fmt.Errorf("error generating container config: %w", err)
+	}
+
+	// Set owner reference (standard pattern)
+	oref := metav1.NewControllerRef(owner, mcfgv1.SchemeGroupVersion.WithKind("MachineOSConfig"))
+	containerMC.SetOwnerReferences([]metav1.OwnerReference{*oref})
+
+	// Create if not exists
+	if _, err := b.mcfgclient.MachineconfigurationV1().MachineConfigs().Create(
+		ctx, containerMC, metav1.CreateOptions{}); err != nil && !k8serrors.IsAlreadyExists(err) {
+		return "", fmt.Errorf("error creating container config: %w", err)
+	}
+
+	return containerMC.Name, nil
+}
+
+func (b *buildReconciler) prepareContainerConfigForMOSC(ctx context.Context, mosc *mcfgv1.MachineOSConfig, mcpName string) (string, error) {
+	// Get associated MachineConfigPool
+	mcp, err := b.machineConfigPoolLister.Get(mosc.Spec.MachineConfigPool.Name)
+	if err != nil {
+		return "", fmt.Errorf("error getting pool: %w", err)
+	}
+
+	// Check if the pool has a valid rendered configuration
+	if mcp.Spec.Configuration.Name == "" {
+		return "", fmt.Errorf("MachineConfigPool %s has no rendered configuration", mcp.Name)
+	}
+
+	// Get desired MachineConfig from pool
+	desMC, err := b.machineConfigLister.Get(mcp.Spec.Configuration.Name)
+	if err != nil {
+		return "", fmt.Errorf("error getting desired MC: %w", err)
+	}
+
+	// Reuse existing container config logic
+	return b.prepareContainerConfig(ctx, desMC, mosc, mcpName)
+}
+
 // Adds a MachineOSBuild.
 func (b *buildReconciler) addMachineOSBuild(ctx context.Context, mosb *mcfgv1.MachineOSBuild) error {
 	return b.syncMachineOSBuild(ctx, mosb)
@@ -471,6 +555,21 @@ func (b *buildReconciler) createNewMachineOSBuildOrReuseExisting(ctx context.Con
 		return fmt.Errorf("could not instantiate new MachineOSBuild: %w", err)
 	}
 
+	var containerName string
+	if mcp.Spec.Configuration.Name != "" {
+		containerName, err = b.prepareContainerConfigForMOSC(ctx, mosc, mcp.Name)
+		if err != nil {
+			return fmt.Errorf("error preparing container config: %w", err)
+		}
+		metav1.SetMetaDataAnnotation(&mosb.ObjectMeta, "mco.openshift.io/container-config", containerName)
+	} else {
+		klog.Warningf("No rendered config for pool %s - skipping container config", mcp.Name)
+	}
+
+	// Add container-config annotation
+	metav1.SetMetaDataAnnotation(&mosb.ObjectMeta, ctrlcommon.GeneratedByControllerVersionAnnotationKey, version.Hash)
+	metav1.SetMetaDataAnnotation(&mosb.ObjectMeta, ctrlcommon.ReleaseImageVersionAnnotationKey, osImageURLs.ReleaseVersion)
+
 	// Set owner reference of the machineOSBuild to the machineOSConfig that created this
 	oref := metav1.NewControllerRef(mosc, mcfgv1.SchemeGroupVersion.WithKind("MachineOSConfig"))
 	mosb.SetOwnerReferences([]metav1.OwnerReference{*oref})

pkg/controller/common/constants.go

@@ -75,6 +75,8 @@ const (
 	// LayeringEnabledPoolLabel is the label that enables the "layered" workflow path for a pool.
 	LayeringEnabledPoolLabel = "machineconfiguration.openshift.io/layering-enabled"
 
+	ContainerBuildAnnotationKey = "machineconfiguration.openshift.io/container-build"
+
 	RebuildPoolLabel = "machineconfiguration.openshift.io/rebuildImage"
 
 	// ExperimentalNewestLayeredImageEquivalentConfigAnnotationKey is the annotation that signifies which rendered config

pkg/controller/common/containerconfig.go

@@ -0,0 +1,55 @@
+package common
+
+import (
+	"fmt"
+
+	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
+	"github.com/openshift/machine-config-operator/pkg/version"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+func GenerateContainerConfig(baseMC *mcfgv1.MachineConfig, cc *mcfgv1.ControllerConfig, mcpName string) (*mcfgv1.MachineConfig, error) {
+	// containerMC := &mcfgv1.MachineConfig{
+	// 	Spec: baseMC.Spec,
+	// }
+
+	containerMC := &mcfgv1.MachineConfig{
+		Spec: mcfgv1.MachineConfigSpec{
+			OSImageURL:      baseMC.Spec.OSImageURL,
+			KernelType:      baseMC.Spec.KernelType,
+			Extensions:      baseMC.Spec.Extensions,
+			KernelArguments: baseMC.Spec.KernelArguments,
+		},
+	}
+
+	// Empty ignition config- testing this because we need a valid version and it fails with
+	// saying the config uses a unsupported spec version
+	containerMC.Spec.Config = runtime.RawExtension{
+		Raw: []byte(`{
+            "ignition": { 
+                "version": "3.4.0" 
+            }
+        }`),
+	}
+
+	// Generate deterministic name
+	hash, err := HashMachineConfigSpec(containerMC.Spec)
+	if err != nil {
+		return nil, fmt.Errorf("error hashing MachineConfigSpec: %w", err)
+	}
+	containerMC.Name = fmt.Sprintf("container-config-%s-%s", mcpName, hash)
+
+	// Initialize annotations with all values at once
+	containerMC.Annotations = map[string]string{
+		ContainerBuildAnnotationKey:               "true",
+		GeneratedByControllerVersionAnnotationKey: version.Hash,
+		ReleaseImageVersionAnnotationKey:          cc.Annotations[ReleaseImageVersionAnnotationKey],
+	}
+
+	// Validate before returning
+	if err := ValidateMachineConfig(containerMC.Spec); err != nil {
+		return nil, fmt.Errorf("invalid container config: %w", err)
+	}
+
+	return containerMC, nil
+}

pkg/controller/common/hash.go

@@ -0,0 +1,47 @@
+package common
+
+import (
+	//nolint:gosec
+	"crypto/md5"
+	"fmt"
+
+	"github.com/ghodss/yaml"
+	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
+)
+
+var (
+	// salt is 80 random bytes.
+	// The salt was generated by `od -vAn -N80 -tu1 < /dev/urandom`. Do not change it.
+	salt = []byte{
+		16, 124, 206, 228, 139, 56, 175, 175, 79, 229, 134, 118, 157, 154, 211, 110,
+		25, 93, 47, 253, 172, 106, 37, 7, 174, 13, 160, 185, 110, 17, 87, 52,
+		219, 131, 12, 206, 218, 141, 116, 135, 188, 181, 192, 151, 233, 62, 126, 165,
+		64, 83, 179, 119, 15, 168, 208, 197, 146, 107, 58, 227, 133, 188, 238, 26,
+		33, 26, 235, 202, 32, 173, 31, 234, 41, 144, 148, 79, 6, 206, 23, 22,
+	}
+)
+
+func HashMachineConfigSpec(spec mcfgv1.MachineConfigSpec) (string, error) {
+	data, err := yaml.Marshal(spec)
+	if err != nil {
+		return "", err
+	}
+
+	h, err := hashData(data)
+	if err != nil {
+		return "", err
+	}
+	return fmt.Sprintf("%x", h), nil
+}
+
+func hashData(data []byte) ([]byte, error) {
+	//nolint:gosec
+	hasher := md5.New()
+	if _, err := hasher.Write(salt); err != nil {
+		return nil, fmt.Errorf("error computing hash: %w", err)
+	}
+	if _, err := hasher.Write(data); err != nil {
+		return nil, fmt.Errorf("error computing hash: %w", err)
+	}
+	return hasher.Sum(nil), nil
+}

pkg/controller/common/helpers.go

@@ -21,6 +21,7 @@ import (
 	"text/template"
 
 	"github.com/clarketm/json"
+
 	fcctbase "github.com/coreos/fcct/base/v0_1"
 	"github.com/coreos/ign-converter/translate/v23tov30"
 	"github.com/coreos/ign-converter/translate/v32tov22"
@@ -1397,3 +1398,16 @@ func GetCAsFromConfigMap(cm *corev1.ConfigMap, key string) ([]byte, error) {
 	}
 	return nil, fmt.Errorf("%s not found in %s/%s", key, cm.Namespace, cm.Name)
 }
+
+// Determines if an on-cluster layering rebuild is required for the changes
+func RequiresRebuild(oldMC, newMC *mcfgv1.MachineConfig) bool {
+	return oldMC.Spec.OSImageURL != newMC.Spec.OSImageURL ||
+		oldMC.Spec.KernelType != newMC.Spec.KernelType ||
+		!reflect.DeepEqual(oldMC.Spec.Extensions, newMC.Spec.Extensions) ||
+		!reflect.DeepEqual(oldMC.Spec.KernelArguments, newMC.Spec.KernelArguments)
+}
+
+// Determines if changes can be safely applied without node disruption
+func CanBeInPlaceApplied(oldMC, newMC *mcfgv1.MachineConfig) bool {
+	return !RequiresRebuild(oldMC, newMC)
+}

pkg/controller/render/render_controller.go

@@ -451,6 +451,17 @@ func (ctrl *Controller) syncMachineConfigPool(key string) error {
 	if err != nil {
 		return err
 	}
+
+	filteredMcs := make([]*mcfgv1.MachineConfig, 0, len(mcs))
+	for _, mc := range mcs {
+		// Only skip container-build configs
+		if _, ok := mc.Annotations[ctrlcommon.ContainerBuildAnnotationKey]; ok {
+			continue
+		}
+		filteredMcs = append(filteredMcs, mc)
+	}
+	mcs = filteredMcs
+
 	sort.SliceStable(mcs, func(i, j int) bool { return mcs[i].Name < mcs[j].Name })
 
 	if len(mcs) == 0 {