diff --git a/pkg/controller/kubelet-config/helpers.go b/pkg/controller/kubelet-config/helpers.go index 50c0c597ab..0bdb2e223f 100644 --- a/pkg/controller/kubelet-config/helpers.go +++ b/pkg/controller/kubelet-config/helpers.go @@ -20,6 +20,7 @@ import ( "k8s.io/apimachinery/pkg/util/yaml" "k8s.io/klog/v2" kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" + kubeletypes "k8s.io/kubernetes/pkg/kubelet/types" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" @@ -390,6 +391,12 @@ func validateUserKubeletConfig(cfg *mcfgv1.KubeletConfig) error { cfg.Spec.AutoSizingReserved != nil && *cfg.Spec.AutoSizingReserved { return fmt.Errorf("KubeletConfiguration: autoSizingReserved and systemdReserved cannot be set together") } + // Validate that systemReservedCgroup matches systemCgroups if both are set + if kcDecoded.SystemReservedCgroup != "" && kcDecoded.SystemCgroups != "" { + if kcDecoded.SystemReservedCgroup != kcDecoded.SystemCgroups { + return fmt.Errorf("KubeletConfiguration: systemReservedCgroup (%s) must match systemCgroups (%s)", kcDecoded.SystemReservedCgroup, kcDecoded.SystemCgroups) + } + } return nil } @@ -506,6 +513,36 @@ func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeCo if err != nil { return nil, nil, nil, fmt.Errorf("could not merge original config and new config: %w", err) } + + // Empty strings are ignored by mergo, so we need to set them to empty string for SystemReservedCgroup explicitly + // Else the old value is retained. Tested by test/e2e-2of2/kubeletcfg_test.go + // However, only clear it if the user explicitly sets enforceNodeAllocatable without system-reserved enforcement + if specKubeletConfig.SystemReservedCgroup == "" && len(specKubeletConfig.EnforceNodeAllocatable) > 0 { + // User explicitly set enforceNodeAllocatable - check if it contains system-reserved enforcement + hasSystemReservedEnforcement := false + for _, val := range specKubeletConfig.EnforceNodeAllocatable { + if val == kubeletypes.SystemReservedEnforcementKey || val == kubeletypes.SystemReservedCompressibleEnforcementKey { + hasSystemReservedEnforcement = true + break + } + } + + // Only clear systemReservedCgroup if user explicitly set enforceNodeAllocatable without system-reserved enforcement + // This prevents validation error: "systemReservedCgroup must be specified when system-reserved is in enforceNodeAllocatable" + if !hasSystemReservedEnforcement { + originalKubeConfig.SystemReservedCgroup = "" + } + } + } + + // Handle systemReservedCgroup and enforceNodeAllocatable based on: + // Check if reservedSystemCPUs is set (incompatible with systemReservedCgroup) + if originalKubeConfig.ReservedSystemCPUs != "" { + klog.Infof("reservedSystemCPUs is set to %s, disabling systemReservedCgroup enforcement", originalKubeConfig.ReservedSystemCPUs) + // Clear systemReservedCgroup + originalKubeConfig.SystemReservedCgroup = "" + // Set enforceNodeAllocatable to only pods (the default) + originalKubeConfig.EnforceNodeAllocatable = []string{kubeletypes.NodeAllocatableEnforcementKey} } // Encode the new config into an Ignition File diff --git a/pkg/controller/kubelet-config/helpers_test.go b/pkg/controller/kubelet-config/helpers_test.go new file mode 100644 index 0000000000..155391fac1 --- /dev/null +++ b/pkg/controller/kubelet-config/helpers_test.go @@ -0,0 +1,268 @@ +package kubeletconfig + +import ( + "testing" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" + kubeletypes "k8s.io/kubernetes/pkg/kubelet/types" + + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" +) + +// TestGenerateKubeletIgnFilesWithReservedSystemCPUs tests that when reservedSystemCPUs is set, +// the systemReservedCgroup is cleared and enforceNodeAllocatable is set to ["pods"] only. +func TestGenerateKubeletIgnFilesWithReservedSystemCPUs(t *testing.T) { + testCases := []struct { + name string + reservedSystemCPUs string + initialSystemReservedCgroup string + initialEnforceNodeAllocatable []string + expectedSystemReservedCgroup string + expectedEnforceNodeAllocatable []string + shouldDisableSystemReservedCgroup bool + }{ + { + name: "reservedSystemCPUs set - should disable systemReservedCgroup", + reservedSystemCPUs: "0-1", + initialSystemReservedCgroup: "/system.slice", + initialEnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey, kubeletypes.SystemReservedCompressibleEnforcementKey}, + expectedSystemReservedCgroup: "", + expectedEnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey}, + shouldDisableSystemReservedCgroup: true, + }, + { + name: "reservedSystemCPUs not set - should preserve systemReservedCgroup", + reservedSystemCPUs: "", + initialSystemReservedCgroup: "/system.slice", + initialEnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey, kubeletypes.SystemReservedCompressibleEnforcementKey}, + expectedSystemReservedCgroup: "/system.slice", + expectedEnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey, kubeletypes.SystemReservedCompressibleEnforcementKey}, + shouldDisableSystemReservedCgroup: false, + }, + { + name: "reservedSystemCPUs set with empty systemReservedCgroup", + reservedSystemCPUs: "0-3", + initialSystemReservedCgroup: "", + initialEnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey}, + expectedSystemReservedCgroup: "", + expectedEnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey}, + shouldDisableSystemReservedCgroup: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup: Create a base kubelet configuration with the initial values + originalKubeConfig := &kubeletconfigv1beta1.KubeletConfiguration{ + ReservedSystemCPUs: tc.reservedSystemCPUs, + SystemReservedCgroup: tc.initialSystemReservedCgroup, + EnforceNodeAllocatable: tc.initialEnforceNodeAllocatable, + } + + // Create a KubeletConfig CR (can be nil for this test) + kubeletConfig := &mcfgv1.KubeletConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kubelet-config", + }, + Spec: mcfgv1.KubeletConfigSpec{}, + } + + // Execute: Generate the kubelet ignition files + kubeletIgnition, _, _, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig) + require.NoError(t, err, "generateKubeletIgnFiles should not return an error") + require.NotNil(t, kubeletIgnition, "kubelet ignition file should not be nil") + + // Verify: Decode the generated kubelet configuration from the ignition file + contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletIgnition.Contents.Source, kubeletIgnition.Contents.Compression) + require.NoError(t, err, "decoding ignition file contents should succeed") + + // Parse the YAML contents back into a KubeletConfiguration + decodedConfig, err := DecodeKubeletConfig(contents) + require.NoError(t, err, "decoding kubelet config should succeed") + + // Verify: Check that systemReservedCgroup matches expected value + require.Equal(t, tc.expectedSystemReservedCgroup, decodedConfig.SystemReservedCgroup, + "systemReservedCgroup should be %q but got %q", tc.expectedSystemReservedCgroup, decodedConfig.SystemReservedCgroup) + + // Verify: Check that enforceNodeAllocatable matches expected value + require.Equal(t, tc.expectedEnforceNodeAllocatable, decodedConfig.EnforceNodeAllocatable, + "enforceNodeAllocatable should be %v but got %v", tc.expectedEnforceNodeAllocatable, decodedConfig.EnforceNodeAllocatable) + + // Verify: Check that reservedSystemCPUs is preserved + require.Equal(t, tc.reservedSystemCPUs, decodedConfig.ReservedSystemCPUs, + "reservedSystemCPUs should be %q but got %q", tc.reservedSystemCPUs, decodedConfig.ReservedSystemCPUs) + }) + } +} + +// TestGenerateKubeletIgnFilesWithKubeletConfigSpec tests that generateKubeletIgnFiles +// properly merges user-provided kubelet configuration with the original config. +func TestGenerateKubeletIgnFilesWithKubeletConfigSpec(t *testing.T) { + // Setup: Create a base kubelet configuration + originalKubeConfig := &kubeletconfigv1beta1.KubeletConfiguration{ + MaxPods: 110, + ReservedSystemCPUs: "0-1", + SystemReservedCgroup: "/system.slice", + EnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey, kubeletypes.SystemReservedCompressibleEnforcementKey}, + } + + // Setup: Create user-provided kubelet configuration with reservedSystemCPUs + userKubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{ + MaxPods: 250, + ReservedSystemCPUs: "0-3", // User wants to reserve more CPUs + } + + // Encode the user config + userKubeletConfigRaw, err := EncodeKubeletConfig(userKubeletConfig, kubeletconfigv1beta1.SchemeGroupVersion, runtime.ContentTypeYAML) + require.NoError(t, err) + + // Create a KubeletConfig CR with user config + kubeletConfig := &mcfgv1.KubeletConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kubelet-config", + }, + Spec: mcfgv1.KubeletConfigSpec{ + KubeletConfig: &runtime.RawExtension{ + Raw: userKubeletConfigRaw, + }, + }, + } + + // Execute: Generate the kubelet ignition files + kubeletIgnition, _, _, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig) + require.NoError(t, err, "generateKubeletIgnFiles should not return an error") + require.NotNil(t, kubeletIgnition, "kubelet ignition file should not be nil") + + // Verify: Decode the generated kubelet configuration from the ignition file + contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletIgnition.Contents.Source, kubeletIgnition.Contents.Compression) + require.NoError(t, err, "decoding ignition file contents should succeed") + + // Parse the YAML contents back into a KubeletConfiguration + decodedConfig, err := DecodeKubeletConfig(contents) + require.NoError(t, err, "decoding kubelet config should succeed") + + // Verify: Check that user config was merged (MaxPods should be from user config) + require.Equal(t, int32(250), decodedConfig.MaxPods, + "MaxPods should be 250 from user config but got %d", decodedConfig.MaxPods) + + // Verify: Check that reservedSystemCPUs was merged from user config + require.Equal(t, "0-3", decodedConfig.ReservedSystemCPUs, + "reservedSystemCPUs should be 0-3 from user config but got %q", decodedConfig.ReservedSystemCPUs) + + // Verify: Check that systemReservedCgroup was cleared (since reservedSystemCPUs is set) + require.Equal(t, "", decodedConfig.SystemReservedCgroup, + "systemReservedCgroup should be empty when reservedSystemCPUs is set but got %q", decodedConfig.SystemReservedCgroup) + + // Verify: Check that enforceNodeAllocatable was set to only ["pods"] + require.Equal(t, []string{kubeletypes.NodeAllocatableEnforcementKey}, decodedConfig.EnforceNodeAllocatable, + "enforceNodeAllocatable should be [pods] when reservedSystemCPUs is set but got %v", decodedConfig.EnforceNodeAllocatable) +} + +// TestGenerateKubeletIgnFilesWithEmptyStringOverride tests that when a user explicitly +// sets systemReservedCgroup to an empty string +func TestGenerateKubeletIgnFilesWithEmptyStringOverride(t *testing.T) { + originalKubeConfig := &kubeletconfigv1beta1.KubeletConfiguration{ + MaxPods: 110, + SystemReservedCgroup: "/system.slice", + EnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey, kubeletypes.SystemReservedCompressibleEnforcementKey}, + } + + userKubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{ + MaxPods: 100, + SystemReservedCgroup: "", // User explicitly wants to clear this + EnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey}, // User only wants pods enforcement + } + + userKubeletConfigRaw, err := EncodeKubeletConfig(userKubeletConfig, kubeletconfigv1beta1.SchemeGroupVersion, runtime.ContentTypeYAML) + require.NoError(t, err) + + kubeletConfig := &mcfgv1.KubeletConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-empty-override", + }, + Spec: mcfgv1.KubeletConfigSpec{ + KubeletConfig: &runtime.RawExtension{ + Raw: userKubeletConfigRaw, + }, + }, + } + + kubeletIgnition, _, _, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig) + require.NoError(t, err, "generateKubeletIgnFiles should not return an error") + require.NotNil(t, kubeletIgnition, "kubelet ignition file should not be nil") + + contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletIgnition.Contents.Source, kubeletIgnition.Contents.Compression) + require.NoError(t, err, "decoding ignition file contents should succeed") + + decodedConfig, err := DecodeKubeletConfig(contents) + require.NoError(t, err, "decoding kubelet config should succeed") + + require.Equal(t, int32(100), decodedConfig.MaxPods, + "MaxPods should be 100 from user config but got %d", decodedConfig.MaxPods) + + // Verify: Check that systemReservedCgroup was set to empty string (not ignored) + // This is the critical test - the empty string should override the default "/system.slice" + require.Equal(t, "", decodedConfig.SystemReservedCgroup, + "systemReservedCgroup should be empty string (overriding default) but got %q", decodedConfig.SystemReservedCgroup) + + require.Equal(t, []string{kubeletypes.NodeAllocatableEnforcementKey}, decodedConfig.EnforceNodeAllocatable, + "enforceNodeAllocatable should be [pods] from user config but got %v", decodedConfig.EnforceNodeAllocatable) +} + +// TestGenerateKubeletIgnFilesWithPartialUserConfig tests the bug scenario where: +// - Base config has systemReservedCgroup="/system.slice" and enforceNodeAllocatable with system-reserved-compressible +// - User provides custom config that doesn't mention systemReservedCgroup at all (e.g., only sets maxPods) +// - Expected: systemReservedCgroup should be preserved from base config (not cleared) +// This prevents validation error: "systemReservedCgroup must be specified when system-reserved is in enforceNodeAllocatable" +func TestGenerateKubeletIgnFilesWithPartialUserConfig(t *testing.T) { + originalKubeConfig := &kubeletconfigv1beta1.KubeletConfiguration{ + MaxPods: 110, + SystemReservedCgroup: "/system.slice", + EnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey, kubeletypes.SystemReservedCompressibleEnforcementKey}, + } + + // User only sets maxPods, doesn't mention systemReservedCgroup or enforceNodeAllocatable + userKubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{ + MaxPods: 200, + } + + userKubeletConfigRaw, err := EncodeKubeletConfig(userKubeletConfig, kubeletconfigv1beta1.SchemeGroupVersion, runtime.ContentTypeYAML) + require.NoError(t, err) + + kubeletConfig := &mcfgv1.KubeletConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-partial-config", + }, + Spec: mcfgv1.KubeletConfigSpec{ + KubeletConfig: &runtime.RawExtension{ + Raw: userKubeletConfigRaw, + }, + }, + } + + kubeletIgnition, _, _, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig) + require.NoError(t, err, "generateKubeletIgnFiles should not return an error") + require.NotNil(t, kubeletIgnition, "kubelet ignition file should not be nil") + + contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletIgnition.Contents.Source, kubeletIgnition.Contents.Compression) + require.NoError(t, err, "decoding ignition file contents should succeed") + + decodedConfig, err := DecodeKubeletConfig(contents) + require.NoError(t, err, "decoding kubelet config should succeed") + + // Verify: maxPods was updated + require.Equal(t, int32(200), decodedConfig.MaxPods, + "MaxPods should be 200 from user config but got %d", decodedConfig.MaxPods) + + // Verify: systemReservedCgroup was preserved from base config (not cleared) + require.Equal(t, "/system.slice", decodedConfig.SystemReservedCgroup, + "systemReservedCgroup should be preserved as /system.slice from base config but got %q", decodedConfig.SystemReservedCgroup) + + // Verify: enforceNodeAllocatable was preserved from base config + require.Equal(t, []string{kubeletypes.NodeAllocatableEnforcementKey, kubeletypes.SystemReservedCompressibleEnforcementKey}, decodedConfig.EnforceNodeAllocatable, + "enforceNodeAllocatable should be preserved from base config but got %v", decodedConfig.EnforceNodeAllocatable) +} diff --git a/templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml b/templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml index e536c6f63f..13813e7f6c 100644 --- a/templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml +++ b/templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml @@ -28,6 +28,10 @@ contents: memorySwap: swapBehavior: NoSwap systemCgroups: /system.slice + systemReservedCgroup: /system.slice + enforceNodeAllocatable: + - pods + - system-reserved-compressible nodeStatusUpdateFrequency: 10s nodeStatusReportFrequency: 5m serverTLSBootstrap: true diff --git a/templates/master/01-master-kubelet/_base/files/kubelet.yaml b/templates/master/01-master-kubelet/_base/files/kubelet.yaml index e536c6f63f..13813e7f6c 100644 --- a/templates/master/01-master-kubelet/_base/files/kubelet.yaml +++ b/templates/master/01-master-kubelet/_base/files/kubelet.yaml @@ -28,6 +28,10 @@ contents: memorySwap: swapBehavior: NoSwap systemCgroups: /system.slice + systemReservedCgroup: /system.slice + enforceNodeAllocatable: + - pods + - system-reserved-compressible nodeStatusUpdateFrequency: 10s nodeStatusReportFrequency: 5m serverTLSBootstrap: true diff --git a/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml b/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml index d7d6bd556e..a67b686070 100644 --- a/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml +++ b/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml @@ -28,6 +28,10 @@ contents: memorySwap: swapBehavior: NoSwap systemCgroups: /system.slice + systemReservedCgroup: /system.slice + enforceNodeAllocatable: + - pods + - system-reserved-compressible nodeStatusUpdateFrequency: 10s nodeStatusReportFrequency: 5m serverTLSBootstrap: true diff --git a/test/e2e-2of2/kubeletcfg_test.go b/test/e2e-2of2/kubeletcfg_test.go index 2cd2f73995..279bbd603a 100644 --- a/test/e2e-2of2/kubeletcfg_test.go +++ b/test/e2e-2of2/kubeletcfg_test.go @@ -20,6 +20,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" + kubeletypes "k8s.io/kubernetes/pkg/kubelet/types" ) const ( @@ -48,9 +49,16 @@ func TestKubeletConfigDefaultUpdateFreq(t *testing.T) { runTestWithKubeletCfg(t, "resources", []string{`"?nodeStatusUpdateFrequency"?: (\S+)`}, []string{"nodeStatusUpdateFrequency"}, [][]string{{"10s"}}, kc1, nil) } func TestKubeletConfigMaxPods(t *testing.T) { - kcRaw1, err := kcfg.EncodeKubeletConfig(&kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, kubeletconfigv1beta1.SchemeGroupVersion, runtime.ContentTypeJSON) - require.Nil(t, err, "failed to encode kubelet config") autoNodeSizing := true + + // kc1: AutoSizingReserved disabled - systemReservedCgroup: "", enforceNodeAllocatable: [pods] + kcRaw1, err := kcfg.EncodeKubeletConfig(&kubeletconfigv1beta1.KubeletConfiguration{ + MaxPods: 100, + SystemReservedCgroup: "", + EnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey}, + }, kubeletconfigv1beta1.SchemeGroupVersion, runtime.ContentTypeJSON) + require.Nil(t, err, "failed to encode kubelet config") + kc1 := &mcfgv1.KubeletConfig{ ObjectMeta: metav1.ObjectMeta{Name: "test-101"}, Spec: mcfgv1.KubeletConfigSpec{ @@ -60,8 +68,15 @@ func TestKubeletConfigMaxPods(t *testing.T) { }, }, } - kcRaw2, err := kcfg.EncodeKubeletConfig(&kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 200}, kubeletconfigv1beta1.SchemeGroupVersion, runtime.ContentTypeJSON) + + // kc2: AutoSizingReserved enabled - systemReservedCgroup: /system.slice, enforceNodeAllocatable: [pods, system-reserved-compressible] + kcRaw2, err := kcfg.EncodeKubeletConfig(&kubeletconfigv1beta1.KubeletConfiguration{ + MaxPods: 200, + SystemReservedCgroup: "/system.slice", + EnforceNodeAllocatable: []string{kubeletypes.NodeAllocatableEnforcementKey, kubeletypes.SystemReservedCompressibleEnforcementKey}, + }, kubeletconfigv1beta1.SchemeGroupVersion, runtime.ContentTypeJSON) require.Nil(t, err, "failed to encode kubelet config") + kc2 := &mcfgv1.KubeletConfig{ ObjectMeta: metav1.ObjectMeta{Name: "test-200"}, Spec: mcfgv1.KubeletConfigSpec{ @@ -71,7 +86,11 @@ func TestKubeletConfigMaxPods(t *testing.T) { }, } - runTestWithKubeletCfg(t, "max-pods", []string{`"?maxPods"?: (\S+)`}, []string{"maxPods"}, [][]string{{"100", "200"}}, kc1, kc2) + runTestWithKubeletCfg(t, "max-pods", + []string{`"?maxPods"?: (\S+)`, `"?systemReservedCgroup"?: "?([^",\n]*)"?`, `"?enforceNodeAllocatable"?:`}, + []string{"maxPods", "systemReservedCgroup", "enforceNodeAllocatable"}, + [][]string{{"100", "200"}, {"", "/system.slice"}, {"pods", "system-reserved-compressible"}}, + kc1, kc2) } // runTestWithKubeletCfg creates a kubelet config and checks whether the expected updates were applied, then deletes the kubelet config and makes @@ -117,12 +136,26 @@ func runTestWithKubeletCfg(t *testing.T, testName string, regexKey []string, str // the kubelet.conf format is yaml when in the default state and becomes a json when we apply a kubelet config CR defaultConfVals := []string{} for i, val := range regexKey { - if strings.Contains(val, "systemReserved") { + if strings.Contains(val, "systemReserved") && stringKey[i] != "systemReservedCgroup" && stringKey[i] != "enforceNodeAllocatable" { defaultConfVals = append(defaultConfVals, "") continue } out, _ := getValueFromKubeletConfig(t, cs, node, val, stringKey[i], kubeletPath) defaultConfVals = append(defaultConfVals, out) + + // Verify default state for systemReservedCgroup and enforceNodeAllocatable before applying kc1 + if stringKey[i] == "systemReservedCgroup" { + require.True(t, strings.Contains(out, "/system.slice"), "default systemReservedCgroup should be '/system.slice'") + t.Logf("Verified default systemReservedCgroup: /system.slice") + continue // Skip early exit check for this field since we're testing disable/re-enable + } + if stringKey[i] == "enforceNodeAllocatable" { + require.True(t, strings.Contains(out, "pods"), "default enforceNodeAllocatable should contain 'pods'") + require.True(t, strings.Contains(out, "system-reserved-compressible"), "default enforceNodeAllocatable should contain 'system-reserved-compressible'") + t.Logf("Verified default enforceNodeAllocatable contains: pods, system-reserved-compressible") + continue // Skip early exit check for this field since we're testing disable/re-enable + } + for _, expect := range expectedConfVals[i] { if defaultConfVals[i] == expect { t.Logf("default configuration value %s same as values being tested against. Consider updating the test", defaultConfVals[i]) @@ -159,6 +192,10 @@ func runTestWithKubeletCfg(t *testing.T, testName string, regexKey []string, str } else { // sometimes it seems regexp does not work here require.True(t, strings.Contains(out, expectedConfVals[i][0])) } + // Additional validation for enforceNodeAllocatable to ensure "pods" is present + if stringKey[i] == "enforceNodeAllocatable" { + require.True(t, strings.Contains(out, "pods"), "enforceNodeAllocatable should contain 'pods'") + } } // Get the new node object which should reflect new values for the allocatables @@ -179,7 +216,7 @@ func runTestWithKubeletCfg(t *testing.T, testName string, regexKey []string, str require.Nil(t, err, "failed to render machine config from second container runtime config") // ensure the second kubelet config update rolls out to the pool helpers.WaitForConfigAndPoolComplete(t, cs, poolName, kcMCName2) - // verify value was changed to match that of the first kubelet config + // verify value was changed to match that of the second kubelet config for i, val := range regexKey { out, found := getValueFromKubeletConfig(t, cs, node, val, stringKey[i], kubeletPath) if found { @@ -187,6 +224,11 @@ func runTestWithKubeletCfg(t *testing.T, testName string, regexKey []string, str } else { // sometimes it seems regexp does not work here require.True(t, strings.Contains(out, expectedConfVals[i][1])) } + // Additional validation for enforceNodeAllocatable to ensure both "pods" and "system-reserved-compressible" are present + if stringKey[i] == "enforceNodeAllocatable" && kc2.Spec.AutoSizingReserved != nil && *kc2.Spec.AutoSizingReserved { + require.True(t, strings.Contains(out, "pods"), "enforceNodeAllocatable should contain 'pods'") + require.True(t, strings.Contains(out, "system-reserved-compressible"), "enforceNodeAllocatable should contain 'system-reserved-compressible'") + } } // cleanup the second kubelet config and make sure it doesn't error @@ -204,6 +246,10 @@ func runTestWithKubeletCfg(t *testing.T, testName string, regexKey []string, str } else { // sometimes it seems regexp does not work here require.True(t, strings.Contains(out, expectedConfVals[i][0])) } + // Additional validation for enforceNodeAllocatable to ensure "pods" is present + if stringKey[i] == "enforceNodeAllocatable" { + require.True(t, strings.Contains(out, "pods"), "enforceNodeAllocatable should contain 'pods'") + } } // cleanup the first kubelet config and make sure it doesn't error @@ -215,6 +261,16 @@ func runTestWithKubeletCfg(t *testing.T, testName string, regexKey []string, str // verify that the config value rolled back to the default value for i, val := range regexKey { out, found := getValueFromKubeletConfig(t, cs, node, val, stringKey[i], kubeletPath) + // Verify default state has system-reserved-compressible enabled + if stringKey[i] == "enforceNodeAllocatable" { + require.True(t, strings.Contains(out, "pods"), "enforceNodeAllocatable should contain 'pods' in default state") + require.True(t, strings.Contains(out, "system-reserved-compressible"), "enforceNodeAllocatable should contain 'system-reserved-compressible' in default state") + continue + } + if stringKey[i] == "systemReservedCgroup" { + require.True(t, strings.Contains(out, "/system.slice"), "systemReservedCgroup should be '/system.slice' in default state") + continue + } if found { require.Equal(t, out, defaultConfVals[i], "value in kubelet config not updated as expected") } else { // sometimes it seems regexp does not work here @@ -298,6 +354,13 @@ func getValueFromKubeletConfig(t *testing.T, cs *framework.ClientSet, node corev if len(matches) != 2 && strings.Contains(string(out), stringKey) { return string(out), false } + + // If the field is not found in the YAML, it means it's empty/omitted (due to omitempty tags) + // This is expected for fields like systemReservedCgroup when set to empty string + if len(matches) == 0 { + return "", true + } + require.Len(t, matches, 2, fmt.Sprintf("failed to get %s", regexKey)) require.NotEmpty(t, matches[1], "regex %s attempted on kubelet config of node %s came back empty", node.Name, regexKey)