Skip to content

Commit c6faace

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #5294 from ngopalak-redhat/ngopalak/mco_swap
OCPNODE-3747: Disable Swap mode in Kubelet and enable drop-in directory
2 parents b0470ee + fc21115 commit c6faace

File tree

9 files changed

+275
-0
lines changed

9 files changed

+275
-0
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,12 @@ func validateUserKubeletConfig(cfg *mcfgv1.KubeletConfig) error {
380380
if kcDecoded.StaticPodPath != "" {
381381
return fmt.Errorf("KubeletConfiguration: staticPodPath is not allowed to be set, but contains: %s", kcDecoded.StaticPodPath)
382382
}
383+
if kcDecoded.FailSwapOn != nil {
384+
return fmt.Errorf("KubeletConfiguration: failSwapOn is not allowed to be set, but contains: %v", *kcDecoded.FailSwapOn)
385+
}
386+
if kcDecoded.MemorySwap.SwapBehavior != "" {
387+
return fmt.Errorf("KubeletConfiguration: swapBehavior is not allowed to be set, but contains: %s", kcDecoded.MemorySwap.SwapBehavior)
388+
}
383389
if kcDecoded.SystemReserved != nil && len(kcDecoded.SystemReserved) > 0 &&
384390
cfg.Spec.AutoSizingReserved != nil && *cfg.Spec.AutoSizingReserved {
385391
return fmt.Errorf("KubeletConfiguration: autoSizingReserved and systemdReserved cannot be set together")

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -926,6 +926,20 @@ func TestKubeletConfigDenylistedOptions(t *testing.T) {
926926
},
927927
},
928928
},
929+
{
930+
name: "test banned failSwapOn",
931+
config: &kubeletconfigv1beta1.KubeletConfiguration{
932+
FailSwapOn: pointer.BoolPtr(true),
933+
},
934+
},
935+
{
936+
name: "test banned swapBehavior",
937+
config: &kubeletconfigv1beta1.KubeletConfiguration{
938+
MemorySwap: kubeletconfigv1beta1.MemorySwapConfiguration{
939+
SwapBehavior: "NoSwap",
940+
},
941+
},
942+
},
929943
}
930944

