Skip to content

Commit 00bb8e1

Browse files
ngopalak-redhatngopalak-redhat
committed
OCPNODE-3201: Make system-reserved-compressible default
1 parent ca0c19d commit 00bb8e1

File tree

5 files changed

+197
-0
lines changed

5 files changed

+197
-0
lines changed

‎pkg/controller/kubelet-config/helpers.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,12 @@ func validateUserKubeletConfig(cfg *mcfgv1.KubeletConfig) error {
390390
cfg.Spec.AutoSizingReserved != nil && *cfg.Spec.AutoSizingReserved {
391391
return fmt.Errorf("KubeletConfiguration: autoSizingReserved and systemdReserved cannot be set together")
392392
}
393+
// Validate that systemReservedCgroup matches systemCgroups if both are set
394+
if kcDecoded.SystemReservedCgroup != "" && kcDecoded.SystemCgroups != "" {
395+
if kcDecoded.SystemReservedCgroup != kcDecoded.SystemCgroups {
396+
return fmt.Errorf("KubeletConfiguration: systemReservedCgroup (%s) must match systemCgroups (%s)", kcDecoded.SystemReservedCgroup, kcDecoded.SystemCgroups)
397+
}
398+
}
393399
return nil
394400
}
395401

@@ -508,6 +514,23 @@ func generateKubeletIgnFiles(kubeletConfig *mcfgv1.KubeletConfig, originalKubeCo
508514
}
509515
}
510516

