From 4919f6ee22b5f6cf53f801e13072f6d64027c215 Mon Sep 17 00:00:00 2001 From: Nico Berlee Date: Thu, 19 Oct 2023 19:59:29 +0200 Subject: [PATCH] feat: add GOMEMLIMIT to shipped manifests with memory limits This commit integrates the GOMEMLIMIT environment variable into shipped K8S manifests when resources.limits.memory is defined. It is set to 95% of the memory limit to optimize the performance of the Go garbage collector, mitigating the risk of OOMKills in containerized environments. When configuring the controller-manager or scheduler custom resources in machine config, they where accepted, but ignored. This commit adds Resources to NewControlPlaneSchedulerController and NewControlPlaneControllerManagerController so machine config resources Fixes #7874 Signed-off-by: Nico Berlee Signed-off-by: Andrey Smirnov --- .../pkg/controllers/k8s/control_plane.go | 2 + .../k8s/control_plane_static_pod.go | 41 +++++++++-- .../k8s/control_plane_static_pod_test.go | 16 ++++- .../pkg/controllers/k8s/control_plane_test.go | 70 +++++++++++++++++++ .../machined/pkg/controllers/k8s/templates.go | 3 + pkg/flannel/gen.go | 6 +- 6 files changed, 128 insertions(+), 10 deletions(-) diff --git a/internal/app/machined/pkg/controllers/k8s/control_plane.go b/internal/app/machined/pkg/controllers/k8s/control_plane.go index bd19282e55..e4d65ccbe7 100644 --- a/internal/app/machined/pkg/controllers/k8s/control_plane.go +++ b/internal/app/machined/pkg/controllers/k8s/control_plane.go @@ -167,6 +167,7 @@ func NewControlPlaneControllerManagerController() *ControlPlaneControllerManager ExtraArgs: cfgProvider.Cluster().ControllerManager().ExtraArgs(), ExtraVolumes: convertVolumes(cfgProvider.Cluster().ControllerManager().ExtraVolumes()), EnvironmentVariables: cfgProvider.Cluster().ControllerManager().Env(), + Resources: convertResources(cfgProvider.Cluster().ControllerManager().Resources()), } return nil @@ -193,6 +194,7 @@ func NewControlPlaneSchedulerController() *ControlPlaneSchedulerController { ExtraArgs: cfgProvider.Cluster().Scheduler().ExtraArgs(), ExtraVolumes: convertVolumes(cfgProvider.Cluster().Scheduler().ExtraVolumes()), EnvironmentVariables: cfgProvider.Cluster().Scheduler().Env(), + Resources: convertResources(cfgProvider.Cluster().Scheduler().Resources()), } return nil diff --git a/internal/app/machined/pkg/controllers/k8s/control_plane_static_pod.go b/internal/app/machined/pkg/controllers/k8s/control_plane_static_pod.go index 3460dc88b6..d37df29749 100644 --- a/internal/app/machined/pkg/controllers/k8s/control_plane_static_pod.go +++ b/internal/app/machined/pkg/controllers/k8s/control_plane_static_pod.go @@ -8,7 +8,7 @@ import ( "context" "fmt" "path/filepath" - "sort" + "slices" "strconv" "strings" @@ -35,6 +35,9 @@ import ( // systemCriticalPriority is copied from scheduling.SystemCriticalPriority in Kubernetes internals. const systemCriticalPriority int32 = 2000000000 +// GoGCMemLimitPercentage set the percentage of memorylimit to use for the golang garbage collection target limit. +const GoGCMemLimitPercentage = 95 + // ControlPlaneStaticPodController manages k8s.StaticPod based on control plane configuration. type ControlPlaneStaticPodController struct{} @@ -264,7 +267,7 @@ func envVars(environment map[string]string) []v1.EnvVar { } keys := maps.Keys(environment) - sort.Strings(keys) + slices.Sort(keys) return xslices.Map(keys, func(key string) v1.EnvVar { // Kubernetes supports variable references in variable values, so escape '$' to prevent that. @@ -323,6 +326,19 @@ func resources(resourcesConfig k8s.Resources, defaultCPU, defaultMemory string) return resources, nil } +func goGCEnvFromResources(resources v1.ResourceRequirements) (envVar v1.EnvVar) { + memoryLimit := resources.Limits[v1.ResourceMemory] + if memoryLimit.Value() > 0 { + gcMemLimit := memoryLimit.Value() * GoGCMemLimitPercentage / 100 + envVar = v1.EnvVar{ + Name: "GOMEMLIMIT", + Value: strconv.FormatInt(gcMemLimit, 10), + } + } + + return envVar +} + func (ctrl *ControlPlaneStaticPodController) manageAPIServer(ctx context.Context, r controller.Runtime, logger *zap.Logger, configResource resource.Resource, secretsVersion, configVersion string, ) (string, error) { @@ -424,6 +440,11 @@ func (ctrl *ControlPlaneStaticPodController) manageAPIServer(ctx context.Context return "", err } + env := envVars(cfg.EnvironmentVariables) + if goGCEnv := goGCEnvFromResources(resources); goGCEnv.Name != "" { + env = append(env, goGCEnv) + } + return k8s.APIServerID, r.Modify(ctx, k8s.NewStaticPod(k8s.NamespaceName, k8s.APIServerID), func(r resource.Resource) error { return k8sadapter.StaticPod(r.(*k8s.StaticPod)).SetPod(&v1.Pod{ TypeMeta: metav1.TypeMeta{ @@ -462,7 +483,7 @@ func (ctrl *ControlPlaneStaticPodController) manageAPIServer(ctx context.Context }, }, }, - envVars(cfg.EnvironmentVariables)...), + env...), VolumeMounts: append([]v1.VolumeMount{ { Name: "secrets", @@ -593,6 +614,11 @@ func (ctrl *ControlPlaneStaticPodController) manageControllerManager(ctx context return "", err } + env := envVars(cfg.EnvironmentVariables) + if goGCEnv := goGCEnvFromResources(resources); goGCEnv.Name != "" { + env = append(env, goGCEnv) + } + //nolint:dupl return k8s.ControllerManagerID, r.Modify(ctx, k8s.NewStaticPod(k8s.NamespaceName, k8s.ControllerManagerID), func(r resource.Resource) error { return k8sadapter.StaticPod(r.(*k8s.StaticPod)).SetPod(&v1.Pod{ @@ -620,7 +646,7 @@ func (ctrl *ControlPlaneStaticPodController) manageControllerManager(ctx context Name: k8s.ControllerManagerID, Image: cfg.Image, Command: args, - Env: envVars(cfg.EnvironmentVariables), + Env: env, VolumeMounts: append([]v1.VolumeMount{ { Name: "secrets", @@ -727,6 +753,11 @@ func (ctrl *ControlPlaneStaticPodController) manageScheduler(ctx context.Context return "", err } + env := envVars(cfg.EnvironmentVariables) + if goGCEnv := goGCEnvFromResources(resources); goGCEnv.Name != "" { + env = append(env, goGCEnv) + } + //nolint:dupl return k8s.SchedulerID, r.Modify(ctx, k8s.NewStaticPod(k8s.NamespaceName, k8s.SchedulerID), func(r resource.Resource) error { return k8sadapter.StaticPod(r.(*k8s.StaticPod)).SetPod(&v1.Pod{ @@ -754,7 +785,7 @@ func (ctrl *ControlPlaneStaticPodController) manageScheduler(ctx context.Context Name: k8s.SchedulerID, Image: cfg.Image, Command: args, - Env: envVars(cfg.EnvironmentVariables), + Env: env, VolumeMounts: append([]v1.VolumeMount{ { Name: "secrets", diff --git a/internal/app/machined/pkg/controllers/k8s/control_plane_static_pod_test.go b/internal/app/machined/pkg/controllers/k8s/control_plane_static_pod_test.go index 4077848f70..50348ffd52 100644 --- a/internal/app/machined/pkg/controllers/k8s/control_plane_static_pod_test.go +++ b/internal/app/machined/pkg/controllers/k8s/control_plane_static_pod_test.go @@ -10,6 +10,7 @@ import ( "fmt" "log" "reflect" + "strconv" "strings" "sync" "testing" @@ -634,8 +635,9 @@ func (suite *ControlPlaneStaticPodSuite) TestReconcileStaticPodResources() { suite.Require().NoError(suite.state.Create(suite.ctx, secretStatus)) tests := []struct { - resources k8s.Resources - expected v1.ResourceRequirements + resources k8s.Resources + expected v1.ResourceRequirements + expectedEnv v1.EnvVar }{ { resources: k8s.Resources{ @@ -672,6 +674,10 @@ func (suite *ControlPlaneStaticPodSuite) TestReconcileStaticPodResources() { v1.ResourceMemory: apiresource.MustParse("1Gi"), }, }, + expectedEnv: v1.EnvVar{ + Name: "GOMEMLIMIT", + Value: strconv.FormatInt(1024*1024*1024*k8sctrl.GoGCMemLimitPercentage/100, 10), + }, }, } for _, test := range tests { @@ -738,6 +744,12 @@ func (suite *ControlPlaneStaticPodSuite) TestReconcileStaticPodResources() { suite.Assert().Equal(test.expected, controllerManagerPod.Spec.Containers[0].Resources) suite.Assert().Equal(test.expected, schedulerPod.Spec.Containers[0].Resources) + if test.expectedEnv.Name != "" { + suite.Assert().Contains(apiServerPod.Spec.Containers[0].Env, test.expectedEnv) + suite.Assert().Contains(controllerManagerPod.Spec.Containers[0].Env, test.expectedEnv) + suite.Assert().Contains(schedulerPod.Spec.Containers[0].Env, test.expectedEnv) + } + suite.Require().NoError(suite.state.Destroy(suite.ctx, configAPIServer.Metadata())) suite.Require().NoError(suite.state.Destroy(suite.ctx, configControllerManager.Metadata())) suite.Require().NoError(suite.state.Destroy(suite.ctx, configScheduler.Metadata())) diff --git a/internal/app/machined/pkg/controllers/k8s/control_plane_test.go b/internal/app/machined/pkg/controllers/k8s/control_plane_test.go index 29b29f812c..b1093c2024 100644 --- a/internal/app/machined/pkg/controllers/k8s/control_plane_test.go +++ b/internal/app/machined/pkg/controllers/k8s/control_plane_test.go @@ -332,6 +332,38 @@ func (suite *K8sControlPlaneSuite) TestReconcileResources() { }, }, }, + ControllerManagerConfig: &v1alpha1.ControllerManagerConfig{ + ResourcesConfig: &v1alpha1.ResourcesConfig{ + Requests: v1alpha1.Unstructured{ + Object: map[string]interface{}{ + "cpu": "50m", + "memory": "500Mi", + }, + }, + Limits: v1alpha1.Unstructured{ + Object: map[string]interface{}{ + "cpu": 1, + "memory": "1000Mi", + }, + }, + }, + }, + SchedulerConfig: &v1alpha1.SchedulerConfig{ + ResourcesConfig: &v1alpha1.ResourcesConfig{ + Requests: v1alpha1.Unstructured{ + Object: map[string]interface{}{ + "cpu": "150m", + "memory": "2Gi", + }, + }, + Limits: v1alpha1.Unstructured{ + Object: map[string]interface{}{ + "cpu": 3, + "memory": "2000Mi", + }, + }, + }, + }, }, }, ), @@ -357,6 +389,44 @@ func (suite *K8sControlPlaneSuite) TestReconcileResources() { ) }, ) + + rtestutils.AssertResources(suite.Ctx(), suite.T(), suite.State(), []resource.ID{k8s.ControllerManagerConfigID}, + func(controllerManager *k8s.ControllerManagerConfig, assert *assert.Assertions) { + controllerManagerCfg := controllerManager.TypedSpec() + + assert.Equal( + k8s.Resources{ + Requests: map[string]string{ + "cpu": "50m", + "memory": "500Mi", + }, + Limits: map[string]string{ + "cpu": "1", + "memory": "1000Mi", + }, + }, controllerManagerCfg.Resources, + ) + }, + ) + + rtestutils.AssertResources(suite.Ctx(), suite.T(), suite.State(), []resource.ID{k8s.SchedulerConfigID}, + func(scheduler *k8s.SchedulerConfig, assert *assert.Assertions) { + schedulerCfg := scheduler.TypedSpec() + + assert.Equal( + k8s.Resources{ + Requests: map[string]string{ + "cpu": "150m", + "memory": "2Gi", + }, + Limits: map[string]string{ + "cpu": "3", + "memory": "2000Mi", + }, + }, schedulerCfg.Resources, + ) + }, + ) } func (suite *K8sControlPlaneSuite) TestReconcileExternalCloudProvider() { diff --git a/internal/app/machined/pkg/controllers/k8s/templates.go b/internal/app/machined/pkg/controllers/k8s/templates.go index a01325bc02..20c2ec5431 100644 --- a/internal/app/machined/pkg/controllers/k8s/templates.go +++ b/internal/app/machined/pkg/controllers/k8s/templates.go @@ -357,6 +357,9 @@ spec: requests: cpu: 100m memory: 70Mi + env: + - name: GOMEMLIMIT + value: "161MiB" args: [ "-conf", "/etc/coredns/Corefile" ] volumeMounts: - name: config-volume diff --git a/pkg/flannel/gen.go b/pkg/flannel/gen.go index b8af9e65eb..4d342cf076 100644 --- a/pkg/flannel/gen.go +++ b/pkg/flannel/gen.go @@ -15,7 +15,7 @@ import ( "regexp" "strings" - "github.com/siderolabs/gen/slices" + "github.com/siderolabs/gen/xslices" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -184,10 +184,10 @@ var Template = []byte(`+"`", url) ds.Spec.Template.Spec.Containers[0].Image = "{{ .FlannelImage }}" - ds.Spec.Template.Spec.Volumes = slices.FilterInPlace(ds.Spec.Template.Spec.Volumes, func(v corev1.Volume) bool { + ds.Spec.Template.Spec.Volumes = xslices.FilterInPlace(ds.Spec.Template.Spec.Volumes, func(v corev1.Volume) bool { return v.Name != "xtables-lock" }) - ds.Spec.Template.Spec.Containers[0].VolumeMounts = slices.FilterInPlace( + ds.Spec.Template.Spec.Containers[0].VolumeMounts = xslices.FilterInPlace( ds.Spec.Template.Spec.Containers[0].VolumeMounts, func(v corev1.VolumeMount) bool { return v.Name != "xtables-lock" })