Skip to content

Add configurable resource requests/limits for the controller #783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kelos-bot wants to merge 1 commit into
base: main
Choose a base branch
from
+194 −17
Open

Add configurable resource requests/limits for the controller #783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/deploy-dev.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
2 changes: 2 additions & 0 deletions docs/reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
20 changes: 19 additions & 1 deletion internal/cli/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
}
Expand All @@ -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
}

Expand Down
163 changes: 152 additions & 11 deletions internal/cli/install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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{})
}{
{
Expand Down Expand Up @@ -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")
}
},
},
{
Expand Down Expand Up @@ -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) {
Expand All @@ -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 ""
}
18 changes: 14 additions & 4 deletions internal/manifests/charts/kelos/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
4 changes: 4 additions & 0 deletions internal/manifests/charts/kelos/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@ spawnerResourceLimits: ""
tokenRefresherImage: ghcr.io/kelos-dev/kelos-token-refresher
tokenRefresherResourceRequests: ""
tokenRefresherResourceLimits: ""
controller:
resources:
requests: ""
limits: ""
Loading