From 0f0777b9f5c8472c0ae717c3e31ef4205334c0d8 Mon Sep 17 00:00:00 2001 From: Sai Ramesh Vanka Date: Sat, 15 Jul 2023 06:17:27 +0530 Subject: [PATCH] OCPNODE-1717: Make cgroupsv2 default in OCP-4.14 1. RHCOS comes up with cgroupsv2 by default. 2. In order to make cgroupsv2 as a default in OCP-4.14, the changes that explicitly set the cgroupsv1 should be removed 3. Explicitly set the cgroupMode to "v2" so that the underlying nodes get updated (RHEL8 worker nodes) Signed-off-by: Sai Ramesh Vanka --- .../kubelet-config/kubelet_config_nodes.go | 11 +++++ .../kubelet_config_nodes_test.go | 4 +- pkg/controller/template/render.go | 43 +++---------------- test/e2e-bootstrap/bootstrap_test.go | 15 +++++-- 4 files changed, 29 insertions(+), 44 deletions(-) diff --git a/pkg/controller/kubelet-config/kubelet_config_nodes.go b/pkg/controller/kubelet-config/kubelet_config_nodes.go index 7b9e6d23ef..88fc6a178e 100644 --- a/pkg/controller/kubelet-config/kubelet_config_nodes.go +++ b/pkg/controller/kubelet-config/kubelet_config_nodes.go @@ -76,6 +76,12 @@ func (ctrl *Controller) syncNodeConfigHandler(key string) error { if err := ctrl.cleanUpDuplicatedMC(managedNodeConfigKeyPrefix); err != nil { return err } + // explicitly setting the cgroupMode to "v2" and also updating the config node's spec if found empty + // This helps in updating the cgroupMode on all the worker nodes if they still have cgroupsv1 (Ex: RHEL8 workers) + if nodeConfig.Spec.CgroupMode == osev1.CgroupModeEmpty { + nodeConfig.Spec.CgroupMode = osev1.CgroupModeV2 + ctrl.configClient.ConfigV1().Nodes().Update(context.TODO(), nodeConfig, metav1.UpdateOptions{}) + } // checking if the Node spec is empty and accordingly returning from here. if reflect.DeepEqual(nodeConfig.Spec, osev1.NodeSpec{}) { klog.V(2).Info("empty Node resource found") @@ -263,6 +269,11 @@ func RunNodeConfigBootstrap(templateDir string, featureGateAccess featuregates.F if nodeConfig == nil { return nil, fmt.Errorf("nodes.config.openshift.io resource not found") } + // explicitly setting the cgroupMode to "v2" and also updating the config node's spec if found empty + // This helps in updating the cgroupMode on all the worker nodes if they still have cgroupsv1 (Ex: RHEL8 workers) + if nodeConfig.Spec.CgroupMode == osev1.CgroupModeEmpty { + nodeConfig.Spec.CgroupMode = osev1.CgroupModeV2 + } // checking if the Node spec is empty and accordingly returning from here. if reflect.DeepEqual(nodeConfig.Spec, osev1.NodeSpec{}) { klog.V(2).Info("empty Node resource found") diff --git a/pkg/controller/kubelet-config/kubelet_config_nodes_test.go b/pkg/controller/kubelet-config/kubelet_config_nodes_test.go index 74afef7a30..a8c1d7d17e 100644 --- a/pkg/controller/kubelet-config/kubelet_config_nodes_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_nodes_test.go @@ -94,8 +94,8 @@ func TestBootstrapNodeConfigDefault(t *testing.T) { if err != nil { t.Errorf("could not run node config bootstrap: %v", err) } - if len(mcs) != 0 { - t.Errorf("expected no machine configs generated with the default node config, got %v machine configs", len(mcs)) + if len(mcs) != 2 { + t.Errorf("expected %v machine configs generated with the default node config, got 0 machine configs", len(mcs)) } }) } diff --git a/pkg/controller/template/render.go b/pkg/controller/template/render.go index e17164e37c..147c815789 100644 --- a/pkg/controller/template/render.go +++ b/pkg/controller/template/render.go @@ -34,13 +34,11 @@ type RenderConfig struct { } const ( - filesDir = "files" - unitsDir = "units" - platformBase = "_base" - platformOnPrem = "on-prem" - sno = "sno" - baseMasterKubeletMC = "01-master-kubelet" - baseWorkerKubeletMC = "01-worker-kubelet" + filesDir = "files" + unitsDir = "units" + platformBase = "_base" + platformOnPrem = "on-prem" + sno = "sno" ) // generateTemplateMachineConfigs returns MachineConfig objects from the templateDir and a config object @@ -124,43 +122,12 @@ func GenerateMachineConfigsForRole(config *RenderConfig, role, templateDir strin if err != nil { return nil, err } - // defaulting the CGroups version to "v1" - // (TODO) This code can be removed if not required in future releases - if name == baseMasterKubeletMC || name == baseWorkerKubeletMC { - updateMCwithDefaultCgroupsVersion(nameConfig) - } cfgs = append(cfgs, nameConfig) } return cfgs, nil } -// updateMCwithDefaultCgroupsVersion updates the MC kArgs with the default CGroups version config -func updateMCwithDefaultCgroupsVersion(mc *mcfgv1.MachineConfig) { - // updating the Machine Config resource with the default cgroupv1 config - var ( - kernelArgsv1 = []string{"systemd.unified_cgroup_hierarchy=0", "systemd.legacy_systemd_cgroup_controller=1"} - kernelArgsv2 = []string{"systemd.unified_cgroup_hierarchy=1", "cgroup_no_v1=\"all\"", "psi=1"} - adjustedKernelArgs []string - ) - // avoiding the undesired kArgs from the already existing kArgs - for _, arg := range mc.Spec.KernelArguments { - // only append the args we want to keep, omitting the undesired - if !ctrlcommon.InSlice(arg, kernelArgsv2) { - adjustedKernelArgs = append(adjustedKernelArgs, arg) - } - } - // appending the desired kArgs to the existing kArgs - for _, arg := range kernelArgsv1 { - // add the additional that aren't already there - if !ctrlcommon.InSlice(arg, adjustedKernelArgs) { - adjustedKernelArgs = append(adjustedKernelArgs, arg) - } - } - // overwrite the KernelArguments with the adjusted KernelArgs - mc.Spec.KernelArguments = adjustedKernelArgs -} - func platformStringFromControllerConfigSpec(ic *mcfgv1.ControllerConfigSpec) (string, error) { if ic.Infra == nil { ic.Infra = &configv1.Infrastructure{ diff --git a/test/e2e-bootstrap/bootstrap_test.go b/test/e2e-bootstrap/bootstrap_test.go index 3be523b3a8..18ee73486f 100644 --- a/test/e2e-bootstrap/bootstrap_test.go +++ b/test/e2e-bootstrap/bootstrap_test.go @@ -121,8 +121,11 @@ kind: Node metadata: name: cluster`), }, - waitForMasterMCs: []string{"99-master-ssh", "99-master-generated-registries"}, - waitForWorkerMCs: []string{"99-worker-ssh", "99-worker-generated-registries"}, + // "CgroupMode" field in the nodes.config resource is empty + // Internally it gets updated to "v2" explicitly + // Hence, 97-{master/worker}-generated-kubelet are expected + waitForMasterMCs: []string{"99-master-ssh", "99-master-generated-registries", "97-master-generated-kubelet"}, + waitForWorkerMCs: []string{"99-worker-ssh", "99-worker-generated-registries", "97-worker-generated-kubelet"}, }, { name: "With a node config manifest empty \"cgroupMode\"", @@ -134,8 +137,10 @@ metadata: spec: workerLatencyProfile: MediumUpdateAverageReaction`), }, - - waitForMasterMCs: []string{"99-master-ssh", "99-master-generated-registries"}, + // "CgroupMode" field in the nodes.config resource is empty + // Internally it gets updated to "v2" explicitly + // Hence, 97-{master/worker}-generated-kubelet are expected + waitForMasterMCs: []string{"99-master-ssh", "99-master-generated-registries", "97-master-generated-kubelet"}, waitForWorkerMCs: []string{"99-worker-ssh", "99-worker-generated-registries", "97-worker-generated-kubelet"}, }, { @@ -262,6 +267,8 @@ spec: memory: 500Mi `), }, + // 97-{master/worker}-generated-kubelet are expected to be created as the empty "cgroupMode" + // internally translates to "v2" waitForMasterMCs: []string{"99-master-ssh", "99-master-generated-registries", "97-master-generated-kubelet"}, waitForWorkerMCs: []string{"99-worker-ssh", "99-worker-generated-registries", "99-worker-generated-kubelet", "97-worker-generated-kubelet"}, },