From 59c31ebe3f83841010fe2a9524023c9cf7f60928 Mon Sep 17 00:00:00 2001 From: aksjadha Date: Mon, 27 Apr 2026 12:31:12 +0530 Subject: [PATCH 1/3] OCPBUGS-83862: Generate autosizing-disabled machineconfig only for master and worker nodes on OCP 4.20 --- .../kubelet_config_autosizing.go | 18 +++- .../kubelet_config_autosizing_test.go | 93 ++++++++++++++++--- .../kubelet_config_controller.go | 2 +- 3 files changed, 95 insertions(+), 18 deletions(-) diff --git a/pkg/controller/kubelet-config/kubelet_config_autosizing.go b/pkg/controller/kubelet-config/kubelet_config_autosizing.go index 6544cd9b92..f0bf57b48f 100644 --- a/pkg/controller/kubelet-config/kubelet_config_autosizing.go +++ b/pkg/controller/kubelet-config/kubelet_config_autosizing.go @@ -27,7 +27,11 @@ SYSTEM_RESERVED_ES=1Gi ` ) -// ensureAutoSizingMachineConfigs ensures auto-sizing MachineConfigs exist for all MachineConfigPools +func isDefaultPool(poolName string) bool { + return poolName == "master" || poolName == "worker" +} + +// ensureAutoSizingMachineConfigs ensures auto-sizing MachineConfigs exist for the master and worker MachineConfigPools func (ctrl *Controller) ensureAutoSizingMachineConfigs(ctx context.Context) error { mcpPools, err := ctrl.mcpLister.List(labels.Everything()) if err != nil { @@ -35,6 +39,10 @@ func (ctrl *Controller) ensureAutoSizingMachineConfigs(ctx context.Context) erro } for _, pool := range mcpPools { + if !isDefaultPool(pool.Name) { + klog.V(4).Infof("Skipping auto-sizing MachineConfig for non-default pool %v", pool.Name) + continue + } if err := ctrl.createAutoSizingMCIfNeeded(ctx, pool); err != nil { return fmt.Errorf("could not ensure auto-sizing MachineConfig for pool %v: %w", pool.Name, err) } @@ -77,12 +85,16 @@ func (ctrl *Controller) createAutoSizingMCIfNeeded(ctx context.Context, pool *mc return nil } -// RunAutoSizingBootstrap generates auto-sizing MachineConfig objects for all mcpPools +// RunAutoSizingBootstrap generates auto-sizing MachineConfig objects for master and worker mcpPools func RunAutoSizingBootstrap(mcpPools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfig, error) { configs := make([]*mcfgv1.MachineConfig, 0, len(mcpPools)) - // Create auto-sizing MachineConfigs for each pool + // Create auto-sizing MachineConfigs only for master and worker pools for _, pool := range mcpPools { + if !isDefaultPool(pool.Name) { + klog.V(4).Infof("Skipping auto-sizing MachineConfig for non-default pool %v during bootstrap", pool.Name) + continue + } autoSizingMC, err := newAutoSizingMachineConfig(pool) if err != nil { return nil, err diff --git a/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go b/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go index b851b6cfa5..f2fbcd39b0 100644 --- a/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go @@ -211,7 +211,7 @@ func TestEnsureAutoSizingMachineConfigs(t *testing.T) { "should have created MC for master pool") }) - t.Run("handles pools with no existing MCs", func(t *testing.T) { + t.Run("skips custom pools", func(t *testing.T) { // Setup: Initialize test fixture with a custom pool f := newFixture(t) f.skipActionsValidation = true @@ -222,18 +222,51 @@ func TestEnsureAutoSizingMachineConfigs(t *testing.T) { ctrl := f.newController(nil) - // Execute: Ensure auto-sizing MC exists for the custom pool + // Execute: Ensure auto-sizing MCs - custom pool should be skipped ctx := context.Background() err := ctrl.ensureAutoSizingMachineConfigs(ctx) - require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed for custom pool") + require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed even with custom pool") - // Verify: Confirm a single MachineConfig was created for the custom pool + // Verify: No MachineConfigs should be created for custom pools mcList, err := ctrl.client.MachineconfigurationV1().MachineConfigs().List(ctx, metav1.ListOptions{}) require.NoError(t, err, "listing MachineConfigs should succeed") - require.Len(t, mcList.Items, 1, - "should have exactly one MachineConfig for the custom pool") - require.Equal(t, "50-custom-auto-sizing-disabled", mcList.Items[0].Name, - "MachineConfig name should be 50-custom-auto-sizing-disabled but got %s", mcList.Items[0].Name) + require.Len(t, mcList.Items, 0, + "should have no MachineConfigs since custom pools are skipped") + }) + + t.Run("creates MCs only for master and worker when mixed with custom pools", func(t *testing.T) { + // Setup: Initialize test fixture with worker, master, and custom pools + f := newFixture(t) + f.skipActionsValidation = true + + workerPool := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + masterPool := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + customPool := helpers.NewMachineConfigPool("infra", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role/infra", ""), "v0") + f.mcpLister = append(f.mcpLister, workerPool, masterPool, customPool) + + ctrl := f.newController(nil) + + // Execute: Ensure auto-sizing MCs exist + ctx := context.Background() + err := ctrl.ensureAutoSizingMachineConfigs(ctx) + require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed") + + // Verify: Only master and worker MCs should be created, not infra + mcList, err := ctrl.client.MachineconfigurationV1().MachineConfigs().List(ctx, metav1.ListOptions{}) + require.NoError(t, err, "listing MachineConfigs should succeed") + require.Len(t, mcList.Items, 2, + "should have exactly 2 MachineConfigs (worker and master only)") + + mcNames := make(map[string]bool) + for _, mc := range mcList.Items { + mcNames[mc.Name] = true + } + require.True(t, mcNames["50-worker-auto-sizing-disabled"], + "should have created MC for worker pool") + require.True(t, mcNames["50-master-auto-sizing-disabled"], + "should have created MC for master pool") + require.False(t, mcNames["50-infra-auto-sizing-disabled"], + "should NOT have created MC for infra pool") }) } @@ -291,20 +324,52 @@ func TestRunAutoSizingBootstrap(t *testing.T) { require.Len(t, mcs, 0, "should generate no MachineConfigs for empty pool list") }) - t.Run("handles single pool", func(t *testing.T) { + t.Run("skips custom pools", func(t *testing.T) { // Setup: Create a single custom pool customPool := helpers.NewMachineConfigPool("custom", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role/custom", ""), "v0") pools := []*mcfgv1.MachineConfigPool{customPool} - // Execute: Generate auto-sizing MC for a single pool + // Execute: Generate auto-sizing MCs - custom pool should be skipped mcs, err := RunAutoSizingBootstrap(pools) - require.NoError(t, err, "RunAutoSizingBootstrap should handle single pool") - require.Len(t, mcs, 1, "should generate exactly one MachineConfig for single pool") - require.Equal(t, "50-custom-auto-sizing-disabled", mcs[0].Name, - "MC name should be 50-custom-auto-sizing-disabled but got %s", mcs[0].Name) + require.NoError(t, err, "RunAutoSizingBootstrap should handle custom pool gracefully") + require.Len(t, mcs, 0, "should generate no MachineConfigs for custom pools") + }) + + t.Run("generates MCs only for master and worker when mixed with custom pools", func(t *testing.T) { + // Setup: Create worker, master, and custom pools + workerPool := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + masterPool := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + infraPool := helpers.NewMachineConfigPool("infra", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role/infra", ""), "v0") + pools := []*mcfgv1.MachineConfigPool{workerPool, masterPool, infraPool} + + // Execute: Generate auto-sizing MCs + mcs, err := RunAutoSizingBootstrap(pools) + require.NoError(t, err, "RunAutoSizingBootstrap should not return an error") + require.Len(t, mcs, 2, "should generate 2 MachineConfigs (worker and master only)") + + mcNames := make(map[string]bool) + for _, mc := range mcs { + mcNames[mc.Name] = true + } + require.True(t, mcNames["50-worker-auto-sizing-disabled"], + "should contain worker auto-sizing MC") + require.True(t, mcNames["50-master-auto-sizing-disabled"], + "should contain master auto-sizing MC") + require.False(t, mcNames["50-infra-auto-sizing-disabled"], + "should NOT contain infra auto-sizing MC") }) } +// TestIsDefaultPool validates the isDefaultPool helper function that determines +// which MachineConfigPools should receive auto-sizing MachineConfigs. +func TestIsDefaultPool(t *testing.T) { + require.True(t, isDefaultPool("master"), "master should be a default pool") + require.True(t, isDefaultPool("worker"), "worker should be a default pool") + require.False(t, isDefaultPool("custom"), "custom should not be a default pool") + require.False(t, isDefaultPool("infra"), "infra should not be a default pool") + require.False(t, isDefaultPool(""), "empty string should not be a default pool") +} + // TestAutoSizingConstants validates that critical auto-sizing constants have the expected values. // These constants define the file paths, naming patterns, and default content for auto-sizing // configurations. Changes to these values could break compatibility with existing clusters. diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 45695e6833..7df8b70e32 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -200,7 +200,7 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) { klog.Info("Starting MachineConfigController-KubeletConfigController") defer klog.Info("Shutting down MachineConfigController-KubeletConfigController") - // Ensure auto-sizing MachineConfigs exist for all pools + // Ensure auto-sizing MachineConfigs exist only for master and worker pools if err := ctrl.ensureAutoSizingMachineConfigs(context.TODO()); err != nil { klog.Errorf("Error ensuring auto-sizing MachineConfigs: %v", err) // Don't return - we want the controller to continue even if this fails From f772a2ccb13342c6d711bb0d3b4259c6524a29ed Mon Sep 17 00:00:00 2001 From: aksjadha Date: Mon, 27 Apr 2026 13:41:22 +0530 Subject: [PATCH 2/3] Update test cases --- .../kubelet_config_autosizing_test.go | 49 ++----------------- 1 file changed, 5 insertions(+), 44 deletions(-) diff --git a/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go b/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go index f2fbcd39b0..e14201800d 100644 --- a/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go @@ -211,47 +211,21 @@ func TestEnsureAutoSizingMachineConfigs(t *testing.T) { "should have created MC for master pool") }) - t.Run("skips custom pools", func(t *testing.T) { - // Setup: Initialize test fixture with a custom pool - f := newFixture(t) - f.skipActionsValidation = true - - // Setup: Create a custom machine config pool with specific selector - customPool := helpers.NewMachineConfigPool("custom", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role/custom", ""), "v0") - f.mcpLister = append(f.mcpLister, customPool) - - ctrl := f.newController(nil) - - // Execute: Ensure auto-sizing MCs - custom pool should be skipped - ctx := context.Background() - err := ctrl.ensureAutoSizingMachineConfigs(ctx) - require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed even with custom pool") - - // Verify: No MachineConfigs should be created for custom pools - mcList, err := ctrl.client.MachineconfigurationV1().MachineConfigs().List(ctx, metav1.ListOptions{}) - require.NoError(t, err, "listing MachineConfigs should succeed") - require.Len(t, mcList.Items, 0, - "should have no MachineConfigs since custom pools are skipped") - }) - - t.Run("creates MCs only for master and worker when mixed with custom pools", func(t *testing.T) { - // Setup: Initialize test fixture with worker, master, and custom pools + t.Run("skips custom pools and creates MCs only for master and worker", func(t *testing.T) { f := newFixture(t) f.skipActionsValidation = true workerPool := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") masterPool := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") - customPool := helpers.NewMachineConfigPool("infra", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role/infra", ""), "v0") - f.mcpLister = append(f.mcpLister, workerPool, masterPool, customPool) + infraPool := helpers.NewMachineConfigPool("infra", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role.kubernetes.io/infra", ""), "v0") + f.mcpLister = append(f.mcpLister, workerPool, masterPool, infraPool) ctrl := f.newController(nil) - // Execute: Ensure auto-sizing MCs exist ctx := context.Background() err := ctrl.ensureAutoSizingMachineConfigs(ctx) require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed") - // Verify: Only master and worker MCs should be created, not infra mcList, err := ctrl.client.MachineconfigurationV1().MachineConfigs().List(ctx, metav1.ListOptions{}) require.NoError(t, err, "listing MachineConfigs should succeed") require.Len(t, mcList.Items, 2, @@ -324,25 +298,12 @@ func TestRunAutoSizingBootstrap(t *testing.T) { require.Len(t, mcs, 0, "should generate no MachineConfigs for empty pool list") }) - t.Run("skips custom pools", func(t *testing.T) { - // Setup: Create a single custom pool - customPool := helpers.NewMachineConfigPool("custom", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role/custom", ""), "v0") - pools := []*mcfgv1.MachineConfigPool{customPool} - - // Execute: Generate auto-sizing MCs - custom pool should be skipped - mcs, err := RunAutoSizingBootstrap(pools) - require.NoError(t, err, "RunAutoSizingBootstrap should handle custom pool gracefully") - require.Len(t, mcs, 0, "should generate no MachineConfigs for custom pools") - }) - - t.Run("generates MCs only for master and worker when mixed with custom pools", func(t *testing.T) { - // Setup: Create worker, master, and custom pools + t.Run("skips custom pools and generates MCs only for master and worker", func(t *testing.T) { workerPool := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") masterPool := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") - infraPool := helpers.NewMachineConfigPool("infra", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role/infra", ""), "v0") + infraPool := helpers.NewMachineConfigPool("infra", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role.kubernetes.io/infra", ""), "v0") pools := []*mcfgv1.MachineConfigPool{workerPool, masterPool, infraPool} - // Execute: Generate auto-sizing MCs mcs, err := RunAutoSizingBootstrap(pools) require.NoError(t, err, "RunAutoSizingBootstrap should not return an error") require.Len(t, mcs, 2, "should generate 2 MachineConfigs (worker and master only)") From 1473263ef4a9fc32e6b2e8282b40fc7b0a10f036 Mon Sep 17 00:00:00 2001 From: aksjadha Date: Fri, 15 May 2026 17:39:58 +0530 Subject: [PATCH 3/3] Removed isDefaultPool helper function --- .../kubelet_config_autosizing.go | 25 +++------ .../kubelet_config_autosizing_test.go | 54 +++---------------- 2 files changed, 13 insertions(+), 66 deletions(-) diff --git a/pkg/controller/kubelet-config/kubelet_config_autosizing.go b/pkg/controller/kubelet-config/kubelet_config_autosizing.go index f0bf57b48f..6b5dfe518f 100644 --- a/pkg/controller/kubelet-config/kubelet_config_autosizing.go +++ b/pkg/controller/kubelet-config/kubelet_config_autosizing.go @@ -8,7 +8,6 @@ import ( mcfgv1 "github.com/openshift/api/machineconfiguration/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/util/retry" "k8s.io/klog/v2" @@ -27,24 +26,15 @@ SYSTEM_RESERVED_ES=1Gi ` ) -func isDefaultPool(poolName string) bool { - return poolName == "master" || poolName == "worker" -} - // ensureAutoSizingMachineConfigs ensures auto-sizing MachineConfigs exist for the master and worker MachineConfigPools func (ctrl *Controller) ensureAutoSizingMachineConfigs(ctx context.Context) error { - mcpPools, err := ctrl.mcpLister.List(labels.Everything()) - if err != nil { - return fmt.Errorf("could not list MachineConfigPools: %w", err) - } - - for _, pool := range mcpPools { - if !isDefaultPool(pool.Name) { - klog.V(4).Infof("Skipping auto-sizing MachineConfig for non-default pool %v", pool.Name) - continue + for _, poolName := range []string{ctrlcommon.MachineConfigPoolMaster, ctrlcommon.MachineConfigPoolWorker} { + pool, err := ctrl.mcpLister.Get(poolName) + if err != nil { + return fmt.Errorf("could not get MachineConfigPool %v: %w", poolName, err) } if err := ctrl.createAutoSizingMCIfNeeded(ctx, pool); err != nil { - return fmt.Errorf("could not ensure auto-sizing MachineConfig for pool %v: %w", pool.Name, err) + return fmt.Errorf("could not ensure auto-sizing MachineConfig for pool %v: %w", poolName, err) } } @@ -87,11 +77,10 @@ func (ctrl *Controller) createAutoSizingMCIfNeeded(ctx context.Context, pool *mc // RunAutoSizingBootstrap generates auto-sizing MachineConfig objects for master and worker mcpPools func RunAutoSizingBootstrap(mcpPools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfig, error) { - configs := make([]*mcfgv1.MachineConfig, 0, len(mcpPools)) + var configs []*mcfgv1.MachineConfig - // Create auto-sizing MachineConfigs only for master and worker pools for _, pool := range mcpPools { - if !isDefaultPool(pool.Name) { + if pool.Name != ctrlcommon.MachineConfigPoolMaster && pool.Name != ctrlcommon.MachineConfigPoolWorker { klog.V(4).Infof("Skipping auto-sizing MachineConfig for non-default pool %v during bootstrap", pool.Name) continue } diff --git a/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go b/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go index e14201800d..fc32719536 100644 --- a/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_autosizing_test.go @@ -173,25 +173,25 @@ func TestCreateAutoSizingMachineConfigIfNeeded(t *testing.T) { } // TestEnsureAutoSizingMachineConfigs verifies that the controller correctly ensures auto-sizing -// MachineConfigs exist for all machine config pools in the cluster. This tests the high-level -// orchestration function that processes multiple pools. +// MachineConfigs exist for the master and worker pools. This tests the high-level +// orchestration function that fetches and processes master and worker pools directly. func TestEnsureAutoSizingMachineConfigs(t *testing.T) { - t.Run("creates MCs for all pools", func(t *testing.T) { + t.Run("creates MCs for master and worker pools", func(t *testing.T) { // Setup: Initialize test fixture and disable action validation for simplicity f := newFixture(t) f.skipActionsValidation = true - // Setup: Create multiple machine config pools (worker and master) + // Setup: Create master and worker machine config pools workerPool := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") masterPool := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") f.mcpLister = append(f.mcpLister, workerPool, masterPool) ctrl := f.newController(nil) - // Execute: Ensure auto-sizing MCs exist for all pools + // Execute: Ensure auto-sizing MCs exist for master and worker ctx := context.Background() err := ctrl.ensureAutoSizingMachineConfigs(ctx) - require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed for multiple pools") + require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed") // Verify: Confirm MachineConfigs were created for both pools mcList, err := ctrl.client.MachineconfigurationV1().MachineConfigs().List(ctx, metav1.ListOptions{}) @@ -210,38 +210,6 @@ func TestEnsureAutoSizingMachineConfigs(t *testing.T) { require.True(t, mcNames["50-master-auto-sizing-disabled"], "should have created MC for master pool") }) - - t.Run("skips custom pools and creates MCs only for master and worker", func(t *testing.T) { - f := newFixture(t) - f.skipActionsValidation = true - - workerPool := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") - masterPool := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") - infraPool := helpers.NewMachineConfigPool("infra", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role.kubernetes.io/infra", ""), "v0") - f.mcpLister = append(f.mcpLister, workerPool, masterPool, infraPool) - - ctrl := f.newController(nil) - - ctx := context.Background() - err := ctrl.ensureAutoSizingMachineConfigs(ctx) - require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed") - - mcList, err := ctrl.client.MachineconfigurationV1().MachineConfigs().List(ctx, metav1.ListOptions{}) - require.NoError(t, err, "listing MachineConfigs should succeed") - require.Len(t, mcList.Items, 2, - "should have exactly 2 MachineConfigs (worker and master only)") - - mcNames := make(map[string]bool) - for _, mc := range mcList.Items { - mcNames[mc.Name] = true - } - require.True(t, mcNames["50-worker-auto-sizing-disabled"], - "should have created MC for worker pool") - require.True(t, mcNames["50-master-auto-sizing-disabled"], - "should have created MC for master pool") - require.False(t, mcNames["50-infra-auto-sizing-disabled"], - "should NOT have created MC for infra pool") - }) } // TestRunAutoSizingBootstrap validates the bootstrap function that generates auto-sizing MachineConfigs @@ -321,16 +289,6 @@ func TestRunAutoSizingBootstrap(t *testing.T) { }) } -// TestIsDefaultPool validates the isDefaultPool helper function that determines -// which MachineConfigPools should receive auto-sizing MachineConfigs. -func TestIsDefaultPool(t *testing.T) { - require.True(t, isDefaultPool("master"), "master should be a default pool") - require.True(t, isDefaultPool("worker"), "worker should be a default pool") - require.False(t, isDefaultPool("custom"), "custom should not be a default pool") - require.False(t, isDefaultPool("infra"), "infra should not be a default pool") - require.False(t, isDefaultPool(""), "empty string should not be a default pool") -} - // TestAutoSizingConstants validates that critical auto-sizing constants have the expected values. // These constants define the file paths, naming patterns, and default content for auto-sizing // configurations. Changes to these values could break compatibility with existing clusters.