From f02ab0336c8a80818814d5fb11f78796dd696ac4 Mon Sep 17 00:00:00 2001 From: Urvashi Date: Thu, 26 Feb 2026 15:52:57 -0500 Subject: [PATCH] Implement osImageStream inheritance for custom MCPs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change allows custom MachineConfigPools (pools other than master, worker, or arbiter) to inherit the osImageStream setting from the worker pool when not explicitly set, aligning with the existing inheritance model for custom MCPs. Inheritance priority: 1. If custom MCP explicitly sets spec.osImageStream.name, use that value 2. If not set, inherit from worker pool's spec.osImageStream.name 3. If worker pool also doesn't set it, use OSImageStream.status.defaultStream The implementation includes: - GetEffectiveOSImageStreamName() helper in common/helpers.go for shared inheritance logic - Modified render controller to use inheritance when determining OS images for rendered MachineConfigs - Bootstrap scenario support with static helper function - Automatic re-rendering of custom pools when worker pool's osImageStream changes - Comprehensive unit tests for all inheritance scenarios Note: This change only affects the render controller (spec → osImageURL). Status field population is handled by PR #5689, which automatically reflects the inherited stream by matching deployed osImageURLs against available streams. Signed-off-by: Urvashi --- pkg/controller/common/helpers.go | 41 +++ pkg/controller/common/helpers_test.go | 91 ++++++ pkg/controller/render/render_controller.go | 90 +++++- .../render/render_controller_test.go | 295 ++++++++++++++++++ pkg/operator/sync.go | 7 +- 5 files changed, 521 insertions(+), 3 deletions(-) diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index 54bb1a4305..247b158210 100644 --- a/pkg/controller/common/helpers.go +++ b/pkg/controller/common/helpers.go @@ -49,6 +49,7 @@ import ( opv1 "github.com/openshift/api/operator/v1" mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" "github.com/openshift/client-go/machineconfiguration/clientset/versioned/scheme" + mcfglistersv1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1" "github.com/openshift/library-go/pkg/crypto" buildconstants "github.com/openshift/machine-config-operator/pkg/controller/build/constants" ) @@ -1430,3 +1431,43 @@ func GetPreBuiltImageMachineConfigsFromList(mcs []*mcfgv1.MachineConfig) map[str return preBuiltMCs } + +// IsCustomPool returns true if the pool is a custom pool (not master, worker, or arbiter) +func IsCustomPool(pool *mcfgv1.MachineConfigPool) bool { + return pool.Name != MachineConfigPoolMaster && + pool.Name != MachineConfigPoolWorker && + pool.Name != MachineConfigPoolArbiter +} + +// GetEffectiveOSImageStreamName returns the effective osImageStream name for a pool, +// taking inheritance from the worker pool into account for custom pools. +// Returns: +// - The pool's explicit osImageStream.Name if set +// - For custom pools: the worker pool's osImageStream.Name if the custom pool has none +// - Empty string (which will resolve to the default stream) +// +// This function can return an error if the worker pool cannot be fetched. +func GetEffectiveOSImageStreamName(pool *mcfgv1.MachineConfigPool, mcpLister mcfglistersv1.MachineConfigPoolLister) (string, error) { + // If the pool explicitly sets an osImageStream, use it + if pool.Spec.OSImageStream.Name != "" { + return pool.Spec.OSImageStream.Name, nil + } + + // Only custom pools inherit from worker; standard pools use default directly + if !IsCustomPool(pool) { + return "", nil + } + + // For custom pools with no explicit stream, try to inherit from worker + workerPool, err := mcpLister.Get(MachineConfigPoolWorker) + if err != nil { + // If worker pool doesn't exist or can't be fetched, use default stream + if !kerr.IsNotFound(err) { + return "", fmt.Errorf("failed to get worker pool for osImageStream inheritance: %w", err) + } + return "", nil + } + + // Return worker pool's stream (which may be empty, indicating default) + return workerPool.Spec.OSImageStream.Name, nil +} diff --git a/pkg/controller/common/helpers_test.go b/pkg/controller/common/helpers_test.go index 6bb48c80a2..797fb19669 100644 --- a/pkg/controller/common/helpers_test.go +++ b/pkg/controller/common/helpers_test.go @@ -20,6 +20,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake" + informers "github.com/openshift/client-go/machineconfiguration/informers/externalversions" "github.com/openshift/machine-config-operator/pkg/controller/common/fixtures" "github.com/openshift/machine-config-operator/test/helpers" ) @@ -1830,3 +1832,92 @@ func assertExpectedNodes(t *testing.T, expected []string, actual []*corev1.Node) t.Helper() assert.Equal(t, expected, helpers.GetNamesFromNodes(actual)) } + +func TestGetEffectiveOSImageStreamName(t *testing.T) { + tests := []struct { + name string + pool *mcfgv1.MachineConfigPool + workerPool *mcfgv1.MachineConfigPool + expectedStream string + expectError bool + }{ + { + name: "custom pool with explicit stream", + pool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-10" + return p + }(), + workerPool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-9" + return p + }(), + expectedStream: "rhel-10", + expectError: false, + }, + { + name: "custom pool inherits from worker", + pool: helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, ""), + workerPool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-9" + return p + }(), + expectedStream: "rhel-9", + expectError: false, + }, + { + name: "custom pool without worker pool uses default", + pool: helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, ""), + workerPool: nil, + expectedStream: "", + expectError: false, + }, + { + name: "standard pool (worker) uses default", + pool: helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, ""), + workerPool: nil, + expectedStream: "", + expectError: false, + }, + { + name: "standard pool (master) uses default", + pool: helpers.NewMachineConfigPool("master", helpers.MasterSelector, nil, ""), + workerPool: nil, + expectedStream: "", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create fake client and informer + objects := []runtime.Object{} + if tt.workerPool != nil { + objects = append(objects, tt.workerPool) + } + + fakeClient := fake.NewSimpleClientset(objects...) + informerFactory := informers.NewSharedInformerFactory(fakeClient, 0) + mcpLister := informerFactory.Machineconfiguration().V1().MachineConfigPools().Lister() + + // Start informer and wait for cache sync + stopCh := make(chan struct{}) + defer close(stopCh) + informerFactory.Start(stopCh) + informerFactory.WaitForCacheSync(stopCh) + + // Call the function + streamName, err := GetEffectiveOSImageStreamName(tt.pool, mcpLister) + + // Check error + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expectedStream, streamName) + } + }) + } +} diff --git a/pkg/controller/render/render_controller.go b/pkg/controller/render/render_controller.go index 1d78e369ea..681e73402f 100644 --- a/pkg/controller/render/render_controller.go +++ b/pkg/controller/render/render_controller.go @@ -192,6 +192,14 @@ func (ctrl *Controller) updateMachineConfigPool(old, cur interface{}) { klog.V(4).Infof("Updating MachineConfigPool %s", oldPool.Name) ctrl.enqueueMachineConfigPool(curPool) + + // If the worker pool's osImageStream changed, enqueue all custom pools that might inherit from it + if curPool.Name == ctrlcommon.MachineConfigPoolWorker && + oldPool.Spec.OSImageStream.Name != curPool.Spec.OSImageStream.Name { + klog.V(4).Infof("Worker pool osImageStream changed from %q to %q, enqueuing custom pools", + oldPool.Spec.OSImageStream.Name, curPool.Spec.OSImageStream.Name) + ctrl.enqueueCustomPools() + } } func (ctrl *Controller) deleteMachineConfigPool(obj interface{}) { @@ -398,6 +406,23 @@ func (ctrl *Controller) enqueueDefault(pool *mcfgv1.MachineConfigPool) { ctrl.enqueueAfter(pool, renderDelay) } +// enqueueCustomPools enqueues all custom pools (pools that are not master, worker, or arbiter). +// This is used when the worker pool changes in a way that might affect custom pools that inherit from it. +func (ctrl *Controller) enqueueCustomPools() { + pools, err := ctrl.mcpLister.List(labels.Everything()) + if err != nil { + klog.Warningf("Failed to list pools when enqueuing custom pools: %v", err) + return + } + + for _, pool := range pools { + if ctrlcommon.IsCustomPool(pool) { + klog.V(4).Infof("Enqueuing custom pool %s due to worker pool change", pool.Name) + ctrl.enqueueMachineConfigPool(pool) + } + } +} + // worker runs a worker thread that just dequeues items, processes them, and marks them done. // It enforces that the syncHandler is never invoked concurrently with the same key. func (ctrl *Controller) worker() { @@ -565,17 +590,51 @@ func (ctrl *Controller) getRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, return generateAndValidateRenderedMachineConfig(currentMC, pool, configs, cc, &mcop.Spec.IrreconcilableValidationOverrides, osImageStreamSet) } +// getOSImageStreamNameForPool determines the OS image stream name for a pool, +// implementing inheritance from the worker pool for custom pools. +// Returns: +// - The pool's explicit osImageStream.Name if set +// - For custom pools: the worker pool's osImageStream.Name if the custom pool has none +// - Empty string (which will resolve to the default stream) +func (ctrl *Controller) getOSImageStreamNameForPool(pool *mcfgv1.MachineConfigPool) string { + streamName, err := ctrlcommon.GetEffectiveOSImageStreamName(pool, ctrl.mcpLister) + if err != nil { + klog.V(2).Infof("Failed to get effective osImageStream for pool %s: %v; using default stream", pool.Name, err) + return "" + } + + // Log what stream is being used + if streamName != "" { + if pool.Spec.OSImageStream.Name != "" { + klog.V(4).Infof("Pool %s using explicit osImageStream: %s", pool.Name, streamName) + } else { + klog.V(4).Infof("Custom pool %s inheriting osImageStream from worker: %s", pool.Name, streamName) + } + } else { + if ctrlcommon.IsCustomPool(pool) { + klog.V(4).Infof("Custom pool %s using default osImageStream (worker also uses default)", pool.Name) + } else { + klog.V(4).Infof("Standard pool %s using default osImageStream", pool.Name) + } + } + + return streamName +} + func (ctrl *Controller) getOSImageStreamForPool(pool *mcfgv1.MachineConfigPool) (*v1alpha1.OSImageStreamSet, error) { if !osimagestream.IsFeatureEnabled(ctrl.fgHandler) || ctrl.osImageStreamLister == nil { return nil, nil } + // Determine which stream name to use based on inheritance logic + streamName := ctrl.getOSImageStreamNameForPool(pool) + imageStream, err := ctrl.osImageStreamLister.Get(ctrlcommon.ClusterInstanceNameOSImageStream) if err != nil { return nil, fmt.Errorf("could not get OSImageStream for pool %s: %w", pool.Name, err) } - imageStreamSet, err := osimagestream.GetOSImageStreamSetByName(imageStream, pool.Spec.OSImageStream.Name) + imageStreamSet, err := osimagestream.GetOSImageStreamSetByName(imageStream, streamName) if err != nil { return nil, fmt.Errorf("could not get OSImageStreamSet for pool %s: %w", pool.Name, err) } @@ -778,6 +837,31 @@ func generateAndValidateRenderedMachineConfig( return generated, nil } +// getOSImageStreamNameForPoolBootstrap implements the same inheritance logic as +// getOSImageStreamNameForPool but for the bootstrap scenario where we only have +// a slice of pools, not a lister. +func getOSImageStreamNameForPoolBootstrap(pool *mcfgv1.MachineConfigPool, pools []*mcfgv1.MachineConfigPool) string { + // If the pool explicitly sets an osImageStream, use it + if pool.Spec.OSImageStream.Name != "" { + return pool.Spec.OSImageStream.Name + } + + // Only custom pools inherit from worker + if !ctrlcommon.IsCustomPool(pool) { + return "" + } + + // Find worker pool in the provided slice + for _, p := range pools { + if p.Name == ctrlcommon.MachineConfigPoolWorker { + return p.Spec.OSImageStream.Name + } + } + + // Worker not found, use default + return "" +} + // RunBootstrap runs the render controller in bootstrap mode. // For each pool, it matches the machineconfigs based on label selector and // returns the generated machineconfigs and pool with CurrentMachineConfig status field set. @@ -793,7 +877,9 @@ func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineCo } var osImageStreamSet *v1alpha1.OSImageStreamSet if osImageStream != nil { - osImageStreamSet, err = osimagestream.GetOSImageStreamSetByName(osImageStream, pool.Spec.OSImageStream.Name) + // Determine which stream name to use based on inheritance logic + streamName := getOSImageStreamNameForPoolBootstrap(pool, pools) + osImageStreamSet, err = osimagestream.GetOSImageStreamSetByName(osImageStream, streamName) if err != nil { return nil, nil, fmt.Errorf("couldn't get the OSImageStream for pool %s %w", pool.Name, err) } diff --git a/pkg/controller/render/render_controller_test.go b/pkg/controller/render/render_controller_test.go index cc8f96b3ba..cc8277ca82 100644 --- a/pkg/controller/render/render_controller_test.go +++ b/pkg/controller/render/render_controller_test.go @@ -613,3 +613,298 @@ func TestGenerateMachineConfigValidation(t *testing.T) { assert.Error(t, err) assert.Nil(t, gmc) } + +func TestIsCustomPool(t *testing.T) { + tests := []struct { + name string + poolName string + expected bool + }{ + { + name: "master pool is not custom", + poolName: "master", + expected: false, + }, + { + name: "worker pool is not custom", + poolName: "worker", + expected: false, + }, + { + name: "arbiter pool is not custom", + poolName: "arbiter", + expected: false, + }, + { + name: "infra pool is custom", + poolName: "infra", + expected: true, + }, + { + name: "custom-role pool is custom", + poolName: "custom-role", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pool := helpers.NewMachineConfigPool(tt.poolName, helpers.WorkerSelector, nil, "") + result := ctrlcommon.IsCustomPool(pool) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetOSImageStreamNameForPool(t *testing.T) { + tests := []struct { + name string + pool *mcfgv1.MachineConfigPool + workerPool *mcfgv1.MachineConfigPool + expectedStream string + }{ + { + name: "custom pool with explicit stream uses explicit stream", + pool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-10" + return p + }(), + workerPool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-9" + return p + }(), + expectedStream: "rhel-10", + }, + { + name: "custom pool without stream inherits from worker", + pool: helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, ""), + workerPool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-9" + return p + }(), + expectedStream: "rhel-9", + }, + { + name: "custom pool without stream, worker has no stream, uses default", + pool: helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, ""), + workerPool: helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, ""), + expectedStream: "", + }, + { + name: "custom pool without stream, worker doesn't exist, uses default", + pool: helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, ""), + workerPool: nil, + expectedStream: "", + }, + { + name: "standard pool (master) with explicit stream uses explicit stream", + pool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("master", helpers.MasterSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-10" + return p + }(), + workerPool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-9" + return p + }(), + expectedStream: "rhel-10", + }, + { + name: "standard pool (worker) without stream uses default", + pool: helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, ""), + workerPool: nil, + expectedStream: "", + }, + { + name: "standard pool (arbiter) without stream uses default", + pool: helpers.NewMachineConfigPool("arbiter", &metav1.LabelSelector{ + MatchLabels: map[string]string{"node-role/arbiter": ""}, + }, nil, ""), + workerPool: nil, + expectedStream: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := newFixture(t) + + // Add worker pool to lister if provided + if tt.workerPool != nil { + f.mcpLister = append(f.mcpLister, tt.workerPool) + f.objects = append(f.objects, tt.workerPool) + } + + c := f.newController() + + result := c.getOSImageStreamNameForPool(tt.pool) + assert.Equal(t, tt.expectedStream, result, "Expected stream name %q, got %q", tt.expectedStream, result) + }) + } +} + +func TestGetOSImageStreamNameForPoolBootstrap(t *testing.T) { + tests := []struct { + name string + pool *mcfgv1.MachineConfigPool + pools []*mcfgv1.MachineConfigPool + expectedStream string + }{ + { + name: "custom pool with explicit stream uses explicit stream", + pool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-10" + return p + }(), + pools: []*mcfgv1.MachineConfigPool{ + func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-9" + return p + }(), + }, + expectedStream: "rhel-10", + }, + { + name: "custom pool without stream inherits from worker", + pool: helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, ""), + pools: []*mcfgv1.MachineConfigPool{ + helpers.NewMachineConfigPool("master", helpers.MasterSelector, nil, ""), + func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-9" + return p + }(), + }, + expectedStream: "rhel-9", + }, + { + name: "custom pool without stream, worker has no stream, uses default", + pool: helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, ""), + pools: []*mcfgv1.MachineConfigPool{ + helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, ""), + }, + expectedStream: "", + }, + { + name: "custom pool without stream, no worker pool exists, uses default", + pool: helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, ""), + pools: []*mcfgv1.MachineConfigPool{ + helpers.NewMachineConfigPool("master", helpers.MasterSelector, nil, ""), + }, + expectedStream: "", + }, + { + name: "standard pool (master) with explicit stream uses explicit stream", + pool: func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("master", helpers.MasterSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-10" + return p + }(), + pools: []*mcfgv1.MachineConfigPool{ + func() *mcfgv1.MachineConfigPool { + p := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + p.Spec.OSImageStream.Name = "rhel-9" + return p + }(), + }, + expectedStream: "rhel-10", + }, + { + name: "standard pool (worker) without stream uses default", + pool: helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, ""), + pools: []*mcfgv1.MachineConfigPool{ + helpers.NewMachineConfigPool("master", helpers.MasterSelector, nil, ""), + }, + expectedStream: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getOSImageStreamNameForPoolBootstrap(tt.pool, tt.pools) + assert.Equal(t, tt.expectedStream, result, "Expected stream name %q, got %q", tt.expectedStream, result) + }) + } +} + +func TestWorkerPoolOSImageStreamChangeEnqueuesCustomPools(t *testing.T) { + f := newFixture(t) + + // Create worker pool with osImageStream + workerPool := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + workerPool.Spec.OSImageStream.Name = "rhel-9" + + // Create custom pools + infraPool := helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, "") + customPool := helpers.NewMachineConfigPool("custom", helpers.InfraSelector, nil, "") + + // Add pools to lister + f.mcpLister = append(f.mcpLister, workerPool, infraPool, customPool) + f.objects = append(f.objects, workerPool, infraPool, customPool) + + c := f.newController() + + // Track which pools get enqueued + enqueuedPools := []string{} + c.enqueueMachineConfigPool = func(pool *mcfgv1.MachineConfigPool) { + enqueuedPools = append(enqueuedPools, pool.Name) + } + + // Update worker pool with new osImageStream + oldWorkerPool := workerPool.DeepCopy() + newWorkerPool := workerPool.DeepCopy() + newWorkerPool.Spec.OSImageStream.Name = "rhel-10" + + c.updateMachineConfigPool(oldWorkerPool, newWorkerPool) + + // Verify worker pool was enqueued (normal behavior) + assert.Contains(t, enqueuedPools, "worker", "Worker pool should be enqueued") + + // Verify custom pools were also enqueued + assert.Contains(t, enqueuedPools, "infra", "Infra pool should be enqueued when worker osImageStream changes") + assert.Contains(t, enqueuedPools, "custom", "Custom pool should be enqueued when worker osImageStream changes") + + // Verify master pool is NOT enqueued (we only have worker and custom pools) + assert.NotContains(t, enqueuedPools, "master", "Master pool should not be enqueued") +} + +func TestWorkerPoolOtherChangeDoesNotEnqueueCustomPools(t *testing.T) { + f := newFixture(t) + + // Create worker pool + workerPool := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "") + workerPool.Spec.OSImageStream.Name = "rhel-9" + + // Create custom pool + infraPool := helpers.NewMachineConfigPool("infra", helpers.InfraSelector, nil, "") + + // Add pools to lister + f.mcpLister = append(f.mcpLister, workerPool, infraPool) + f.objects = append(f.objects, workerPool, infraPool) + + c := f.newController() + + // Track which pools get enqueued + enqueuedPools := []string{} + c.enqueueMachineConfigPool = func(pool *mcfgv1.MachineConfigPool) { + enqueuedPools = append(enqueuedPools, pool.Name) + } + + // Update worker pool with SAME osImageStream but different field (e.g., paused) + oldWorkerPool := workerPool.DeepCopy() + newWorkerPool := workerPool.DeepCopy() + newWorkerPool.Spec.Paused = true + + c.updateMachineConfigPool(oldWorkerPool, newWorkerPool) + + // Verify only worker pool was enqueued, NOT custom pools + assert.Contains(t, enqueuedPools, "worker", "Worker pool should be enqueued") + assert.NotContains(t, enqueuedPools, "infra", "Infra pool should NOT be enqueued when osImageStream unchanged") + assert.Len(t, enqueuedPools, 1, "Only worker pool should be enqueued") +} diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 5fa418d28e..56526810ef 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -1789,7 +1789,12 @@ func (optr *Operator) syncRequiredMachineConfigPools(config *renderConfig, co *c _, hasRequiredPoolLabel := pool.Labels[requiredForUpgradeMachineConfigPoolLabelKey] if hasRequiredPoolLabel { - opURL, _, err := optr.getOsImageURLs(optr.namespace, pool.Spec.OSImageStream.Name) + streamName, err := ctrlcommon.GetEffectiveOSImageStreamName(pool, optr.mcpLister) + if err != nil { + klog.Errorf("Error getting effective osImageStream name for pool %s: %q", pool.Name, err) + return false, nil + } + opURL, _, err := optr.getOsImageURLs(optr.namespace, streamName) if err != nil { klog.Errorf("Error getting OS images: %q", err) return false, nil