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..eb58147a 100644 --- a/internal/cli/install.go +++ b/internal/cli/install.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "strings" "time" "github.com/spf13/cobra" @@ -39,6 +40,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 +60,8 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command { spawnerResourceLimits, tokenRefresherResourceRequests, tokenRefresherResourceLimits, + controllerResourceRequests, + controllerResourceLimits, ) controllerManifest, err := helmchart.Render(manifests.ChartFS, vals) if err != nil { @@ -111,12 +116,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,9 +150,38 @@ func buildHelmValues(ver string, pullPolicy string, disableHeartbeat bool, spawn if tokenRefresherResourceLimits != "" { vals["tokenRefresherResourceLimits"] = tokenRefresherResourceLimits } + controllerResources := map[string]interface{}{} + if controllerResourceRequests != "" { + controllerResources["requests"] = parseResourceString(controllerResourceRequests) + } + if controllerResourceLimits != "" { + controllerResources["limits"] = parseResourceString(controllerResourceLimits) + } + if len(controllerResources) > 0 { + vals["controller"] = map[string]interface{}{ + "resources": controllerResources, + } + } return vals } +// parseResourceString converts a comma-separated key=value string (e.g. +// "cpu=100m,memory=256Mi") into a map suitable for Helm values. +func parseResourceString(s string) map[string]interface{} { + result := map[string]interface{}{} + for _, pair := range strings.Split(s, ",") { + pair = strings.TrimSpace(pair) + if pair == "" { + continue + } + parts := strings.SplitN(pair, "=", 2) + if len(parts) == 2 { + result[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1]) + } + } + return result +} + // kelosGVRs lists the kelos custom resource GVRs that need to be cleaned up // before the controller and CRDs can be safely removed. Resources with // finalizers (tasks, taskspawners) must be deleted while the controller is diff --git a/internal/cli/install_test.go b/internal/cli/install_test.go index 56c0b851..13bce9e4 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,32 @@ 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{}) + req := res["requests"].(map[string]interface{}) + if req["cpu"] != "10m" || req["memory"] != "64Mi" { + t.Errorf("expected controller.resources.requests={cpu:10m,memory:64Mi}, got %v", req) + } + }, + }, + { + 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{}) + lim := res["limits"].(map[string]interface{}) + if lim["cpu"] != "500m" || lim["memory"] != "128Mi" { + t.Errorf("expected controller.resources.limits={cpu:500m,memory:128Mi}, got %v", lim) + } + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -833,8 +960,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..575aa64c 100644 --- a/internal/manifests/charts/kelos/templates/deployment.yaml +++ b/internal/manifests/charts/kelos/templates/deployment.yaml @@ -94,10 +94,18 @@ 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 $k, $v := .Values.controller.resources.limits }} + {{ $k }}: {{ $v }} + {{- end }} + {{- end }} + {{- if .Values.controller.resources.requests }} requests: - cpu: 10m - memory: 64Mi + {{- range $k, $v := .Values.controller.resources.requests }} + {{ $k }}: {{ $v }} + {{- end }} + {{- end }} + {{- end }} diff --git a/internal/manifests/charts/kelos/values.yaml b/internal/manifests/charts/kelos/values.yaml index 70ef9044..bc4b9802 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: {}