minimizing reboot for scaling image mode enabled node

Author: dkhater-redhat

manifests/machineconfigserver/clusterrole.yaml

@@ -9,10 +9,19 @@ rules:
 - apiGroups: ["machineconfiguration.openshift.io"]
   resources: ["controllerconfigs"]
   verbs: ["get", "watch", "list"]
+- apiGroups: ["machineconfiguration.openshift.io"]
+  resources: ["machineosconfigs", "machineosbuilds"]
+  verbs: ["get", "list", "watch"]
 - apiGroups: ["security.openshift.io"]
   resourceNames: ["hostnetwork"]
   resources: ["securitycontextconstraints"]
   verbs: ["use"]
 - apiGroups: [""]
   resources: ["configmaps"]
   verbs: ["get", "list", "watch"]
+- apiGroups: [""]
+  resources: ["services"]
+  verbs: ["get", "list"]
+- apiGroups: ["route.openshift.io"]
+  resources: ["routes"]
+  verbs: ["get", "list"]

pkg/controller/build/reconciler.go

@@ -999,7 +999,7 @@ func (b *buildReconciler) deleteMOSBImage(ctx context.Context, mosb *mcfgv1.Mach
 	}
 
 	image := string(mosb.Spec.RenderedImagePushSpec)
-	isOpenShiftRegistry, err := b.isOpenShiftRegistry(image)
+	isOpenShiftRegistry, err := ctrlcommon.IsOpenShiftRegistry(context.TODO(), image, b.kubeclient, b.routeclient)
 	if err != nil {
 		return err
 	}
@@ -1039,53 +1039,6 @@ func (b *buildReconciler) deleteMOSBImage(ctx context.Context, mosb *mcfgv1.Mach
 	return nil
 }
 
-// getInternalRegistryHostnames discovers OpenShift internal registry hostnames
-func (b *buildReconciler) getInternalRegistryHostnames(ctx context.Context) ([]string, error) {
-	var hostnames []string
-
-	// Get the list of services in the openshift-image-registry namespace (cluster-local)
-	services, err := b.kubeclient.CoreV1().Services("openshift-image-registry").List(ctx, metav1.ListOptions{})
-	if err != nil {
-		return nil, err
-	}
-	for _, svc := range services.Items {
-		clusterHostname := fmt.Sprintf("%s.%s.svc", svc.Name, svc.Namespace)
-		if len(svc.Spec.Ports) > 0 {
-			port := svc.Spec.Ports[0].Port
-			hostnames = append(hostnames, fmt.Sprintf("%s:%d", clusterHostname, port))
-		} else {
-			hostnames = append(hostnames, clusterHostname)
-		}
-	}
-
-	// Get the list of routes in the openshift-image-registry namespace (external access)
-	routes, err := b.routeclient.RouteV1().Routes("openshift-image-registry").List(ctx, metav1.ListOptions{})
-	if err != nil {
-		return nil, err
-	}
-	for _, route := range routes.Items {
-		if route.Spec.Host != "" {
-			hostnames = append(hostnames, route.Spec.Host)
-		}
-	}
-
-	return hostnames, nil
-}
-
-// isOpenShiftRegistry checks if the imageRef points to one of the known internal hostnames
-func (b *buildReconciler) isOpenShiftRegistry(imageRef string) (bool, error) {
-	registryHosts, err := b.getInternalRegistryHostnames(context.TODO())
-	if err != nil {
-		return false, err
-	}
-	for _, host := range registryHosts {
-		if strings.HasPrefix(imageRef, host) {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
 // Finds and deletes any other running builds for a given MachineOSConfig.
 func (b *buildReconciler) deleteOtherBuildsForMachineOSConfig(ctx context.Context, newMosb *mcfgv1.MachineOSBuild, mosc *mcfgv1.MachineOSConfig) error {
 	mosbList, err := b.getMachineOSBuildsForMachineOSConfig(mosc)

pkg/controller/common/registry_utils.go

@@ -0,0 +1,61 @@
+package common
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	routeclientset "github.com/openshift/client-go/route/clientset/versioned"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	clientset "k8s.io/client-go/kubernetes"
+)
+
+// GetInternalRegistryHostnames discovers OpenShift internal registry hostnames
+// by querying Services and Routes in the openshift-image-registry namespace.
+func GetInternalRegistryHostnames(ctx context.Context, kubeclient clientset.Interface, routeclient routeclientset.Interface) ([]string, error) {
+	var hostnames []string
+
+	// Get the list of services in the openshift-image-registry namespace (cluster-local)
+	services, err := kubeclient.CoreV1().Services("openshift-image-registry").List(ctx, metav1.ListOptions{})
+	if err != nil {
+		return nil, err
+	}
+	for _, svc := range services.Items {
+		clusterHostname := fmt.Sprintf("%s.%s.svc", svc.Name, svc.Namespace)
+		if len(svc.Spec.Ports) > 0 {
+			port := svc.Spec.Ports[0].Port
+			hostnames = append(hostnames, fmt.Sprintf("%s:%d", clusterHostname, port))
+		} else {
+			hostnames = append(hostnames, clusterHostname)
+		}
+	}
+
+	// Get the list of routes in the openshift-image-registry namespace (external access)
+	routes, err := routeclient.RouteV1().Routes("openshift-image-registry").List(ctx, metav1.ListOptions{})
+	if err != nil {
+		return nil, err
+	}
+	for _, route := range routes.Items {
+		if route.Spec.Host != "" {
+			hostnames = append(hostnames, route.Spec.Host)
+		}
+	}
+
+	return hostnames, nil
+}
+
+// IsOpenShiftRegistry checks if the imageRef points to one of the known internal registry hostnames
+func IsOpenShiftRegistry(ctx context.Context, imageRef string, kubeclient clientset.Interface, routeclient routeclientset.Interface) (bool, error) {
+	registryHosts, err := GetInternalRegistryHostnames(ctx, kubeclient, routeclient)
+	if err != nil {
+		return false, err
+	}
+
+	for _, host := range registryHosts {
+		if strings.HasPrefix(imageRef, host) {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}

pkg/daemon/daemon.go

@@ -2174,7 +2174,12 @@ func (dn *Daemon) checkStateOnFirstRun() error {
 
 	// Bootstrapping state is when we have the node annotations file
 	if state.bootstrapping {
-		targetOSImageURL := state.currentConfig.Spec.OSImageURL
+		// During bootstrap, prefer the image from node annotations (if set) over the MC from cluster lister
+		targetOSImageURL := state.currentImage
+		if targetOSImageURL == "" {
+			// Fall back to MC's OSImageURL if no image annotation was provided
+			targetOSImageURL = state.currentConfig.Spec.OSImageURL
+		}
 		osMatch := dn.checkOS(targetOSImageURL)
 		if !osMatch {
 			logSystem("Bootstrap pivot required to: %s", targetOSImageURL)

pkg/server/cluster_server.go

@@ -1,6 +1,7 @@
 package server
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -9,14 +10,19 @@ import (
 	"path/filepath"
 	"time"
 
+	ign3types "github.com/coreos/ignition/v2/config/v3_5/types"
 	yaml "github.com/ghodss/yaml"
+	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	mcfginformers "github.com/openshift/client-go/machineconfiguration/informers/externalversions"
 
+	routeclientset "github.com/openshift/client-go/route/clientset/versioned"
 	"github.com/openshift/machine-config-operator/internal/clients"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/informers"
+	clientset "k8s.io/client-go/kubernetes"
 	corelisterv1 "k8s.io/client-go/listers/core/v1"
 	"k8s.io/client-go/tools/cache"
 	clientcmdv1 "k8s.io/client-go/tools/clientcmd/api/v1"
@@ -42,6 +48,11 @@ type clusterServer struct {
 	machineConfigLister     v1.MachineConfigLister
 	controllerConfigLister  v1.ControllerConfigLister
 	configMapLister         corelisterv1.ConfigMapLister
+	machineOSConfigLister   v1.MachineOSConfigLister
+	machineOSBuildLister    v1.MachineOSBuildLister
+
+	kubeclient  clientset.Interface
+	routeclient routeclientset.Interface
 
 	kubeconfigFunc kubeconfigFunc
 	apiserverURL   string
@@ -73,26 +84,31 @@ func NewClusterServer(kubeConfig, apiserverURL string) (Server, error) {
 
 	machineConfigClient := clientsBuilder.MachineConfigClientOrDie("machine-config-shared-informer")
 	kubeClient := clientsBuilder.KubeClientOrDie("kube-client-shared-informer")
+	routeClient := clientsBuilder.RouteClientOrDie("route-client")
 	sharedInformerFactory := mcfginformers.NewSharedInformerFactory(machineConfigClient, resyncPeriod()())
 	kubeNamespacedSharedInformer := informers.NewFilteredSharedInformerFactory(kubeClient, resyncPeriod()(), "openshift-machine-config-operator", nil)
 
-	mcpInformer, mcInformer, ccInformer, cmInformer :=
+	mcpInformer, mcInformer, ccInformer, cmInformer, moscInformer, mosbInformer :=
 		sharedInformerFactory.Machineconfiguration().V1().MachineConfigPools(),
 		sharedInformerFactory.Machineconfiguration().V1().MachineConfigs(),
 		sharedInformerFactory.Machineconfiguration().V1().ControllerConfigs(),
-		kubeNamespacedSharedInformer.Core().V1().ConfigMaps()
-	mcpLister, mcLister, ccLister, cmLister := mcpInformer.Lister(), mcInformer.Lister(), ccInformer.Lister(), cmInformer.Lister()
-	mcpListerHasSynced, mcListerHasSynced, ccListerHasSynced, cmListerHasSynced :=
+		kubeNamespacedSharedInformer.Core().V1().ConfigMaps(),
+		sharedInformerFactory.Machineconfiguration().V1().MachineOSConfigs(),
+		sharedInformerFactory.Machineconfiguration().V1().MachineOSBuilds()
+	mcpLister, mcLister, ccLister, cmLister, moscLister, mosbLister := mcpInformer.Lister(), mcInformer.Lister(), ccInformer.Lister(), cmInformer.Lister(), moscInformer.Lister(), mosbInformer.Lister()
+	mcpListerHasSynced, mcListerHasSynced, ccListerHasSynced, cmListerHasSynced, moscListerHasSynced, mosbListerHasSynced :=
 		mcpInformer.Informer().HasSynced,
 		mcInformer.Informer().HasSynced,
 		ccInformer.Informer().HasSynced,
-		cmInformer.Informer().HasSynced
+		cmInformer.Informer().HasSynced,
+		moscInformer.Informer().HasSynced,
+		mosbInformer.Informer().HasSynced
 
 	var informerStopCh chan struct{}
 	go sharedInformerFactory.Start(informerStopCh)
 	go kubeNamespacedSharedInformer.Start(informerStopCh)
 
-	if !cache.WaitForCacheSync(informerStopCh, mcpListerHasSynced, mcListerHasSynced, ccListerHasSynced, cmListerHasSynced) {
+	if !cache.WaitForCacheSync(informerStopCh, mcpListerHasSynced, mcListerHasSynced, ccListerHasSynced, cmListerHasSynced, moscListerHasSynced, mosbListerHasSynced) {
 		return nil, errors.New("failed to wait for cache sync")
 	}
 
@@ -101,6 +117,10 @@ func NewClusterServer(kubeConfig, apiserverURL string) (Server, error) {
 		machineConfigLister:     mcLister,
 		controllerConfigLister:  ccLister,
 		configMapLister:         cmLister,
+		machineOSConfigLister:   moscLister,
+		machineOSBuildLister:    mosbLister,
+		kubeclient:              kubeClient,
+		routeclient:             routeClient,
 		kubeconfigFunc:          func() ([]byte, []byte, error) { return kubeconfigFromSecret(bootstrapTokenDir, apiserverURL, nil) },
 		apiserverURL:            apiserverURL,
 	}, nil
@@ -163,7 +183,27 @@ func (cs *clusterServer) GetConfig(cr poolRequest) (*runtime.RawExtension, error
 
 	addDataAndMaybeAppendToIgnition(caBundleFilePath, cc.Spec.KubeAPIServerServingCAData, &ignConf)
 	addDataAndMaybeAppendToIgnition(cloudProviderCAPath, cc.Spec.CloudProviderCAData, &ignConf)
-	appenders := getAppenders(currConf, cr.version, cs.kubeconfigFunc, []string{}, "")
+
+	desiredImage := cs.resolveDesiredImageForPool(mp)
+	klog.Infof("desiredImage is found to be %s", desiredImage)
+
+	appenders := []appenderFunc{
+		func(cfg *ign3types.Config, _ *mcfgv1.MachineConfig) error {
+			return appendNodeAnnotations(cfg, currConf, desiredImage)
+		},
+		func(cfg *ign3types.Config, _ *mcfgv1.MachineConfig) error {
+			return appendKubeConfig(cfg, cs.kubeconfigFunc)
+		},
+		appendInitialMachineConfig,
+		func(cfg *ign3types.Config, _ *mcfgv1.MachineConfig) error { return appendCerts(cfg, []string{}, "") },
+		// Inject desired image into the MC
+		appendDesiredOSImage(desiredImage),
+		// Must be last
+		func(cfg *ign3types.Config, mc *mcfgv1.MachineConfig) error {
+			return appendEncapsulated(cfg, mc, cr.version)
+		},
+	}
+
 	for _, a := range appenders {
 		if err := a(&ignConf, mc); err != nil {
 			return nil, err
@@ -224,3 +264,116 @@ func kubeconfigFromSecret(secretDir, apiserverURL string, caData []byte) ([]byte
 	}
 	return kcData, caData, nil
 }
+
+// Finds the MOSC that targets this MCO and verfies that the MOSC has an image in status
+// locates the matching MOSB for the MCP's current or next rendered MC and confirms build succeeded
+// and returns the image pullspec when its ready
+func (cs *clusterServer) resolveDesiredImageForPool(pool *mcfgv1.MachineConfigPool) string {
+	// If listers are not initialized (e.g., in tests or clusters without layering), return empty
+	if cs.machineOSConfigLister == nil || cs.machineOSBuildLister == nil {
+		return ""
+	}
+
+	// Find MachineOSConfig for this pool
+	moscList, err := cs.machineOSConfigLister.List(labels.Everything())
+	if err != nil {
+		// MOSC resources not available
+		klog.Infof("Could not list MachineOSConfigs for pool %s: %v", pool.Name, err)
+		return ""
+	}
+
+	// TODO(dkhater): Simplify to directly get MOSC using pool name with admin_ack enforcing MOSC name == pool name.
+	// Versions 4.18.21-4.18.23 lack OCPBUGS-60904 validation (MOSCs could have non-matching names).
+	// Can use admin_ack (e.g., at 4.23) to block upgrades until MOSCs are corrected, then replace List+filter with: mosc, err := cs.machineOSConfigLister.Get(pool.Name)
+	// See: https://issues.redhat.com/browse/OCPBUGS-60904 for validation bug.
+	// See: https://issues.redhat.com/browse/MCO-1935 for cleanup story.
+
+	var mosc *mcfgv1.MachineOSConfig
+	for _, config := range moscList {
+		if config.Spec.MachineConfigPool.Name == pool.Name {
+			mosc = config
+			break
+		}
+	}
+
+	// No MOSC for this pool - not layered
+	if mosc == nil {
+		return ""
+	}
+
+	// Check if image is ready in MOSC status
+	moscState := ctrlcommon.NewMachineOSConfigState(mosc)
+	if !moscState.HasOSImage() {
+		klog.Infof("Pool %s has MachineOSConfig but image not ready yet", pool.Name)
+		return ""
+	}
+
+	// Also verify the corresponding MOSB is successful to ensure we don't serve an image that hasn't been built yet
+	mosbList, err := cs.machineOSBuildLister.List(labels.Everything())
+	if err != nil {
+		klog.Infof("Could not list MachineOSBuilds for pool %s: %v", pool.Name, err)
+		return ""
+	}
+
+	var currentConf string
+	if pool.Status.UpdatedMachineCount > 0 {
+		currentConf = pool.Spec.Configuration.Name
+	} else {
+		currentConf = pool.Status.Configuration.Name
+	}
+
+	var mosb *mcfgv1.MachineOSBuild
+	for _, build := range mosbList {
+		if build.Spec.MachineOSConfig.Name == mosc.Name &&
+			build.Spec.MachineConfig.Name == currentConf {
+			mosb = build
+			break
+		}
+	}
+
+	if mosb == nil {
+		klog.Infof("Pool %s has MachineOSConfig but no matching MachineOSBuild for MC %s", pool.Name, currentConf)
+		return ""
+	}
+
+	// Check build is successful
+	mosbState := ctrlcommon.NewMachineOSBuildState(mosb)
+	if !mosbState.IsBuildSuccess() {
+		klog.Infof("Pool %s has MachineOSBuild but build not successful yet", pool.Name)
+		return ""
+	}
+
+	imageSpec := moscState.GetOSImage()
+
+	// Don't serve internal registry images during bootstrap because cluster DNS is not available yet
+	// New nodes will bootstrap with base image, then update to layered image after joining cluster (2 reboots)
+	// External registries (Quay, etc.) will still get the 1-reboot optimization
+	isInternal, err := ctrlcommon.IsOpenShiftRegistry(context.TODO(), imageSpec, cs.kubeclient, cs.routeclient)
+	if err != nil {
+		klog.Errorf("Failed to check if image is internal registry for pool %s: %v", pool.Name, err)
+		return ""
+	}
+
+	if isInternal {
+		klog.V(4).Infof("New nodes will bootstrap with base image, then update to layered image after joining cluster")
+		return ""
+	}
+
+	klog.Infof("Resolved layered image for pool %s: %s", pool.Name, imageSpec)
+	return imageSpec
+}
+
+// appendDesiredOSImage mutates the MC to include the desired OS image.
+// This runs appendEncapsulated so mcd-firstboot sees the image on first boot.
+func appendDesiredOSImage(desired string) appenderFunc {
+	return func(_ *ign3types.Config, mc *mcfgv1.MachineConfig) error {
+		if desired == "" {
+			return nil
+		}
+		if mc.Spec.OSImageURL != desired {
+			klog.Infof("overriding MC OSImageURL: %q -> %q", mc.Spec.OSImageURL, desired)
+			mc.Spec.OSImageURL = desired
+		}
+		return nil
+	}
+}