Skip to content
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.

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

gha: Make client-go rate limiter params configurable #4

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ spec:
{{- with .Values.flags.maxConcurrentReconcilesForAutoscalingListener }}
- "--max-concurrent-reconciles-for-autoscaling-listener={{ . }}"
{{- end }}
{{- with .Values.flags.clientGoRateLimiterQPS }}
- "--client-go-rate-limiter-qps={{ . }}"
{{- end }}
{{- with .Values.flags.clientGoRateLimiterBurst }}
- "--client-go-rate-limiter-burst={{ . }}"
{{- end }}
command:
- "/manager"
{{- if or .Values.metrics .Values.pprof }}
Expand Down
4 changes: 4 additions & 0 deletions charts/gha-runner-scale-set-controller/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,7 @@ flags:
# maxConcurrentReconcilesForEphemeralRunnerSet: 1
# maxConcurrentReconcilesForEphemeralRunner: 1
# maxConcurrentReconcilesForAutoscalingListener: 1

## Defines the client-go rate limiter parameters.
# clientGoRateLimiterQPS: 20
# clientGoRateLimiterBurst: 30
15 changes: 11 additions & 4 deletions controllers/actions.github.com/ephemeralrunner_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ type EphemeralRunnerReconciler struct {
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := r.Log.WithValues("ephemeralrunner", req.NamespacedName)
log.Info("Start Reconcile")

ephemeralRunner := new(v1alpha1.EphemeralRunner)
if err := r.Get(ctx, req.NamespacedName, ephemeralRunner); err != nil {
Expand Down Expand Up @@ -183,6 +184,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return r.updateStatusWithRunnerConfig(ctx, ephemeralRunner, log)
}

now := time.Now()
secret := new(corev1.Secret)
if err := r.Get(ctx, req.NamespacedName, secret); err != nil {
if !kerrors.IsNotFound(err) {
Expand All @@ -193,6 +195,7 @@ func (r *EphemeralRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
log.Info("Creating new ephemeral runner secret for jitconfig.")
return r.createSecret(ctx, ephemeralRunner, log)
}
log.Info("Get Secret", "duration_ms", time.Since(now).Milliseconds())

pod := new(corev1.Pod)
if err := r.Get(ctx, req.NamespacedName, pod); err != nil {
Expand Down Expand Up @@ -344,8 +347,10 @@ func (r *EphemeralRunnerReconciler) cleanupResources(ctx context.Context, epheme
log.Info("Pod is deleted")

log.Info("Cleaning up the runner jitconfig secret")
now := time.Now()
secret := new(corev1.Secret)
err = r.Get(ctx, types.NamespacedName{Namespace: ephemeralRunner.Namespace, Name: ephemeralRunner.Name}, secret)
log.Info("Get Secret for cleanup", "duration_ms", time.Since(now).Milliseconds())
switch {
case err == nil:
if secret.ObjectMeta.DeletionTimestamp.IsZero() {
Expand Down Expand Up @@ -516,7 +521,7 @@ func (r *EphemeralRunnerReconciler) deletePodAsFailed(ctx context.Context, ephem
func (r *EphemeralRunnerReconciler) updateStatusWithRunnerConfig(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) (ctrl.Result, error) {
// Runner is not registered with the service. We need to register it first
log.Info("Creating ephemeral runner JIT config")
actionsClient, err := r.actionsClientFor(ctx, ephemeralRunner)
actionsClient, err := r.actionsClientFor(ctx, ephemeralRunner, log)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get actions client for generating JIT config: %v", err)
}
Expand Down Expand Up @@ -712,11 +717,13 @@ func (r *EphemeralRunnerReconciler) updateRunStatusFromPod(ctx context.Context,
return nil
}

func (r *EphemeralRunnerReconciler) actionsClientFor(ctx context.Context, runner *v1alpha1.EphemeralRunner) (actions.ActionsService, error) {
func (r *EphemeralRunnerReconciler) actionsClientFor(ctx context.Context, runner *v1alpha1.EphemeralRunner, log logr.Logger) (actions.ActionsService, error) {
now := time.Now()
secret := new(corev1.Secret)
if err := r.Get(ctx, types.NamespacedName{Namespace: runner.Namespace, Name: runner.Spec.GitHubConfigSecret}, secret); err != nil {
return nil, fmt.Errorf("failed to get secret: %w", err)
}
log.Info("Get Secret for GitHub client", "duration_ms", time.Since(now).Milliseconds())

opts, err := r.actionsClientOptionsFor(ctx, runner)
if err != nil {
Expand Down Expand Up @@ -782,7 +789,7 @@ func (r *EphemeralRunnerReconciler) actionsClientOptionsFor(ctx context.Context,
// runnerRegisteredWithService checks if the runner is still registered with the service
// Returns found=false and err=nil if ephemeral runner does not exist in GitHub service and should be deleted
func (r EphemeralRunnerReconciler) runnerRegisteredWithService(ctx context.Context, runner *v1alpha1.EphemeralRunner, log logr.Logger) (found bool, err error) {
actionsClient, err := r.actionsClientFor(ctx, runner)
actionsClient, err := r.actionsClientFor(ctx, runner, log)
if err != nil {
return false, fmt.Errorf("failed to get Actions client for ScaleSet: %w", err)
}
Expand All @@ -809,7 +816,7 @@ func (r EphemeralRunnerReconciler) runnerRegisteredWithService(ctx context.Conte
}

func (r *EphemeralRunnerReconciler) deleteRunnerFromService(ctx context.Context, ephemeralRunner *v1alpha1.EphemeralRunner, log logr.Logger) error {
client, err := r.actionsClientFor(ctx, ephemeralRunner)
client, err := r.actionsClientFor(ctx, ephemeralRunner, log)
if err != nil {
return fmt.Errorf("failed to get actions client for runner: %v", err)
}
Expand Down
11 changes: 10 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ func main() {
maxConcurrentReconcilesForEphemeralRunnerSet int
maxConcurrentReconcilesForEphemeralRunner int
maxConcurrentReconcilesForAutoscalingListener int

rateLimiterQPS int
rateLimiterBurst int
)
var c github.Config
err = envconfig.Process("github", &c)
Expand Down Expand Up @@ -156,6 +159,8 @@ func main() {
flag.IntVar(&maxConcurrentReconcilesForEphemeralRunnerSet, "max-concurrent-reconciles-for-ephemeral-runner-set", 1, "The maximum number of concurrent reconciles for EphemeralRunnerSet.")
flag.IntVar(&maxConcurrentReconcilesForEphemeralRunner, "max-concurrent-reconciles-for-ephemeral-runner", 1, "The maximum number of concurrent reconciles for EphemeralRunner.")
flag.IntVar(&maxConcurrentReconcilesForAutoscalingListener, "max-concurrent-reconciles-for-autoscaling-listener", 1, "The maximum number of concurrent reconciles for AutoscalingListener.")
flag.IntVar(&rateLimiterQPS, "client-go-rate-limiter-qps", 20, "The QPS value of client-go rate limiter.")
flag.IntVar(&rateLimiterBurst, "client-go-rate-limiter-burst", 30, "The burst value of client-go rate limiter.")
Comment on lines +162 to +163
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

flag.Parse()

runnerPodDefaults.RunnerImagePullSecrets = runnerImagePullSecrets
Expand Down Expand Up @@ -225,7 +230,11 @@ func main() {
})
}

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
cfg := ctrl.GetConfigOrDie()
cfg.QPS = float32(rateLimiterQPS)
cfg.Burst = rateLimiterBurst

mgr, err := ctrl.NewManager(cfg, ctrl.Options{
Scheme: scheme,
Metrics: metricsserver.Options{
BindAddress: metricsAddr,
Expand Down
Loading