From 09ba5770fa1f355ef5d8827dbb1e1eafd2d579e6 Mon Sep 17 00:00:00 2001 From: Gunju Kim Date: Wed, 25 Mar 2026 02:07:35 +0000 Subject: [PATCH] Add configurable resource requests/limits for the controller --- .github/workflows/deploy-dev.yaml | 4 +- docs/reference.md | 2 + internal/cli/install.go | 20 ++- internal/cli/install_test.go | 163 ++++++++++++++++-- .../charts/kelos/templates/deployment.yaml | 18 +- internal/manifests/charts/kelos/values.yaml | 4 + 6 files changed, 194 insertions(+), 17 deletions(-) diff --git a/.github/workflows/deploy-dev.yaml b/.github/workflows/deploy-dev.yaml index 863cee50..47de0830 100644 --- a/.github/workflows/deploy-dev.yaml +++ b/.github/workflows/deploy-dev.yaml @@ -53,7 +53,9 @@ jobs: run: | bin/kelos install --version main --image-pull-policy Always \ --spawner-resource-requests cpu=100m,memory=128Mi \ - --token-refresher-resource-requests cpu=50m,memory=64Mi + --token-refresher-resource-requests cpu=50m,memory=64Mi \ + --controller-resource-requests cpu=10m,memory=64Mi \ + --controller-resource-limits cpu=500m,memory=128Mi kubectl rollout restart deployment/kelos-controller-manager -n kelos-system kubectl rollout status deployment/kelos-controller-manager -n kelos-system --timeout=120s kubectl rollout restart deployment -l app.kubernetes.io/component=spawner -n "${KELOS_NAMESPACE}" diff --git a/docs/reference.md b/docs/reference.md index 63b8032f..bdf9fc0f 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -313,6 +313,8 @@ The `kelos` CLI lets you manage the full lifecycle without writing YAML. - `--spawner-resource-limits`: Resource limits for spawner containers as comma-separated `name=value` pairs - `--token-refresher-resource-requests`: Resource requests for token refresher sidecars as comma-separated `name=value` pairs, for example `cpu=100m,memory=128Mi` - `--token-refresher-resource-limits`: Resource limits for token refresher sidecars as comma-separated `name=value` pairs, for example `cpu=200m,memory=256Mi` +- `--controller-resource-requests`: Resource requests for the controller container as comma-separated `name=value` pairs, for example `cpu=10m,memory=64Mi` +- `--controller-resource-limits`: Resource limits for the controller container as comma-separated `name=value` pairs, for example `cpu=500m,memory=128Mi` ### `kelos run` Flags diff --git a/internal/cli/install.go b/internal/cli/install.go index 79b8b52f..ef8375f5 100644 --- a/internal/cli/install.go +++ b/internal/cli/install.go @@ -39,6 +39,8 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command { var spawnerResourceLimits string var tokenRefresherResourceRequests string var tokenRefresherResourceLimits string + var controllerResourceRequests string + var controllerResourceLimits string cmd := &cobra.Command{ Use: "install", @@ -57,6 +59,8 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command { spawnerResourceLimits, tokenRefresherResourceRequests, tokenRefresherResourceLimits, + controllerResourceRequests, + controllerResourceLimits, ) controllerManifest, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { @@ -111,12 +115,14 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command { cmd.Flags().StringVar(&spawnerResourceLimits, "spawner-resource-limits", "", "resource limits for spawner containers (e.g., cpu=1,memory=1Gi)") cmd.Flags().StringVar(&tokenRefresherResourceRequests, "token-refresher-resource-requests", "", "resource requests for token refresher sidecars (e.g., cpu=100m,memory=128Mi)") cmd.Flags().StringVar(&tokenRefresherResourceLimits, "token-refresher-resource-limits", "", "resource limits for token refresher sidecars (e.g., cpu=200m,memory=256Mi)") + cmd.Flags().StringVar(&controllerResourceRequests, "controller-resource-requests", "", "resource requests for the controller container (e.g., cpu=10m,memory=64Mi)") + cmd.Flags().StringVar(&controllerResourceLimits, "controller-resource-limits", "", "resource limits for the controller container (e.g., cpu=500m,memory=128Mi)") return cmd } // buildHelmValues constructs the values map for Helm chart rendering from CLI flags. -func buildHelmValues(ver string, pullPolicy string, disableHeartbeat bool, spawnerResourceRequests string, spawnerResourceLimits string, tokenRefresherResourceRequests string, tokenRefresherResourceLimits string) map[string]interface{} { +func buildHelmValues(ver string, pullPolicy string, disableHeartbeat bool, spawnerResourceRequests string, spawnerResourceLimits string, tokenRefresherResourceRequests string, tokenRefresherResourceLimits string, controllerResourceRequests string, controllerResourceLimits string) map[string]interface{} { imageVals := map[string]interface{}{ "tag": ver, } @@ -143,6 +149,18 @@ func buildHelmValues(ver string, pullPolicy string, disableHeartbeat bool, spawn if tokenRefresherResourceLimits != "" { vals["tokenRefresherResourceLimits"] = tokenRefresherResourceLimits } + controllerResources := map[string]interface{}{} + if controllerResourceRequests != "" { + controllerResources["requests"] = controllerResourceRequests + } + if controllerResourceLimits != "" { + controllerResources["limits"] = controllerResourceLimits + } + if len(controllerResources) > 0 { + vals["controller"] = map[string]interface{}{ + "resources": controllerResources, + } + } return vals } diff --git a/internal/cli/install_test.go b/internal/cli/install_test.go index 56c0b851..50c4e43d 100644 --- a/internal/cli/install_test.go +++ b/internal/cli/install_test.go @@ -149,7 +149,7 @@ func TestParseManifests_EmbeddedCRDs(t *testing.T) { func renderDefaultChart(t *testing.T) []byte { t.Helper() - vals := buildHelmValues("v0.0.0-test", "", false, "", "", "", "") + vals := buildHelmValues("v0.0.0-test", "", false, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -178,7 +178,7 @@ func TestRenderChart_DefaultValues(t *testing.T) { } func TestRenderChart_VersionSubstitution(t *testing.T) { - vals := buildHelmValues("v0.5.0", "", false, "", "", "", "") + vals := buildHelmValues("v0.5.0", "", false, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -192,7 +192,7 @@ func TestRenderChart_VersionSubstitution(t *testing.T) { } func TestRenderChart_ImageArgs(t *testing.T) { - vals := buildHelmValues("v0.3.0", "", false, "", "", "", "") + vals := buildHelmValues("v0.3.0", "", false, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -213,7 +213,7 @@ func TestRenderChart_ImageArgs(t *testing.T) { } func TestRenderChart_ImagePullPolicy(t *testing.T) { - vals := buildHelmValues("v0.1.0", "Always", false, "", "", "", "") + vals := buildHelmValues("v0.1.0", "Always", false, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -236,7 +236,7 @@ func TestRenderChart_ImagePullPolicy(t *testing.T) { } func TestRenderChart_NoPullPolicyByDefault(t *testing.T) { - vals := buildHelmValues("latest", "", false, "", "", "", "") + vals := buildHelmValues("latest", "", false, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -250,7 +250,7 @@ func TestRenderChart_NoPullPolicyByDefault(t *testing.T) { } func TestRenderChart_DisableHeartbeat(t *testing.T) { - vals := buildHelmValues("latest", "", true, "", "", "", "") + vals := buildHelmValues("latest", "", true, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -277,7 +277,7 @@ func TestRenderChart_DisableHeartbeat(t *testing.T) { } func TestRenderChart_EnableHeartbeat(t *testing.T) { - vals := buildHelmValues("latest", "", false, "", "", "", "") + vals := buildHelmValues("latest", "", false, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -537,8 +537,104 @@ func TestInstallCommand_NoTokenRefresherResourcesByDefault(t *testing.T) { } } +func TestInstallCommand_ControllerResourceRequestsFlag(t *testing.T) { + cmd := NewRootCommand() + cmd.SetArgs([]string{"install", "--dry-run", "--controller-resource-requests", "cpu=10m,memory=64Mi"}) + + output := captureStdout(t, func() { + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + if !strings.Contains(output, "cpu: 10m") { + t.Errorf("expected cpu: 10m in output") + } + if !strings.Contains(output, "memory: 64Mi") { + t.Errorf("expected memory: 64Mi in output") + } +} + +func TestInstallCommand_ControllerResourceLimitsFlag(t *testing.T) { + cmd := NewRootCommand() + cmd.SetArgs([]string{"install", "--dry-run", "--controller-resource-limits", "cpu=500m,memory=128Mi"}) + + output := captureStdout(t, func() { + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + if !strings.Contains(output, "cpu: 500m") { + t.Errorf("expected cpu: 500m in output") + } + if !strings.Contains(output, "memory: 128Mi") { + t.Errorf("expected memory: 128Mi in output") + } +} + +func TestInstallCommand_NoControllerResourcesByDefault(t *testing.T) { + cmd := NewRootCommand() + cmd.SetArgs([]string{"install", "--dry-run"}) + + output := captureStdout(t, func() { + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + // Extract only the Deployment document so we don't match resources from + // the telemetry CronJob (which legitimately contains cpu: 10m / memory: 64Mi). + deployment := extractYAMLDocument(t, []byte(output), "kind: Deployment") + + // Verify neither old limit defaults nor old request defaults are rendered. + for _, needle := range []string{"cpu: 500m", "memory: 128Mi", "cpu: 10m", "memory: 64Mi"} { + if strings.Contains(deployment, needle) { + t.Errorf("expected no hardcoded %s in controller Deployment when resources not set", needle) + } + } +} + +func TestRenderChart_ControllerResources(t *testing.T) { + vals := buildHelmValues("latest", "", false, "", "", "", "", "cpu=100m,memory=256Mi", "cpu=1,memory=512Mi") + data, err := helmchart.Render(manifests.ChartFS, vals) + if err != nil { + t.Fatalf("rendering chart: %v", err) + } + if !bytes.Contains(data, []byte("cpu: 100m")) { + t.Error("expected cpu: 100m in rendered output for controller requests") + } + if !bytes.Contains(data, []byte("memory: 256Mi")) { + t.Error("expected memory: 256Mi in rendered output for controller requests") + } + if !bytes.Contains(data, []byte("cpu: 1\n")) { + t.Error("expected cpu: 1 in rendered output for controller limits") + } + if !bytes.Contains(data, []byte("memory: 512Mi")) { + t.Error("expected memory: 512Mi in rendered output for controller limits") + } +} + +func TestRenderChart_NoControllerResourcesByDefault(t *testing.T) { + vals := buildHelmValues("latest", "", false, "", "", "", "", "", "") + data, err := helmchart.Render(manifests.ChartFS, vals) + if err != nil { + t.Fatalf("rendering chart: %v", err) + } + // Extract only the Deployment document so we don't match resources from + // the telemetry CronJob (which legitimately contains cpu: 10m / memory: 64Mi). + deployment := extractYAMLDocument(t, data, "kind: Deployment") + + // Verify neither old limit defaults nor old request defaults are rendered. + for _, needle := range []string{"cpu: 500m", "memory: 128Mi", "cpu: 10m", "memory: 64Mi"} { + if strings.Contains(deployment, needle) { + t.Errorf("expected no hardcoded %s in controller Deployment when resources not set", needle) + } + } +} + func TestRenderChart_SpawnerResources(t *testing.T) { - vals := buildHelmValues("latest", "", false, "cpu=250m,memory=512Mi", "cpu=1,memory=1Gi", "", "") + vals := buildHelmValues("latest", "", false, "cpu=250m,memory=512Mi", "cpu=1,memory=1Gi", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -552,7 +648,7 @@ func TestRenderChart_SpawnerResources(t *testing.T) { } func TestRenderChart_TokenRefresherResources(t *testing.T) { - vals := buildHelmValues("latest", "", false, "", "", "cpu=100m,memory=128Mi", "cpu=200m,memory=256Mi") + vals := buildHelmValues("latest", "", false, "", "", "cpu=100m,memory=128Mi", "cpu=200m,memory=256Mi", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -566,7 +662,7 @@ func TestRenderChart_TokenRefresherResources(t *testing.T) { } func TestRenderChart_NoSpawnerResourcesByDefault(t *testing.T) { - vals := buildHelmValues("latest", "", false, "", "", "", "") + vals := buildHelmValues("latest", "", false, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -580,7 +676,7 @@ func TestRenderChart_NoSpawnerResourcesByDefault(t *testing.T) { } func TestRenderChart_NoTokenRefresherResourcesByDefault(t *testing.T) { - vals := buildHelmValues("latest", "", false, "", "", "", "") + vals := buildHelmValues("latest", "", false, "", "", "", "", "", "") data, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { t.Fatalf("rendering chart: %v", err) @@ -730,6 +826,8 @@ func TestBuildHelmValues(t *testing.T) { spawnerResourceLimits string tokenRefresherResourceRequests string tokenRefresherResourceLimits string + controllerResourceRequests string + controllerResourceLimits string checkFn func(t *testing.T, vals map[string]interface{}) }{ { @@ -758,6 +856,9 @@ func TestBuildHelmValues(t *testing.T) { if _, ok := vals["tokenRefresherResourceLimits"]; ok { t.Error("expected no tokenRefresherResourceLimits when empty") } + if _, ok := vals["controller"]; ok { + t.Error("expected no controller key when empty") + } }, }, { @@ -822,6 +923,30 @@ func TestBuildHelmValues(t *testing.T) { } }, }, + { + name: "with controller resource requests", + version: "latest", + controllerResourceRequests: "cpu=10m,memory=64Mi", + checkFn: func(t *testing.T, vals map[string]interface{}) { + ctrl := vals["controller"].(map[string]interface{}) + res := ctrl["resources"].(map[string]interface{}) + if res["requests"] != "cpu=10m,memory=64Mi" { + t.Errorf("expected controller.resources.requests=cpu=10m,memory=64Mi, got %v", res["requests"]) + } + }, + }, + { + name: "with controller resource limits", + version: "latest", + controllerResourceLimits: "cpu=500m,memory=128Mi", + checkFn: func(t *testing.T, vals map[string]interface{}) { + ctrl := vals["controller"].(map[string]interface{}) + res := ctrl["resources"].(map[string]interface{}) + if res["limits"] != "cpu=500m,memory=128Mi" { + t.Errorf("expected controller.resources.limits=cpu=500m,memory=128Mi, got %v", res["limits"]) + } + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -833,8 +958,24 @@ func TestBuildHelmValues(t *testing.T) { tt.spawnerResourceLimits, tt.tokenRefresherResourceRequests, tt.tokenRefresherResourceLimits, + tt.controllerResourceRequests, + tt.controllerResourceLimits, ) tt.checkFn(t, vals) }) } } + +// extractYAMLDocument returns the first YAML document from data whose content +// contains the given marker string. Documents are separated by "---". +func extractYAMLDocument(t *testing.T, data []byte, marker string) string { + t.Helper() + docs := strings.Split(string(data), "---") + for _, doc := range docs { + if strings.Contains(doc, marker) { + return doc + } + } + t.Fatalf("no YAML document containing %q found in rendered output", marker) + return "" +} diff --git a/internal/manifests/charts/kelos/templates/deployment.yaml b/internal/manifests/charts/kelos/templates/deployment.yaml index 7168375a..e2d68c2e 100644 --- a/internal/manifests/charts/kelos/templates/deployment.yaml +++ b/internal/manifests/charts/kelos/templates/deployment.yaml @@ -94,10 +94,20 @@ spec: port: 8081 initialDelaySeconds: 5 periodSeconds: 10 + {{- if or .Values.controller.resources.requests .Values.controller.resources.limits }} resources: + {{- if .Values.controller.resources.limits }} limits: - cpu: 500m - memory: 128Mi + {{- range splitList "," .Values.controller.resources.limits }} + {{- $kv := splitList "=" . }} + {{ trim (index $kv 0) }}: {{ trim (index $kv 1) }} + {{- end }} + {{- end }} + {{- if .Values.controller.resources.requests }} requests: - cpu: 10m - memory: 64Mi + {{- range splitList "," .Values.controller.resources.requests }} + {{- $kv := splitList "=" . }} + {{ trim (index $kv 0) }}: {{ trim (index $kv 1) }} + {{- end }} + {{- end }} + {{- end }} diff --git a/internal/manifests/charts/kelos/values.yaml b/internal/manifests/charts/kelos/values.yaml index 70ef9044..d000fb55 100644 --- a/internal/manifests/charts/kelos/values.yaml +++ b/internal/manifests/charts/kelos/values.yaml @@ -17,3 +17,7 @@ spawnerResourceLimits: "" tokenRefresherImage: ghcr.io/kelos-dev/kelos-token-refresher tokenRefresherResourceRequests: "" tokenRefresherResourceLimits: "" +controller: + resources: + requests: "" + limits: ""