517+
// Handle systemReservedCgroup and enforceNodeAllocatable based on:
518+
// reservedSystemCPUs being set (incompatible with systemReservedCgroup)
519+
shouldDisableSystemReservedCgroup := false
520+
521+
// Check if reservedSystemCPUs is set (incompatible with systemReservedCgroup)
522+
if originalKubeConfig.ReservedSystemCPUs != "" {
523+
shouldDisableSystemReservedCgroup = true
524+
klog.Infof("reservedSystemCPUs is set to %s, disabling systemReservedCgroup enforcement", originalKubeConfig.ReservedSystemCPUs)
525+
}
526+
527+
if shouldDisableSystemReservedCgroup {
528+
// Clear systemReservedCgroup
529+
originalKubeConfig.SystemReservedCgroup = ""
530+
// Set enforceNodeAllocatable to only pods
531+
originalKubeConfig.EnforceNodeAllocatable = []string{"pods"}
532+
}
533+
511534
// Encode the new config into an Ignition File
512535
kubeletIgnition, err := kubeletConfigToIgnFile(originalKubeConfig)
513536
if err != nil {

‎pkg/controller/kubelet-config/helpers_test.go‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package kubeletconfig
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
"k8s.io/apimachinery/pkg/runtime"
9+
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
10+
11+
mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
12+
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
13+
)
14+
15+
// TestGenerateKubeletIgnFilesWithReservedSystemCPUs tests that when reservedSystemCPUs is set,
16+
// the systemReservedCgroup is cleared and enforceNodeAllocatable is set to ["pods"] only.
17+
func TestGenerateKubeletIgnFilesWithReservedSystemCPUs(t *testing.T) {
18+
testCases := []struct {
19+
name string
20+
reservedSystemCPUs string
21+
initialSystemReservedCgroup string
22+
initialEnforceNodeAllocatable []string
23+
expectedSystemReservedCgroup string
24+
expectedEnforceNodeAllocatable []string
25+
shouldDisableSystemReservedCgroup bool
26+
}{
27+
{
28+
name: "reservedSystemCPUs set - should disable systemReservedCgroup",
29+
reservedSystemCPUs: "0-1",
30+
initialSystemReservedCgroup: "/system.slice",
31+
initialEnforceNodeAllocatable: []string{"pods", "system-reserved-compressible"},
32+
expectedSystemReservedCgroup: "",
33+
expectedEnforceNodeAllocatable: []string{"pods"},
34+
shouldDisableSystemReservedCgroup: true,
35+
},
36+
{
37+
name: "reservedSystemCPUs not set - should preserve systemReservedCgroup",
38+
reservedSystemCPUs: "",
39+
initialSystemReservedCgroup: "/system.slice",
40+
initialEnforceNodeAllocatable: []string{"pods", "system-reserved-compressible"},
41+
expectedSystemReservedCgroup: "/system.slice",
42+
expectedEnforceNodeAllocatable: []string{"pods", "system-reserved-compressible"},
43+
shouldDisableSystemReservedCgroup: false,
44+
},
45+
{
46+
name: "reservedSystemCPUs set with empty systemReservedCgroup",
47+
reservedSystemCPUs: "0-3",
48+
initialSystemReservedCgroup: "",
49+
initialEnforceNodeAllocatable: []string{"pods"},
50+
expectedSystemReservedCgroup: "",
51+
expectedEnforceNodeAllocatable: []string{"pods"},
52+
shouldDisableSystemReservedCgroup: true,
53+
},
54+
}
55+
56+
for _, tc := range testCases {
57+
t.Run(tc.name, func(t *testing.T) {
58+
// Setup: Create a base kubelet configuration with the initial values
59+
originalKubeConfig := &kubeletconfigv1beta1.KubeletConfiguration{
60+
ReservedSystemCPUs: tc.reservedSystemCPUs,
61+
SystemReservedCgroup: tc.initialSystemReservedCgroup,
62+
EnforceNodeAllocatable: tc.initialEnforceNodeAllocatable,
63+
}
64+
65+
// Create a KubeletConfig CR (can be nil for this test)
66+
kubeletConfig := &mcfgv1.KubeletConfig{
67+
ObjectMeta: metav1.ObjectMeta{
68+
Name: "test-kubelet-config",
69+
},
70+
Spec: mcfgv1.KubeletConfigSpec{},
71+
}
72+
73+
// Execute: Generate the kubelet ignition files
74+
kubeletIgnition, _, _, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig)
75+
require.NoError(t, err, "generateKubeletIgnFiles should not return an error")
76+
require.NotNil(t, kubeletIgnition, "kubelet ignition file should not be nil")
77+
78+
// Verify: Decode the generated kubelet configuration from the ignition file
79+
contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletIgnition.Contents.Source, kubeletIgnition.Contents.Compression)
80+
require.NoError(t, err, "decoding ignition file contents should succeed")
81+
82+
// Parse the YAML contents back into a KubeletConfiguration
83+
decodedConfig, err := DecodeKubeletConfig(contents)
84+
require.NoError(t, err, "decoding kubelet config should succeed")
85+
86+
// Verify: Check that systemReservedCgroup matches expected value
87+
require.Equal(t, tc.expectedSystemReservedCgroup, decodedConfig.SystemReservedCgroup,
88+
"systemReservedCgroup should be %q but got %q", tc.expectedSystemReservedCgroup, decodedConfig.SystemReservedCgroup)
89+
90+
// Verify: Check that enforceNodeAllocatable matches expected value
91+
require.Equal(t, tc.expectedEnforceNodeAllocatable, decodedConfig.EnforceNodeAllocatable,
92+
"enforceNodeAllocatable should be %v but got %v", tc.expectedEnforceNodeAllocatable, decodedConfig.EnforceNodeAllocatable)
93+
94+
// Verify: Check that reservedSystemCPUs is preserved
95+
require.Equal(t, tc.reservedSystemCPUs, decodedConfig.ReservedSystemCPUs,
96+
"reservedSystemCPUs should be %q but got %q", tc.reservedSystemCPUs, decodedConfig.ReservedSystemCPUs)
97+
})
98+
}
99+
}
100+
101+
// TestGenerateKubeletIgnFilesWithKubeletConfigSpec tests that generateKubeletIgnFiles
102+
// properly merges user-provided kubelet configuration with the original config.
103+
func TestGenerateKubeletIgnFilesWithKubeletConfigSpec(t *testing.T) {
104+
// Setup: Create a base kubelet configuration
105+
originalKubeConfig := &kubeletconfigv1beta1.KubeletConfiguration{
106+
MaxPods: 110,
107+
ReservedSystemCPUs: "0-1",
108+
SystemReservedCgroup: "/system.slice",
109+
EnforceNodeAllocatable: []string{"pods", "system-reserved-compressible"},
110+
}
111+
112+
// Setup: Create user-provided kubelet configuration with reservedSystemCPUs
113+
userKubeletConfig := &kubeletconfigv1beta1.KubeletConfiguration{
114+
MaxPods: 250,
115+
ReservedSystemCPUs: "0-3", // User wants to reserve more CPUs
116+
}
117+
118+
// Encode the user config
119+
userKubeletConfigRaw, err := EncodeKubeletConfig(userKubeletConfig, kubeletconfigv1beta1.SchemeGroupVersion, runtime.ContentTypeYAML)
120+
require.NoError(t, err)
121+
122+
// Create a KubeletConfig CR with user config
123+
kubeletConfig := &mcfgv1.KubeletConfig{
124+
ObjectMeta: metav1.ObjectMeta{
125+
Name: "test-kubelet-config",
126+
},
127+
Spec: mcfgv1.KubeletConfigSpec{
128+
KubeletConfig: &runtime.RawExtension{
129+
Raw: userKubeletConfigRaw,
130+
},
131+
},
132+
}
133+
134+
// Execute: Generate the kubelet ignition files
135+
kubeletIgnition, _, _, err := generateKubeletIgnFiles(kubeletConfig, originalKubeConfig)
136+
require.NoError(t, err, "generateKubeletIgnFiles should not return an error")
137+
require.NotNil(t, kubeletIgnition, "kubelet ignition file should not be nil")
138+
139+
// Verify: Decode the generated kubelet configuration from the ignition file
140+
contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletIgnition.Contents.Source, kubeletIgnition.Contents.Compression)
141+
require.NoError(t, err, "decoding ignition file contents should succeed")
142+
143+
// Parse the YAML contents back into a KubeletConfiguration
144+
decodedConfig, err := DecodeKubeletConfig(contents)
145+
require.NoError(t, err, "decoding kubelet config should succeed")
146+
147+
// Verify: Check that user config was merged (MaxPods should be from user config)
148+
require.Equal(t, int32(250), decodedConfig.MaxPods,
149+
"MaxPods should be 250 from user config but got %d", decodedConfig.MaxPods)
150+
151+
// Verify: Check that reservedSystemCPUs was merged from user config
152+
require.Equal(t, "0-3", decodedConfig.ReservedSystemCPUs,
153+
"reservedSystemCPUs should be 0-3 from user config but got %q", decodedConfig.ReservedSystemCPUs)
154+
155+
// Verify: Check that systemReservedCgroup was cleared (since reservedSystemCPUs is set)
156+
require.Equal(t, "", decodedConfig.SystemReservedCgroup,
157+
"systemReservedCgroup should be empty when reservedSystemCPUs is set but got %q", decodedConfig.SystemReservedCgroup)
158+
159+
// Verify: Check that enforceNodeAllocatable was set to only ["pods"]
160+
require.Equal(t, []string{"pods"}, decodedConfig.EnforceNodeAllocatable,
161+
"enforceNodeAllocatable should be [pods] when reservedSystemCPUs is set but got %v", decodedConfig.EnforceNodeAllocatable)
162+
}