931945
successTests := []struct {

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package kubeletconfig
2+
3+
import (
4+
"testing"
5+
6+
osev1 "github.com/openshift/api/config/v1"
7+
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
// TestFailSwapOnConfiguration verifies that failSwapOn is correctly configured in kubelet templates.
13+
// This test ensures that:
14+
// - Master and arbiter nodes have failSwapOn: true (swap is disabled)
15+
// - Worker nodes have failSwapOn: false (swap is allowed but controlled via memorySwap.swapBehavior)
16+
// - All nodes have memorySwap.swapBehavior set to "NoSwap"
17+
func TestFailSwapOnConfiguration(t *testing.T) {
18+
for _, platform := range []osev1.PlatformType{osev1.AWSPlatformType, osev1.NonePlatformType, "unrecognized"} {
19+
t.Run(string(platform), func(t *testing.T) {
20+
f := newFixture(t)
21+
fgHandler := ctrlcommon.NewFeatureGatesHardcodedHandler([]osev1.FeatureGateName{"Example"}, nil)
22+
23+
cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform)
24+
f.ccLister = append(f.ccLister, cc)
25+
26+
ctrl := f.newController(fgHandler)
27+
testCases := []struct {
28+
nodeRole string
29+
expectedFail bool
30+
}{
31+
{"master", true},
32+
{"arbiter", true},
33+
{"worker", false},
34+
}
35+
36+
for _, tc := range testCases {
37+
t.Run(tc.nodeRole, func(t *testing.T) {
38+
kubeletConfig, err := generateOriginalKubeletConfigIgn(cc, ctrl.templatesDir, tc.nodeRole, &osev1.APIServer{})
39+
require.NoError(t, err, "Failed to generate kubelet config for %s", tc.nodeRole)
40+
41+
contents, err := ctrlcommon.DecodeIgnitionFileContents(kubeletConfig.Contents.Source, kubeletConfig.Contents.Compression)
42+
require.NoError(t, err, "Failed to decode ignition file contents for %s", tc.nodeRole)
43+
44+
originalKubeConfig, err := DecodeKubeletConfig(contents)
45+
require.NoError(t, err, "Failed to decode kubelet config for %s", tc.nodeRole)
46+
47+
require.NotNil(t, originalKubeConfig.FailSwapOn, "failSwapOn should not be nil for %s node role", tc.nodeRole)
48+
assert.Equal(t, tc.expectedFail, *originalKubeConfig.FailSwapOn, "failSwapOn should be %v for %s node role", tc.expectedFail, tc.nodeRole)
49+
50+
assert.Equal(t, "NoSwap", originalKubeConfig.MemorySwap.SwapBehavior, "memorySwap.swapBehavior should be NoSwap for %s node role", tc.nodeRole)
51+
})
52+
}
53+
})
54+
}
55+
}

‎pkg/controller/template/kubelet_config_dir_test.go‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
package template
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
configv1 "github.com/openshift/api/config/v1"
8+
mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
9+
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// TestKubeletConfigDirParameter verifies that the --config-dir parameter is correctly configured
15+
// in kubelet.service systemd unit files for worker nodes only.
16+
// This test ensures that:
17+
// - Worker nodes have --config-dir=/etc/openshift/kubelet.conf.d in their kubelet.service
18+
// - Master and arbiter nodes do NOT have the --config-dir parameter
19+
//
20+
// The --config-dir parameter allows the kubelet to load additional configuration from a directory,
21+
// which is useful for dynamic kubelet configuration updates on worker nodes.
22+
func TestKubeletConfigDirParameter(t *testing.T) {
23+
testCases := []struct {
24+
name string
25+
platform configv1.PlatformType
26+
controllerConfig string
27+
expectedWorkerFlag bool
28+
expectedMasterFlag bool
29+
}{
30+
{
31+
name: "AWS",
32+
platform: configv1.AWSPlatformType,
33+
controllerConfig: "./test_data/controller_config_aws.yaml",
34+
expectedWorkerFlag: true,
35+
expectedMasterFlag: false,
36+
},
37+
{
38+
name: "BareMetal",
39+
platform: configv1.BareMetalPlatformType,
40+
controllerConfig: "./test_data/controller_config_baremetal.yaml",
41+
expectedWorkerFlag: true,
42+
expectedMasterFlag: false,
43+
},
44+
{
45+
name: "GCP",
46+
platform: configv1.GCPPlatformType,
47+
controllerConfig: "./test_data/controller_config_gcp.yaml",
48+
expectedWorkerFlag: true,
49+
expectedMasterFlag: false,
50+
},
51+
{
52+
name: "None",
53+
platform: configv1.NonePlatformType,
54+
controllerConfig: "./test_data/controller_config_none.yaml",
55+
expectedWorkerFlag: true,
56+
expectedMasterFlag: false,
57+
},
58+
}
59+
60+
for _, tc := range testCases {
61+
t.Run(tc.name, func(t *testing.T) {
62+
controllerConfig, err := controllerConfigFromFile(tc.controllerConfig)
63+
require.NoError(t, err, "Failed to load controller config for %s", tc.name)
64+
65+
cfgs, err := generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, templateDir)
66+
require.NoError(t, err, "Failed to generate machine configs for %s", tc.name)
67+
68+
kubeletConfigs := make(map[string]*string)
69+
70+
for _, cfg := range cfgs {
71+
role, ok := cfg.Labels[mcfgv1.MachineConfigRoleLabelKey]
72+
require.True(t, ok, "Machine config missing role label")
73+
74+
ign, err := ctrlcommon.ParseAndConvertConfig(cfg.Spec.Config.Raw)
75+
require.NoError(t, err, "Failed to parse Ignition config for %s/%s", tc.name, role)
76+
77+
for _, unit := range ign.Systemd.Units {
78+
if unit.Name == "kubelet.service" && unit.Contents != nil {
79+
kubeletConfigs[role] = unit.Contents
80+
break
81+
}
82+
}
83+
}
84+
85+
for role, kubeletUnit := range kubeletConfigs {
86+
87+
hasConfigDirFlag := strings.Contains(*kubeletUnit, "--config-dir=/etc/openshift/kubelet.conf.d")
88+
89+
switch role {
90+
case workerRole:
91+
if tc.expectedWorkerFlag {
92+
assert.True(t, hasConfigDirFlag,
93+
"Worker node should have --config-dir parameter for platform %s", tc.name)
94+
} else {
95+
assert.False(t, hasConfigDirFlag,
96+
"Worker node should not have --config-dir parameter for platform %s", tc.name)
97+
}
98+
case masterRole:
99+
if tc.expectedMasterFlag {
100+
assert.True(t, hasConfigDirFlag,
101+
"Master node should have --config-dir parameter for platform %s", tc.name)
102+
} else {
103+
assert.False(t, hasConfigDirFlag,
104+
"Master node should not have --config-dir parameter for platform %s", tc.name)
105+
}
106+
case arbiterRole:
107+
if tc.expectedMasterFlag {
108+
assert.True(t, hasConfigDirFlag,
109+
"Arbiter node should have --config-dir parameter for platform %s", tc.name)
110+
} else {
111+
assert.False(t, hasConfigDirFlag,
112+
"Arbiter node should not have --config-dir parameter for platform %s", tc.name)
113+
}
114+
}
115+
116+
if hasConfigDirFlag {
117+
assert.Contains(t, *kubeletUnit, "--config-dir=/etc/openshift/kubelet.conf.d",
118+
"--config-dir should point to /etc/openshift/kubelet.conf.d for %s/%s", tc.name, role)
119+
}
120+
}
121+
})
122+
}
123+
}
124+
125+
// TestKubeletConfigDirParameterSpecific tests the exact location and format of the --config-dir parameter
126+
func TestKubeletConfigDirParameterSpecific(t *testing.T) {
127+
controllerConfig, err := controllerConfigFromFile("./test_data/controller_config_aws.yaml")
128+
require.NoError(t, err, "Failed to load AWS controller config")
129+
130+
cfgs, err := generateTemplateMachineConfigs(&RenderConfig{&controllerConfig.Spec, `{"dummy":"dummy"}`, "dummy", nil, nil}, templateDir)
131+
require.NoError(t, err, "Failed to generate machine configs")
132+
133+
var kubeletUnit *string
134+
for _, cfg := range cfgs {
135+
if role, ok := cfg.Labels[mcfgv1.MachineConfigRoleLabelKey]; ok && role == workerRole {
136+
ign, err := ctrlcommon.ParseAndConvertConfig(cfg.Spec.Config.Raw)
137+
require.NoError(t, err, "Failed to parse worker Ignition config")
138+
139+
for _, unit := range ign.Systemd.Units {
140+
if unit.Name == "kubelet.service" && unit.Contents != nil {
141+
kubeletUnit = unit.Contents
142+
break
143+
}
144+
}
145+
if kubeletUnit != nil {
146+
break
147+
}
148+
}
149+
}
150+
151+
require.NotNil(t, kubeletUnit, "kubelet.service unit not found in worker config")
152+
153+
lines := strings.Split(*kubeletUnit, "\n")
154+
var execStartLines []string
155+
156+
inExecStart := false
157+
for _, line := range lines {
158+
trimmedLine := strings.TrimSpace(line)
159+
if strings.HasPrefix(trimmedLine, "ExecStart=") {
160+
inExecStart = true
161+
execStartLines = append(execStartLines, trimmedLine)
162+
} else if inExecStart && strings.HasSuffix(trimmedLine, "\\") {
163+
execStartLines = append(execStartLines, trimmedLine)
164+
} else if inExecStart && !strings.HasSuffix(trimmedLine, "\\") {
165+
execStartLines = append(execStartLines, trimmedLine)
166+
break
167+
}
168+
}
169+
170+
require.NotEmpty(t, execStartLines, "ExecStart lines not found in kubelet.service")
171+
172+
fullExecStart := strings.Join(execStartLines, " ")
173+
174+
assert.Contains(t, fullExecStart, "--config-dir=/etc/openshift/kubelet.conf.d",
175+
"Worker kubelet.service should contain --config-dir=/etc/openshift/kubelet.conf.d")
176+
177+
configIndex := strings.Index(fullExecStart, "--config=/etc/kubernetes/kubelet.conf")
178+
configDirIndex := strings.Index(fullExecStart, "--config-dir=/etc/openshift/kubelet.conf.d")
179+
bootstrapIndex := strings.Index(fullExecStart, "--bootstrap-kubeconfig=/etc/kubernetes/kubeconfig")
180+
181+
assert.Greater(t, configDirIndex, configIndex,
182+
"--config-dir should appear after --config parameter")
183+
assert.Less(t, configDirIndex, bootstrapIndex,
184+
"--config-dir should appear before --bootstrap-kubeconfig parameter")
185+
186+
t.Logf("Worker kubelet.service ExecStart:\n%s", fullExecStart)
187+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ contents:
1616
clusterDomain: cluster.local
1717
containerLogMaxSize: 50Mi
1818
enableSystemLogQuery: true
19+
failSwapOn: true
1920
maxPods: 250
2021
kubeAPIQPS: 50
2122
kubeAPIBurst: 100
@@ -24,6 +25,8 @@ contents:
2425
rotateCertificates: true
2526
serializeImagePulls: false
2627
staticPodPath: /etc/kubernetes/manifests
28+
memorySwap:
29+
swapBehavior: NoSwap
2730
systemCgroups: /system.slice
2831
nodeStatusUpdateFrequency: 10s
2932
nodeStatusReportFrequency: 5m

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ contents:
1616
clusterDomain: cluster.local
1717
containerLogMaxSize: 50Mi
1818
enableSystemLogQuery: true
19+
failSwapOn: true
1920
maxPods: 250
2021
kubeAPIQPS: 50
2122
kubeAPIBurst: 100
@@ -24,6 +25,8 @@ contents:
2425
rotateCertificates: true
2526
serializeImagePulls: false
2627
staticPodPath: /etc/kubernetes/manifests
28+
memorySwap:
29+
swapBehavior: NoSwap
2730
systemCgroups: /system.slice
2831
nodeStatusUpdateFrequency: 10s
2932
nodeStatusReportFrequency: 5m

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ contents:
1616
clusterDomain: cluster.local
1717
containerLogMaxSize: 50Mi
1818
enableSystemLogQuery: true
19+
failSwapOn: false
1920
maxPods: 250
2021
kubeAPIQPS: 50
2122
kubeAPIBurst: 100
@@ -24,6 +25,8 @@ contents:
2425
rotateCertificates: true
2526
serializeImagePulls: false
2627
staticPodPath: /etc/kubernetes/manifests
28+
memorySwap:
29+
swapBehavior: NoSwap
2730
systemCgroups: /system.slice
2831
nodeStatusUpdateFrequency: 10s
2932
nodeStatusReportFrequency: 5m

‎templates/worker/01-worker-kubelet/_base/units/kubelet.service.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ contents: |
1010
[Service]
1111
Type=notify
1212
ExecStartPre=/bin/mkdir --parents /etc/kubernetes/manifests
13+
ExecStartPre=/bin/mkdir --parents /etc/openshift/kubelet.conf.d
1314
ExecStartPre=-/usr/sbin/restorecon -ri /var/lib/kubelet/pod-resources /usr/local/bin/kubenswrapper /usr/bin/kubensenter
1415
{{- if eq .IPFamilies "IPv6"}}
1516
Environment="KUBELET_NODE_IP=::"
@@ -24,6 +25,7 @@ contents: |
2425
ExecStart=/usr/local/bin/kubenswrapper \
2526
/usr/bin/kubelet \
2627
--config=/etc/kubernetes/kubelet.conf \
28+
--config-dir=/etc/openshift/kubelet.conf.d \
2729
--bootstrap-kubeconfig=/etc/kubernetes/kubeconfig \
2830
--kubeconfig=/var/lib/kubelet/kubeconfig \
2931
--container-runtime-endpoint=/var/run/crio/crio.sock \

‎templates/worker/01-worker-kubelet/on-prem/units/kubelet.service.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ contents: |
1010
[Service]
1111
Type=notify
1212
ExecStartPre=/bin/mkdir --parents /etc/kubernetes/manifests
13+
ExecStartPre=/bin/mkdir --parents /etc/openshift/kubelet.conf.d
1314
ExecStartPre=-/usr/sbin/restorecon -ri /var/lib/kubelet/pod-resources /usr/local/bin/kubenswrapper /usr/bin/kubensenter
1415
{{- if eq .IPFamilies "IPv6"}}
1516
Environment="KUBELET_NODE_IP=::"
@@ -24,6 +25,7 @@ contents: |
2425
ExecStart=/usr/local/bin/kubenswrapper \
2526
/usr/bin/kubelet \
2627
--config=/etc/kubernetes/kubelet.conf \
28+
--config-dir=/etc/openshift/kubelet.conf.d \
2729
--bootstrap-kubeconfig=/etc/kubernetes/kubeconfig \
2830
--kubeconfig=/var/lib/kubelet/kubeconfig \
2931
--container-runtime-endpoint=/var/run/crio/crio.sock \

0 commit comments

Comments
 (0)