WIP: Modify user slice cpu weights

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
 }

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)
+}

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)

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)

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

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" <<EOF
+    [Slice]
+    CPUShares=100
+    EOF
+
+        echo "Created $CPU_WEIGHT_CONF_FILE with CPUShares=100"
+
+        # Reload systemd to pick up the changes
+        systemctl daemon-reload
+    elif [ "$SYSTEM_RESERVED_COMPRESSIBLE_ENABLED" == "false" ]; then
+        # Remove the CPU weight configuration file if it exists
+        if [ -f "$CPU_WEIGHT_CONF_FILE" ]; then
+            rm -f "$CPU_WEIGHT_CONF_FILE"
+            echo "Deleted $CPU_WEIGHT_CONF_FILE"
+
+            # Reload systemd to pick up the changes
+            systemctl daemon-reload
+        else
+            echo "$CPU_WEIGHT_CONF_FILE does not exist, nothing to delete"
+        fi
+    else
+        echo "Unrecognized command line option. Valid options are \"true\" or \"false\""
+        exit 1
+    fi

templates/common/_base/files/system-reserved-compressible.yaml

@@ -0,0 +1,5 @@
+mode: 0644
+path: "/etc/system-reserved-compressible.env"
+contents:
+  inline: |
+    SYSTEM_RESERVED_COMPRESSIBLE_ENABLED=false

templates/common/_base/units/configure-user-slice.service.yaml

@@ -0,0 +1,18 @@
+name: configure-user-slice.service
+enabled: true
+contents: |
+  [Unit]
+  Description=Configure user.slice CPU shares based on systemReservedCgroup
+  Wants=network-online.target
+  After=network-online.target firstboot-osupdate.target
+  Before=kubelet-dependencies.target
+
+  [Service]
+  # Need oneshot to delay kubelet
+  Type=oneshot
+  RemainAfterExit=yes
+  EnvironmentFile=/etc/system-reserved-compressible.env
+  ExecStart=/bin/bash /usr/local/sbin/configure-user-slice.sh ${SYSTEM_RESERVED_COMPRESSIBLE_ENABLED}
+
+  [Install]
+  RequiredBy=kubelet-dependencies.target