Skip to content

OCPBUGS-83862: Generate autosizing-disabled machineconfig only for master and worker nodes on OCP 4.20 #5885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 3 commits into from
Jun 2, 2026
Merged

OCPBUGS-83862: Generate autosizing-disabled machineconfig only for master and worker nodes on OCP 4.20 #5885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
59c31eb
OCPBUGS-83862: Generate autosizing-disabled machineconfig only for ma…
aksjadha Apr 27, 2026
f772a2c
Update test cases
aksjadha Apr 27, 2026
1473263
Removed isDefaultPool helper function
aksjadha May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions pkg/controller/kubelet-config/kubelet_config_autosizing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -27,16 +26,15 @@ SYSTEM_RESERVED_ES=1Gi
`
)

// ensureAutoSizingMachineConfigs ensures auto-sizing MachineConfigs exist for all MachineConfigPools
// 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 {
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)
}
}

Expand Down Expand Up @@ -77,12 +75,15 @@ 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))
var configs []*mcfgv1.MachineConfig

@ngopalak-redhat ngopalak-redhat May 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this changed?

@aksjadha aksjadha May 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from make([]*mcfgv1.MachineConfig, 0, len(mcpPools)) to var configs []*mcfgv1.MachineConfig was made because the pre-allocated capacity is no longer accurate.

  • Before: Every pool produced a config, so len(mcpPools) was the exact capacity needed.
  • After: This change skips non-master/non-worker pools, so the actual number of configs will be less than or equal to len(mcpPools). Pre-allocating len(mcpPools) slots would over-allocate for any custom pools passed in.

The optimization of pre-allocating capacity is not required. A simple var configs []*mcfgv1.MachineConfig (nil slice, grown via append) avoids implying the slice.


// Create auto-sizing MachineConfigs for each pool
for _, pool := range mcpPools {
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
}
autoSizingMC, err := newAutoSizingMachineConfig(pool)
if err != nil {
return nil, err
Expand Down
64 changes: 24 additions & 40 deletions pkg/controller/kubelet-config/kubelet_config_autosizing_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand All @@ -210,31 +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("handles pools with no existing MCs", 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 MC exists for the custom pool
ctx := context.Background()
err := ctrl.ensureAutoSizingMachineConfigs(ctx)
require.NoError(t, err, "ensureAutoSizingMachineConfigs should succeed for custom pool")

// Verify: Confirm a single MachineConfig was created for the custom pool
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)
})
}

// TestRunAutoSizingBootstrap validates the bootstrap function that generates auto-sizing MachineConfigs
Expand Down Expand Up @@ -291,17 +266,26 @@ 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) {
// Setup: Create a single custom pool
customPool := helpers.NewMachineConfigPool("custom", nil, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "node-role/custom", ""), "v0")
pools := []*mcfgv1.MachineConfigPool{customPool}
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")
Comment thread
aksjadha marked this conversation as resolved.
masterPool := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "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 MC for a single pool
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 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")
})
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/kubelet-config/kubelet_config_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down