diff --git a/pkg/controller/kubelet-config/helpers.go b/pkg/controller/kubelet-config/helpers.go index 50c0c597ab..a6de7b72f8 100644 --- a/pkg/controller/kubelet-config/helpers.go +++ b/pkg/controller/kubelet-config/helpers.go @@ -78,6 +78,12 @@ func createNewKubeletLogLevelIgnition(level int32) *ign3types.File { return &r } +func createSystemReservedCompressibleIgnition(enabled bool) *ign3types.File { + config := fmt.Sprintf("SYSTEM_RESERVED_COMPRESSIBLE_ENABLED=%t\n", enabled) + r := ctrlcommon.NewIgnFileBytesOverwriting("/etc/system-reserved-compressible.env", []byte(config)) + return &r +} + func createNewKubeletIgnition(yamlConfig []byte) *ign3types.File { r := ctrlcommon.NewIgnFileBytesOverwriting("/etc/kubernetes/kubelet.conf", yamlConfig) @@ -390,6 +396,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 } @@ -460,18 +472,19 @@ func kubeletConfigToIgnFile(cfg *kubeletconfigv1beta1.KubeletConfiguration) (*ig } // generateKubeletIgnFiles generates the Ignition files from the kubelet config -func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeConfig *kubeletconfigv1beta1.KubeletConfiguration) (*ign3types.File, *ign3types.File, *ign3types.File, error) { +func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeConfig *kubeletconfigv1beta1.KubeletConfiguration) (*ign3types.File, *ign3types.File, *ign3types.File, *ign3types.File, error) { var ( - kubeletIgnition *ign3types.File - logLevelIgnition *ign3types.File - autoSizingReservedIgnition *ign3types.File + kubeletIgnition *ign3types.File + logLevelIgnition *ign3types.File + autoSizingReservedIgnition *ign3types.File + systemReservedCompressibleIgnition *ign3types.File ) userDefinedSystemReserved := make(map[string]string) if kubeletConfig.Spec.KubeletConfig != nil && kubeletConfig.Spec.KubeletConfig.Raw != nil { specKubeletConfig, err := DecodeKubeletConfig(kubeletConfig.Spec.KubeletConfig.Raw) if err != nil { - return nil, nil, nil, fmt.Errorf("could not deserialize the new Kubelet config: %w", err) + return nil, nil, nil, nil, fmt.Errorf("could not deserialize the new Kubelet config: %w", err) } if val, ok := specKubeletConfig.SystemReserved["memory"]; ok { @@ -504,14 +517,37 @@ func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeCo // Merge the Old and New err = mergo.Merge(originalKubeConfig, specKubeletConfig, mergo.WithOverride) if err != nil { - return nil, nil, nil, fmt.Errorf("could not merge original config and new config: %w", err) + return nil, nil, nil, nil, fmt.Errorf("could not merge original config and new config: %w", err) } } + // Handle systemReservedCgroup and enforceNodeAllocatable based on: + // reservedSystemCPUs being set (incompatible with systemReservedCgroup) + shouldDisableSystemReservedCgroup := false + + // Check if reservedSystemCPUs is set (incompatible with systemReservedCgroup) + if originalKubeConfig.ReservedSystemCPUs != "" { + shouldDisableSystemReservedCgroup = true + klog.Infof("reservedSystemCPUs is set to %s, disabling systemReservedCgroup enforcement", originalKubeConfig.ReservedSystemCPUs) + } + + if shouldDisableSystemReservedCgroup { + // Clear systemReservedCgroup + originalKubeConfig.SystemReservedCgroup = "" + // Set enforceNodeAllocatable to only pods + originalKubeConfig.EnforceNodeAllocatable = []string{"pods"} + } + + // Create system reserved compressible ignition based on SystemReservedCgroup + // If SystemReservedCgroup is set (not empty), enable compressible (set to true) to set CPUShares=100 + // Otherwise, disable compressible (set to false) to not set CPUShares + systemReservedCompressibleEnabled := originalKubeConfig.SystemReservedCgroup != "" + systemReservedCompressibleIgnition = createSystemReservedCompressibleIgnition(systemReservedCompressibleEnabled) + // Encode the new config into an Ignition File kubeletIgnition, err := kubeletConfigToIgnFile(originalKubeConfig) if err != nil { - return nil, nil, nil, fmt.Errorf("could not encode JSON: %w", err) + return nil, nil, nil, nil, fmt.Errorf("could not encode JSON: %w", err) } if kubeletConfig.Spec.LogLevel != nil { @@ -524,5 +560,5 @@ func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeCo autoSizingReservedIgnition = createNewKubeletDynamicSystemReservedIgnition(nil, userDefinedSystemReserved) } - return kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, nil + return kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, systemReservedCompressibleIgnition, nil } diff --git a/pkg/controller/kubelet-config/helpers_test.go b/pkg/controller/kubelet-config/helpers_test.go new file mode 100644 index 0000000000..115d2f6d4c --- /dev/null +++ b/pkg/controller/kubelet-config/helpers_test.go @@ -0,0 +1,162 @@ +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" + + 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{"pods", "system-reserved-compressible"}, + expectedSystemReservedCgroup: "", + expectedEnforceNodeAllocatable: []string{"pods"}, + shouldDisableSystemReservedCgroup: true, + }, + { + name: "reservedSystemCPUs not set - should preserve systemReservedCgroup", + reservedSystemCPUs: "", + initialSystemReservedCgroup: "/system.slice", + initialEnforceNodeAllocatable: []string{"pods", "system-reserved-compressible"}, + expectedSystemReservedCgroup: "/system.slice", + expectedEnforceNodeAllocatable: []string{"pods", "system-reserved-compressible"}, + shouldDisableSystemReservedCgroup: false, + }, + { + name: "reservedSystemCPUs set with empty systemReservedCgroup", + reservedSystemCPUs: "0-3", + initialSystemReservedCgroup: "", + initialEnforceNodeAllocatable: []string{"pods"}, + expectedSystemReservedCgroup: "", + expectedEnforceNodeAllocatable: []string{"pods"}, + 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{"pods", "system-reserved-compressible"}, + } + + // 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{"pods"}, decodedConfig.EnforceNodeAllocatable, + "enforceNodeAllocatable should be [pods] when reservedSystemCPUs is set but got %v", decodedConfig.EnforceNodeAllocatable) +} diff --git a/pkg/controller/kubelet-config/kubelet_config_bootstrap.go b/pkg/controller/kubelet-config/kubelet_config_bootstrap.go index 2dced82bab..5258ab066e 100644 --- a/pkg/controller/kubelet-config/kubelet_config_bootstrap.go +++ b/pkg/controller/kubelet-config/kubelet_config_bootstrap.go @@ -55,20 +55,23 @@ func RunKubeletBootstrap(templateDir string, kubeletConfigs []*mcfgv1.KubeletCon originalKubeConfig.TLSCipherSuites = observedCipherSuites } - kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig) + kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, systemReservedCompressibleIgnition, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig) if err != nil { return nil, err } tempIgnConfig := ctrlcommon.NewIgnConfig() + if kubeletIgnition != nil { + tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *kubeletIgnition) + } if autoSizingReservedIgnition != nil { tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *autoSizingReservedIgnition) } if logLevelIgnition != nil { tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *logLevelIgnition) } - if kubeletIgnition != nil { - tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *kubeletIgnition) + if systemReservedCompressibleIgnition != nil { + tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *systemReservedCompressibleIgnition) } rawIgn, err := json.Marshal(tempIgnConfig) diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 2316c0827d..62c4b11fcf 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -652,7 +652,7 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { originalKubeConfig.TLSCipherSuites = observedCipherSuites } - kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, err := generateKubeletIgnFiles(cfg, originalKubeConfig) + kubeletIgnition, logLevelIgnition, autoSizingReservedIgnition, systemReservedCompressibleIgnition, err := generateKubeletIgnFiles(cfg, originalKubeConfig) if err != nil { return ctrl.syncStatusOnly(cfg, err) } @@ -686,14 +686,17 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { } tempIgnConfig := ctrlcommon.NewIgnConfig() + if kubeletIgnition != nil { + tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *kubeletIgnition) + } if autoSizingReservedIgnition != nil { tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *autoSizingReservedIgnition) } if logLevelIgnition != nil { tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *logLevelIgnition) } - if kubeletIgnition != nil { - tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *kubeletIgnition) + if systemReservedCompressibleIgnition != nil { + tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *systemReservedCompressibleIgnition) } rawIgn, err := json.Marshal(tempIgnConfig) 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/common/_base/files/configure-user-slice.yaml b/templates/common/_base/files/configure-user-slice.yaml new file mode 100644 index 0000000000..1b9959073f --- /dev/null +++ b/templates/common/_base/files/configure-user-slice.yaml @@ -0,0 +1,40 @@ +mode: 0755 +path: "/usr/local/sbin/configure-user-slice.sh" +contents: + inline: | + #!/bin/bash + set -e + + SYSTEM_RESERVED_COMPRESSIBLE_ENABLED=$1 + CPU_WEIGHT_CONF_DIR="/etc/systemd/system/user.slice.d" + CPU_WEIGHT_CONF_FILE="${CPU_WEIGHT_CONF_DIR}/99-cpu-weight.conf" + + if [ "$SYSTEM_RESERVED_COMPRESSIBLE_ENABLED" == "true" ]; then + # Create directory if it doesn't exist + mkdir -p "$CPU_WEIGHT_CONF_DIR" + + # Create the CPU weight configuration file + cat > "$CPU_WEIGHT_CONF_FILE" <