‎templates/arbiter/01-arbiter-kubelet/_base/files/kubelet.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ contents:
2828
memorySwap:
2929
swapBehavior: NoSwap
3030
systemCgroups: /system.slice
31+
systemReservedCgroup: /system.slice
32+
enforceNodeAllocatable:
33+
- pods
34+
- system-reserved-compressible
3135
nodeStatusUpdateFrequency: 10s
3236
nodeStatusReportFrequency: 5m
3337
serverTLSBootstrap: true

‎templates/master/01-master-kubelet/_base/files/kubelet.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ contents:
2828
memorySwap:
2929
swapBehavior: NoSwap
3030
systemCgroups: /system.slice
31+
systemReservedCgroup: /system.slice
32+
enforceNodeAllocatable:
33+
- pods
34+
- system-reserved-compressible
3135
nodeStatusUpdateFrequency: 10s
3236
nodeStatusReportFrequency: 5m
3337
serverTLSBootstrap: true

‎templates/worker/01-worker-kubelet/_base/files/kubelet.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ contents:
2828
memorySwap:
2929
swapBehavior: NoSwap
3030
systemCgroups: /system.slice
31+
systemReservedCgroup: /system.slice
32+
enforceNodeAllocatable:
33+
- pods
34+
- system-reserved-compressible
3135
nodeStatusUpdateFrequency: 10s
3236
nodeStatusReportFrequency: 5m
3337
serverTLSBootstrap: true

0 commit comments

Comments
 (0)