Skip to content

feat: add kube-api-qps and kube-api-burst flags for API client tuning - #347

Merged
Syspretor merged 1 commit into
sgl-project:mainfrom
Syspretor:feat/controller-tuning-flags
May 22, 2026
Merged

Syspretor merged 1 commit into
sgl-project:mainfrom
Syspretor:feat/controller-tuning-flags

Conversation

@Syspretor

Copy link
Copy Markdown
Collaborator

Summary

  • Add --kube-api-qps (default: 20) and --kube-api-burst (default: 30) flags to the controller binary, allowing operators to tune client-go rate limiting for large-scale deployments
  • Expose controllerTuning.maxConcurrentReconciles, controllerTuning.kubeApiQPS, and controllerTuning.kubeApiBurst in the Helm chart values.yaml
  • Pass all three tuning flags as container args in the Helm deployment template

Motivation

When running at scale (hundreds of RBGs), the default client-go rate limits (QPS=20, Burst=30) can become a bottleneck, causing client-side throttling and slower reconciliation. This change gives operators control over these parameters without code changes.

The --max-concurrent-reconciles flag already existed but was not configurable via Helm — now it is.

Changes

File Change
cmd/rbgs/main.go Add --kube-api-qps and --kube-api-burst flags, apply to rest.Config
deploy/helm/rbgs/values.yaml Add controllerTuning section
deploy/helm/rbgs/templates/manager.yaml Pass tuning flags as container args

Backward Compatibility

Defaults match existing client-go defaults (QPS=20, Burst=30, max-concurrent-reconciles=10). No behavioral change for existing deployments.

Test plan

  • make build passes
  • golangci-lint passes on ./cmd/rbgs/...
  • helm template renders correct args with both default and custom values
  • Deploy with custom values and verify controller starts with expected config

🤖 Generated with Claude Code

@Syspretor
Syspretor requested a review from cheyang May 21, 2026 09:03

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces Kubernetes API client tuning parameters, specifically QPS and Burst settings, along with a concurrency configuration for the controller. These are exposed via command-line flags and integrated into the Helm chart. Review feedback identifies a high-severity issue where flag.Parse() is executed before all flags (like logger options) are registered, which would cause them to be ignored. There is also a recommendation to improve the Helm template's robustness by adding conditional checks for the tuning configuration to prevent rendering errors and allow for zero-value overrides.

Comment thread cmd/rbgs/main.go Outdated
Comment thread deploy/helm/rbgs/templates/manager.yaml

@cheyang cheyang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

Add --kube-api-qps and --kube-api-burst flags to the controller,
allowing operators to tune client-go rate limiting for large-scale
deployments. Also expose max-concurrent-reconciles, kube-api-qps,
and kube-api-burst via the Helm chart under controllerTuning.

Defaults match client-go defaults (QPS=20, Burst=30) so this is
fully backward-compatible. When controllerTuning is omitted from
Helm values, the controller falls back to its internal defaults.

Also fix flag.Parse() ordering: move opts.BindFlags() before
flag.Parse() so that zap logger flags (--zap-log-level etc.) are
properly recognized.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: 玖宇 <guotongyu.gty@alibaba-inc.com>
@Syspretor
Syspretor force-pushed the feat/controller-tuning-flags branch from acc731d to 79b4cff Compare May 21, 2026 09:17
@Syspretor
Syspretor merged commit 5d9034e into sgl-project:main May 22, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants