From c3dfb396d3dbd398d8964d7ffbd3f25ad9841135 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Tue, 8 Sep 2026 12:34:45 +0200 Subject: [PATCH 1/5] Fix nil ResourceCache panic in stale scale set tests (#4628) --- .../actions.github.com/autoscalingrunnerset_controller_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index 30a30c46..efdd2df1 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -2196,6 +2196,7 @@ var _ = Describe("Test AutoscalingRunnerSet with a stale runner scale set", Orde ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( @@ -2268,6 +2269,7 @@ var _ = Describe("Test AutoscalingRunnerSet with a stale runner scale set", Orde ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( @@ -2328,6 +2330,7 @@ var _ = Describe("Test AutoscalingRunnerSet with a stale runner scale set", Orde ControllerNamespace: autoscalingNS.Name, DefaultRunnerScaleSetListenerImage: "ghcr.io/actions/arc", ResourceBuilder: ResourceBuilder{ + ResourceCache: newTestResourceCache(), SecretResolver: secretresolver.New(mgr.GetClient(), scalefake.NewMultiClient( scalefake.WithClient( scalefake.NewClient( From f0d0b1d4399189c42954262be5defb22edc94a44 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Tue, 8 Sep 2026 14:32:24 +0200 Subject: [PATCH 2/5] Add guards on timings so we don't publish incorrect metrics (#4621) --- cmd/ghalistener/metrics/metrics.go | 16 +- cmd/ghalistener/metrics/metrics_test.go | 302 ++++++++++++++++++++++++ 2 files changed, 317 insertions(+), 1 deletion(-) diff --git a/cmd/ghalistener/metrics/metrics.go b/cmd/ghalistener/metrics/metrics.go index a1bbd472..fd8021ec 100644 --- a/cmd/ghalistener/metrics/metrics.go +++ b/cmd/ghalistener/metrics/metrics.go @@ -485,9 +485,17 @@ func (e *exporter) RecordStatistics(stats *scaleset.RunnerScaleSetStatistic) { } func (e *exporter) RecordJobStarted(msg *scaleset.JobStarted) { + if msg.RunnerAssignTime.IsZero() { + return + } + l := e.startedJobLabels(msg) e.incCounter(MetricStartedJobsTotal, l) + if msg.ScaleSetAssignTime.IsZero() || msg.RunnerAssignTime.Before(msg.ScaleSetAssignTime) { + return + } + startupDuration := msg.RunnerAssignTime.Unix() - msg.ScaleSetAssignTime.Unix() e.observeHistogram(MetricJobStartupDurationSeconds, l, float64(startupDuration)) } @@ -499,7 +507,13 @@ func (e *exporter) RecordJobCompleted(msg *scaleset.JobCompleted) { l := e.completedJobLabels(msg) e.incCounter(MetricCompletedJobsTotal, l) - e.observeHistogram(MetricJobExecutionDurationSeconds, l, float64(msg.FinishTime.Unix()-msg.RunnerAssignTime.Unix())) + + if msg.FinishTime.IsZero() || msg.FinishTime.Before(msg.RunnerAssignTime) { + return + } + + finishDuration := msg.FinishTime.Unix() - msg.RunnerAssignTime.Unix() + e.observeHistogram(MetricJobExecutionDurationSeconds, l, float64(finishDuration)) } func (e *exporter) RecordDesiredRunners(count int) { diff --git a/cmd/ghalistener/metrics/metrics_test.go b/cmd/ghalistener/metrics/metrics_test.go index e62d77e7..8b23d688 100644 --- a/cmd/ghalistener/metrics/metrics_test.go +++ b/cmd/ghalistener/metrics/metrics_test.go @@ -3,9 +3,12 @@ package metrics import ( "log/slog" "testing" + "time" "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" + "github.com/actions/scaleset" "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -265,3 +268,302 @@ func TestExporterConfigDefaults(t *testing.T) { assert.Equal(t, want, config) } + +func newTestExporter(t *testing.T, metricsConfig v1alpha1.MetricsConfig) (*exporter, *prometheus.Registry) { + t.Helper() + reg := prometheus.NewRegistry() + m := installMetrics(metricsConfig, reg, discardLogger) + e := &exporter{ + scaleSetLabels: prometheus.Labels{ + labelKeyEnterprise: "test-enterprise", + labelKeyOrganization: "test-org", + labelKeyRepository: "test-repo", + labelKeyRunnerScaleSetName: "test-scale-set", + labelKeyRunnerScaleSetNamespace: "test-namespace", + }, + metrics: m, + } + return e, reg +} + +func gatherMetrics(t *testing.T, reg *prometheus.Registry) map[string]*dto.MetricFamily { + t.Helper() + mfs, err := reg.Gather() + require.NoError(t, err) + result := make(map[string]*dto.MetricFamily, len(mfs)) + for _, mf := range mfs { + result[mf.GetName()] = mf + } + return result +} + +func counterValue(t *testing.T, metrics map[string]*dto.MetricFamily, name string) float64 { + t.Helper() + mf, ok := metrics[name] + if !ok || len(mf.GetMetric()) == 0 { + return 0 + } + return mf.GetMetric()[0].GetCounter().GetValue() +} + +func histogramSampleCount(t *testing.T, metrics map[string]*dto.MetricFamily, name string) uint64 { + t.Helper() + mf, ok := metrics[name] + if !ok || len(mf.GetMetric()) == 0 { + return 0 + } + return mf.GetMetric()[0].GetHistogram().GetSampleCount() +} + +func histogramSampleSum(t *testing.T, metrics map[string]*dto.MetricFamily, name string) float64 { + t.Helper() + mf, ok := metrics[name] + if !ok || len(mf.GetMetric()) == 0 { + return 0 + } + return mf.GetMetric()[0].GetHistogram().GetSampleSum() +} + +func TestRecordJobStarted(t *testing.T) { + startedMetrics := v1alpha1.MetricsConfig{ + Counters: map[string]*v1alpha1.CounterMetric{ + MetricStartedJobsTotal: { + Labels: []string{ + labelKeyEnterprise, labelKeyOrganization, labelKeyRepository, + labelKeyJobName, labelKeyJobWorkflowRef, labelKeyJobWorkflowName, + labelKeyJobWorkflowTarget, labelKeyEventName, + }, + }, + }, + Histograms: map[string]*v1alpha1.HistogramMetric{ + MetricJobStartupDurationSeconds: { + Labels: []string{ + labelKeyEnterprise, labelKeyOrganization, labelKeyRepository, + labelKeyJobName, labelKeyJobWorkflowRef, labelKeyJobWorkflowName, + labelKeyJobWorkflowTarget, labelKeyEventName, + }, + }, + }, + } + + now := time.Now() + + tests := []struct { + name string + msg scaleset.JobStarted + wantCounterValue float64 + wantSampleCount uint64 + wantSampleSum float64 + }{ + { + name: "zero RunnerAssignTime and ScaleSetAssignTime", + msg: scaleset.JobStarted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + RunnerAssignTime: time.Time{}, + ScaleSetAssignTime: time.Time{}, + }, + }, + wantCounterValue: 0, + wantSampleCount: 0, + }, + { + name: "zero RunnerAssignTime", + msg: scaleset.JobStarted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + RunnerAssignTime: time.Time{}, + ScaleSetAssignTime: now, + }, + }, + wantCounterValue: 0, + wantSampleCount: 0, + }, + { + name: "zero ScaleSetAssignTime still counts the started job but skips the duration", + msg: scaleset.JobStarted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + RunnerAssignTime: now, + ScaleSetAssignTime: time.Time{}, + }, + }, + wantCounterValue: 1, + wantSampleCount: 0, + }, + { + name: "RunnerAssignTime before ScaleSetAssignTime still counts the started job but skips the duration", + msg: scaleset.JobStarted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + RunnerAssignTime: now, + ScaleSetAssignTime: now.Add(10 * time.Second), + }, + }, + wantCounterValue: 1, + wantSampleCount: 0, + }, + { + name: "valid timestamps", + msg: scaleset.JobStarted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + ScaleSetAssignTime: now, + RunnerAssignTime: now.Add(10 * time.Second), + }, + }, + wantCounterValue: 1, + wantSampleCount: 1, + wantSampleSum: 10, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e, reg := newTestExporter(t, startedMetrics) + e.RecordJobStarted(&tt.msg) + metrics := gatherMetrics(t, reg) + assert.Equal(t, tt.wantCounterValue, counterValue(t, metrics, MetricStartedJobsTotal)) + assert.Equal(t, tt.wantSampleCount, histogramSampleCount(t, metrics, MetricJobStartupDurationSeconds)) + assert.Equal(t, tt.wantSampleSum, histogramSampleSum(t, metrics, MetricJobStartupDurationSeconds)) + }) + } +} + +func TestRecordJobCompleted(t *testing.T) { + completedMetrics := v1alpha1.MetricsConfig{ + Counters: map[string]*v1alpha1.CounterMetric{ + MetricCompletedJobsTotal: { + Labels: []string{ + labelKeyEnterprise, labelKeyOrganization, labelKeyRepository, + labelKeyJobName, labelKeyJobWorkflowRef, labelKeyJobWorkflowName, + labelKeyJobWorkflowTarget, labelKeyEventName, labelKeyJobResult, + }, + }, + }, + Histograms: map[string]*v1alpha1.HistogramMetric{ + MetricJobExecutionDurationSeconds: { + Labels: []string{ + labelKeyEnterprise, labelKeyOrganization, labelKeyRepository, + labelKeyJobName, labelKeyJobWorkflowRef, labelKeyJobWorkflowName, + labelKeyJobWorkflowTarget, labelKeyEventName, labelKeyJobResult, + }, + }, + }, + } + + now := time.Now() + + tests := []struct { + name string + msg scaleset.JobCompleted + wantCounterValue float64 + wantSampleCount uint64 + wantSampleSum float64 + }{ + { + name: "zero RunnerAssignTime", + msg: scaleset.JobCompleted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + RunnerAssignTime: time.Time{}, + FinishTime: now, + }, + Result: "success", + }, + wantCounterValue: 0, + wantSampleCount: 0, + }, + { + name: "zero FinishTime still counts the completed job but skips the duration", + msg: scaleset.JobCompleted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + RunnerAssignTime: now, + FinishTime: time.Time{}, + }, + Result: "success", + }, + wantCounterValue: 1, + wantSampleCount: 0, + }, + { + name: "FinishTime before RunnerAssignTime still counts the completed job but skips the duration", + msg: scaleset.JobCompleted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + RunnerAssignTime: now.Add(20 * time.Second), + FinishTime: now, + }, + Result: "success", + }, + wantCounterValue: 1, + wantSampleCount: 0, + }, + { + name: "valid timestamps", + msg: scaleset.JobCompleted{ + JobMessageBase: scaleset.JobMessageBase{ + OwnerName: "myorg", + RepositoryName: "myrepo", + JobDisplayName: "build", + JobWorkflowRef: "myorg/myrepo/.github/workflows/build.yml@refs/heads/main", + EventName: "push", + // ScaleSetAssignTime represents queue-wait time and must not + // leak into the execution duration, which should only measure + // FinishTime - RunnerAssignTime. + ScaleSetAssignTime: now.Add(-1 * time.Minute), + RunnerAssignTime: now, + FinishTime: now.Add(10 * time.Second), + }, + Result: "success", + }, + wantCounterValue: 1, + wantSampleCount: 1, + wantSampleSum: 10, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e, reg := newTestExporter(t, completedMetrics) + e.RecordJobCompleted(&tt.msg) + metrics := gatherMetrics(t, reg) + assert.Equal(t, tt.wantCounterValue, counterValue(t, metrics, MetricCompletedJobsTotal)) + assert.Equal(t, tt.wantSampleCount, histogramSampleCount(t, metrics, MetricJobExecutionDurationSeconds)) + assert.Equal(t, tt.wantSampleSum, histogramSampleSum(t, metrics, MetricJobExecutionDurationSeconds)) + }) + } +} From bc677d306c437da4518614680936cce526ec4e45 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Tue, 8 Sep 2026 15:47:46 +0200 Subject: [PATCH 3/5] Add @actions/actions-runtime to codeowners (#4631) --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index 1cb5a596..7540f803 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,2 +1,2 @@ # actions-runner-controller maintainers -* @mumoshu @toast-gear @github/actions-runtime @nikola-jokic @rentziass @steve-glass +* @mumoshu @toast-gear @actions/actions-runtime @nikola-jokic @rentziass @steve-glass From 7c68e1d318f02530ca5f7e6930e9c4e7fffbc1e9 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Tue, 8 Sep 2026 15:49:58 +0200 Subject: [PATCH 4/5] Set listener qps and burst (#4558) --- .../v1alpha1/autoscalinglistener_types.go | 3 + .../v1alpha1/autoscalingrunnerset_types.go | 3 + .../v1alpha1/listenerconfig_types.go | 26 ++ .../v1alpha1/zz_generated.deepcopy.go | 55 +++ ...tions.github.com_autoscalinglisteners.yaml | 16 + ...ions.github.com_autoscalingrunnersets.yaml | 14 + ...tions.github.com_autoscalinglisteners.yaml | 16 + ...ions.github.com_autoscalingrunnersets.yaml | 14 + .../templates/_listener_template.tpl | 6 +- .../templates/autoscalingrunnserset.yaml | 60 ++- ...ling_runner_set_listener_metrics_test.yaml | 131 ++++++- ...runner_set_listener_pod_template_test.yaml | 72 +++- ...r_set_listener_scaler_validation_test.yaml | 122 ++++++ .../values.yaml | 352 +++++++++--------- .../templates/autoscalingrunnerset.yaml | 6 + .../tests/template_test.go | 165 ++++++++ .../gha-runner-scale-set/values.schema.json | 32 ++ charts/gha-runner-scale-set/values.yaml | 6 + cmd/ghalistener/config/config.go | 25 +- cmd/ghalistener/main.go | 1 + cmd/ghalistener/scaler/scaler.go | 56 ++- cmd/ghalistener/scaler/scaler_test.go | 111 ++++++ ...tions.github.com_autoscalinglisteners.yaml | 16 + ...ions.github.com_autoscalingrunnersets.yaml | 14 + .../autoscalinglistener_controller.go | 6 +- .../autoscalinglistener_controller_test.go | 57 +++ .../actions.github.com/resourcebuilder.go | 2 + 27 files changed, 1161 insertions(+), 226 deletions(-) create mode 100644 apis/actions.github.com/v1alpha1/listenerconfig_types.go create mode 100644 charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_scaler_validation_test.yaml create mode 100644 charts/gha-runner-scale-set/values.schema.json diff --git a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go index 4556c455..b1bb0136 100644 --- a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go @@ -82,6 +82,9 @@ type AutoscalingListenerSpec struct { // +optional RoleBindingMetadata *ResourceMeta `json:"roleBindingMetadata,omitempty"` + + // +optional + ListenerConfig *ListenerConfig `json:"listenerConfig,omitempty"` } func (s *AutoscalingListenerSpec) Hash() string { diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index dd1bce7f..afcb4c93 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -121,6 +121,9 @@ type AutoscalingRunnerSetSpec struct { // +optional // +kubebuilder:validation:Minimum:=0 MinRunners *int `json:"minRunners,omitempty"` + + // +optional + ListenerConfig *ListenerConfig `json:"listenerConfig,omitempty"` } type TLSConfig struct { diff --git a/apis/actions.github.com/v1alpha1/listenerconfig_types.go b/apis/actions.github.com/v1alpha1/listenerconfig_types.go new file mode 100644 index 00000000..227114a6 --- /dev/null +++ b/apis/actions.github.com/v1alpha1/listenerconfig_types.go @@ -0,0 +1,26 @@ +package v1alpha1 + +// ListenerConfig holds configuration for the ghalistener pod. +type ListenerConfig struct { + // +optional + Scaler *ScalerConfig `json:"scaler,omitempty"` +} + +// GetScaler returns the ScalerConfig, or nil if not set. +func (c *ListenerConfig) GetScaler() *ScalerConfig { + if c == nil { + return nil + } + return c.Scaler +} + +// ScalerConfig configures the Kubernetes client used by the ghalistener scaler. +type ScalerConfig struct { + // +optional + // +kubebuilder:validation:Minimum:=1 + QPS *int `json:"qps,omitempty"` + + // +optional + // +kubebuilder:validation:Minimum:=1 + Burst *int `json:"burst,omitempty"` +} diff --git a/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go b/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go index 0b5c34cb..464ec734 100644 --- a/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go +++ b/apis/actions.github.com/v1alpha1/zz_generated.deepcopy.go @@ -138,6 +138,11 @@ func (in *AutoscalingListenerSpec) DeepCopyInto(out *AutoscalingListenerSpec) { *out = new(ResourceMeta) (*in).DeepCopyInto(*out) } + if in.ListenerConfig != nil { + in, out := &in.ListenerConfig, &out.ListenerConfig + *out = new(ListenerConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AutoscalingListenerSpec. @@ -308,6 +313,11 @@ func (in *AutoscalingRunnerSetSpec) DeepCopyInto(out *AutoscalingRunnerSetSpec) *out = new(int) **out = **in } + if in.ListenerConfig != nil { + in, out := &in.ListenerConfig, &out.ListenerConfig + *out = new(ListenerConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AutoscalingRunnerSetSpec. @@ -627,6 +637,26 @@ func (in *HistogramMetric) DeepCopy() *HistogramMetric { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ListenerConfig) DeepCopyInto(out *ListenerConfig) { + *out = *in + if in.Scaler != nil { + in, out := &in.Scaler, &out.Scaler + *out = new(ScalerConfig) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ListenerConfig. +func (in *ListenerConfig) DeepCopy() *ListenerConfig { + if in == nil { + return nil + } + out := new(ListenerConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MetricsConfig) DeepCopyInto(out *MetricsConfig) { *out = *in @@ -764,6 +794,31 @@ func (in *ResourceMeta) DeepCopy() *ResourceMeta { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ScalerConfig) DeepCopyInto(out *ScalerConfig) { + *out = *in + if in.QPS != nil { + in, out := &in.QPS, &out.QPS + *out = new(int) + **out = **in + } + if in.Burst != nil { + in, out := &in.Burst, &out.Burst + *out = new(int) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ScalerConfig. +func (in *ScalerConfig) DeepCopy() *ScalerConfig { + if in == nil { + return nil + } + out := new(ScalerConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TLSCertificateSource) DeepCopyInto(out *TLSCertificateSource) { *out = *in diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml index 29cc9bb4..820a55f1 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml @@ -123,6 +123,22 @@ spec: type: object x-kubernetes-map-type: atomic type: array + listenerConfig: + description: ListenerConfig holds configuration for the ghalistener + pod. + properties: + scaler: + description: ScalerConfig configures the Kubernetes client used + by the ghalistener scaler. + properties: + burst: + minimum: 1 + type: integer + qps: + minimum: 1 + type: integer + type: object + type: object maxRunners: minimum: 0 type: integer diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml index a2dfd46d..8b78ce24 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml @@ -145,6 +145,20 @@ spec: x-kubernetes-map-type: atomic type: object type: object + listenerConfig: + description: ListenerConfig holds configuration for the ghalistener pod. + properties: + scaler: + description: ScalerConfig configures the Kubernetes client used by the ghalistener scaler. + properties: + burst: + minimum: 1 + type: integer + qps: + minimum: 1 + type: integer + type: object + type: object listenerConfigSecretMetadata: description: ResourceMeta carries metadata common to all internal resources properties: diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml index 29cc9bb4..820a55f1 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml @@ -123,6 +123,22 @@ spec: type: object x-kubernetes-map-type: atomic type: array + listenerConfig: + description: ListenerConfig holds configuration for the ghalistener + pod. + properties: + scaler: + description: ScalerConfig configures the Kubernetes client used + by the ghalistener scaler. + properties: + burst: + minimum: 1 + type: integer + qps: + minimum: 1 + type: integer + type: object + type: object maxRunners: minimum: 0 type: integer diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml index a2dfd46d..8b78ce24 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml @@ -145,6 +145,20 @@ spec: x-kubernetes-map-type: atomic type: object type: object + listenerConfig: + description: ListenerConfig holds configuration for the ghalistener pod. + properties: + scaler: + description: ScalerConfig configures the Kubernetes client used by the ghalistener scaler. + properties: + burst: + minimum: 1 + type: integer + qps: + minimum: 1 + type: integer + type: object + type: object listenerConfigSecretMetadata: description: ResourceMeta carries metadata common to all internal resources properties: diff --git a/charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl b/charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl index f3f97a4c..5236a2ce 100644 --- a/charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl +++ b/charts/gha-runner-scale-set-experimental/templates/_listener_template.tpl @@ -1,8 +1,8 @@ {{- define "listener-template.pod" -}} -{{- $metadata := .Values.listenerPodTemplate.metadata | default dict -}} -{{- $spec := .Values.listenerPodTemplate.spec | default dict -}} +{{- $metadata := .metadata | default dict -}} +{{- $spec := .spec | default dict -}} {{- if and (empty $metadata) (empty $spec) -}} - {{- fail "listenerPodTemplate must have at least metadata or spec defined" -}} + {{- fail ".Values.listener.podTemplate must have at least metadata or spec defined" -}} {{- end -}} {{- with $metadata -}} metadata: diff --git a/charts/gha-runner-scale-set-experimental/templates/autoscalingrunnserset.yaml b/charts/gha-runner-scale-set-experimental/templates/autoscalingrunnserset.yaml index 6e2cba35..d42e59f2 100644 --- a/charts/gha-runner-scale-set-experimental/templates/autoscalingrunnserset.yaml +++ b/charts/gha-runner-scale-set-experimental/templates/autoscalingrunnserset.yaml @@ -5,6 +5,55 @@ {{- $kubeDefaults := (index $kubeMode "default" | default true) }} {{- $kubeServiceAccountName := (index $kubeMode "serviceAccountName" | default "") }} {{- $usesKubernetesSecrets := or (not .Values.secretResolution) (eq .Values.secretResolution.type "kubernetes") }} +{{- /* All listener configuration lives under .Values.listener */ -}} +{{- if hasKey .Values "listenerMetrics" }} + {{- fail ".Values.listenerMetrics has moved to .Values.listener.metrics" }} +{{- end }} +{{- if hasKey .Values "listenerPodTemplate" }} + {{- fail ".Values.listenerPodTemplate has moved to .Values.listener.podTemplate" }} +{{- end }} +{{- $listener := .Values.listener | default dict }} +{{- if not (kindIs "map" $listener) }} + {{- fail ".Values.listener must be an object" }} +{{- end }} +{{- $listenerKeys := list "metrics" "podTemplate" "scaler" }} +{{- range $name, $_ := $listener }} + {{- if not (has $name $listenerKeys) }} + {{- fail (printf ".Values.listener.%s is not a supported key; supported keys are: %s" $name (join ", " $listenerKeys)) }} + {{- end }} +{{- end }} +{{- range $name := list "metrics" "podTemplate" }} + {{- $value := index $listener $name | default dict }} + {{- if not (kindIs "map" $value) }} + {{- fail (printf ".Values.listener.%s must be an object" $name) }} + {{- end }} +{{- end }} +{{- $scaler := $listener.scaler | default dict }} +{{- if not (kindIs "map" $scaler) }} + {{- fail ".Values.listener.scaler must be an object" }} +{{- end }} +{{- $scalerKeys := list "burst" "qps" }} +{{- range $name, $_ := $scaler }} + {{- if not (has $name $scalerKeys) }} + {{- fail (printf ".Values.listener.scaler.%s is not a supported key; supported keys are: %s" $name (join ", " $scalerKeys)) }} + {{- end }} +{{- end }} +{{- range $name := $scalerKeys }} + {{- if hasKey $scaler $name }} + {{- $value := index $scaler $name }} + {{- if not (or (kindIs "int" $value) (kindIs "int64" $value) (kindIs "float64" $value)) }} + {{- fail (printf ".Values.listener.scaler.%s must be an integer greater than 0" $name) }} + {{- else }} + {{- $number := float64 $value }} + {{- if ne $number (floor $number) }} + {{- fail (printf ".Values.listener.scaler.%s must be an integer greater than 0" $name) }} + {{- else if lt $number (float64 1) }} + {{- fail (printf ".Values.listener.scaler.%s must be greater than 0" $name) }} + {{- end }} + {{- end }} + {{- end }} +{{- end }} +{{- $listenerPodTemplate := $listener.podTemplate | default dict }} {{- $runnerPod := (index $runner "pod" | default dict) -}} {{- if not (kindIs "map" $runnerPod) -}} @@ -165,12 +214,17 @@ spec: minRunners: {{ .Values.scaleset.minRunners | int }} {{- end }} - {{- if and .Values.listenerPodTemplate (or .Values.listenerPodTemplate.metadata .Values.listenerPodTemplate.spec) }} + {{- if or $listenerPodTemplate.metadata $listenerPodTemplate.spec }} listenerTemplate: - {{- include "listener-template.pod" . | nindent 4}} + {{- include "listener-template.pod" $listenerPodTemplate | nindent 4}} {{- end }} - {{- with .Values.listenerMetrics }} + {{- with $listener.scaler }} + listenerConfig: + scaler: + {{- toYaml . | nindent 6 }} + {{- end }} + {{- with $listener.metrics }} listenerMetrics: {{- toYaml . | nindent 4 }} {{- end }} diff --git a/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_metrics_test.yaml b/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_metrics_test.yaml index 3a2b4ff8..eb6f9478 100644 --- a/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_metrics_test.yaml +++ b/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_metrics_test.yaml @@ -1,7 +1,111 @@ -suite: "Test AutoscalingRunnerSet Listener Metrics" +suite: "Test AutoscalingRunnerSet Listener" templates: - autoscalingrunnserset.yaml tests: + - it: should render default scaler values when listener.scaler is not overridden + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: spec.listenerConfig.scaler.qps + value: 50 + - equal: + path: spec.listenerConfig.scaler.burst + value: 100 + + - it: should render custom scaler values when overridden + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + listener: + scaler: + qps: 100 + burst: 200 + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: spec.listenerConfig.scaler.qps + value: 100 + - equal: + path: spec.listenerConfig.scaler.burst + value: 200 + + - it: should render qps override while keeping default burst + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + listener: + scaler: + qps: 75 + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: spec.listenerConfig.scaler.qps + value: 75 + - equal: + path: spec.listenerConfig.scaler.burst + value: 100 + + - it: should render scaler and metrics together + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + listener: + scaler: + qps: 75 + metrics: + counters: + gha_started_jobs_total: + labels: + - repository + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: spec.listenerConfig.scaler.qps + value: 75 + - exists: + path: spec.listenerMetrics + - equal: + path: spec.listenerMetrics.counters.gha_started_jobs_total.labels[0] + value: repository + + - it: should not render listenerConfig when listener is null + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + listener: null + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - notExists: + path: spec.listenerConfig + - it: should not render listenerMetrics when not configured set: scaleset.name: "test" @@ -23,18 +127,19 @@ tests: auth.githubToken: "gh_token12345" controllerServiceAccount.name: "arc" controllerServiceAccount.namespace: "arc-system" - listenerMetrics: - counters: - gha_started_jobs_total: - labels: - - repository - - organization - histograms: - gha_job_startup_duration_seconds: - buckets: - - 0.1 - - 1 - - 2.5 + listener: + metrics: + counters: + gha_started_jobs_total: + labels: + - repository + - organization + histograms: + gha_job_startup_duration_seconds: + buckets: + - 0.1 + - 1 + - 2.5 release: name: "test-name" namespace: "test-namespace" diff --git a/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_pod_template_test.yaml b/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_pod_template_test.yaml index 4e02866c..7976c0e1 100644 --- a/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_pod_template_test.yaml +++ b/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_pod_template_test.yaml @@ -1,21 +1,22 @@ -suite: "AutoscalingRunnerSet listenerPodTemplate" +suite: "AutoscalingRunnerSet listener.podTemplate" templates: - autoscalingrunnserset.yaml tests: - - it: should render listenerTemplate from listenerPodTemplate + - it: should render listenerTemplate from listener.podTemplate set: scaleset.name: "test" auth.url: "https://github.com/org" auth.githubToken: "gh_token12345" controllerServiceAccount.name: "arc" controllerServiceAccount.namespace: "arc-system" - listenerPodTemplate: - spec: - containers: - - name: listener - image: "ghcr.io/actions/actions-runner-controller/actionsmetricsserver:latest" - securityContext: - runAsUser: 1000 + listener: + podTemplate: + spec: + containers: + - name: listener + image: "ghcr.io/actions/actions-runner-controller/actionsmetricsserver:latest" + securityContext: + runAsUser: 1000 release: name: "test-name" namespace: "test-namespace" @@ -34,9 +35,10 @@ tests: auth.githubToken: "gh_token12345" controllerServiceAccount.name: "arc" controllerServiceAccount.namespace: "arc-system" - listenerPodTemplate: - spec: - restartPolicy: Always + listener: + podTemplate: + spec: + restartPolicy: Always release: name: "test-name" namespace: "test-namespace" @@ -44,3 +46,49 @@ tests: - equal: path: spec.listenerTemplate.spec.containers[0].name value: listener + + - it: should render podTemplate alongside scaler and metrics + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + listener: + scaler: + qps: 75 + podTemplate: + spec: + restartPolicy: Always + metrics: + counters: + gha_started_jobs_total: + labels: + - repository + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - equal: + path: spec.listenerConfig.scaler.qps + value: 75 + - equal: + path: spec.listenerTemplate.spec.containers[0].name + value: listener + - equal: + path: spec.listenerMetrics.counters.gha_started_jobs_total.labels[0] + value: repository + + - it: should not render listenerTemplate when podTemplate is empty + set: + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" + release: + name: "test-name" + namespace: "test-namespace" + asserts: + - notExists: + path: spec.listenerTemplate diff --git a/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_scaler_validation_test.yaml b/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_scaler_validation_test.yaml new file mode 100644 index 00000000..1450d9b1 --- /dev/null +++ b/charts/gha-runner-scale-set-experimental/tests/autoscaling_runner_set_listener_scaler_validation_test.yaml @@ -0,0 +1,122 @@ +suite: "Test AutoscalingRunnerSet Listener Validation" +templates: + - autoscalingrunnserset.yaml +set: &base + scaleset.name: "test" + auth.url: "https://github.com/org" + auth.githubToken: "gh_token12345" + controllerServiceAccount.name: "arc" + controllerServiceAccount.namespace: "arc-system" +tests: + - it: rejects zero qps + set: + <<: *base + listener.scaler.qps: 0 + asserts: + - failedTemplate: + errorMessage: ".Values.listener.scaler.qps must be greater than 0" + + - it: rejects negative burst + set: + <<: *base + listener.scaler.burst: -1 + asserts: + - failedTemplate: + errorMessage: ".Values.listener.scaler.burst must be greater than 0" + + - it: rejects fractional qps + set: + <<: *base + listener.scaler.qps: 1.5 + asserts: + - failedTemplate: + errorMessage: ".Values.listener.scaler.qps must be an integer greater than 0" + + - it: rejects a string burst + set: + <<: *base + listener.scaler.burst: "100" + asserts: + - failedTemplate: + errorMessage: ".Values.listener.scaler.burst must be an integer greater than 0" + + - it: rejects a boolean qps + set: + <<: *base + listener.scaler.qps: true + asserts: + - failedTemplate: + errorMessage: ".Values.listener.scaler.qps must be an integer greater than 0" + + - it: rejects a scaler that is not an object + set: + <<: *base + listener.scaler: "nope" + asserts: + - failedTemplate: + errorMessage: ".Values.listener.scaler must be an object" + + - it: rejects a listener that is not an object + set: + <<: *base + listener: "nope" + asserts: + - failedTemplate: + errorMessage: ".Values.listener must be an object" + + - it: rejects a metrics value that is not an object + set: + <<: *base + listener.metrics: "nope" + asserts: + - failedTemplate: + errorMessage: ".Values.listener.metrics must be an object" + + - it: rejects a podTemplate that is not an object + set: + <<: *base + listener.podTemplate: "nope" + asserts: + - failedTemplate: + errorMessage: ".Values.listener.podTemplate must be an object" + + - it: rejects a misspelled key under listener + set: + <<: *base + listener.metrcs.counters: {} + asserts: + - failedTemplate: + errorMessage: ".Values.listener.metrcs is not a supported key; supported keys are: metrics, podTemplate, scaler" + + - it: rejects a misspelled key under listener.scaler + set: + <<: *base + listener.scaler.qbs: 10 + asserts: + - failedTemplate: + errorMessage: ".Values.listener.scaler.qbs is not a supported key; supported keys are: burst, qps" + + - it: rejects the legacy top-level listenerMetrics key + set: + <<: *base + listenerMetrics.counters: {} + asserts: + - failedTemplate: + errorMessage: ".Values.listenerMetrics has moved to .Values.listener.metrics" + + - it: rejects the legacy top-level listenerPodTemplate key + set: + <<: *base + listenerPodTemplate.spec.restartPolicy: Always + asserts: + - failedTemplate: + errorMessage: ".Values.listenerPodTemplate has moved to .Values.listener.podTemplate" + + - it: treats a podTemplate with neither metadata nor spec as a no-op + set: + <<: *base + listener.podTemplate.spec: {} + listener.podTemplate.metadata: {} + asserts: + - notExists: + path: spec.listenerTemplate diff --git a/charts/gha-runner-scale-set-experimental/values.yaml b/charts/gha-runner-scale-set-experimental/values.yaml index 41dc900a..b6698c6d 100644 --- a/charts/gha-runner-scale-set-experimental/values.yaml +++ b/charts/gha-runner-scale-set-experimental/values.yaml @@ -75,23 +75,6 @@ secretResolution: # - example.com # - example.org - ## listenerTemplate is the PodSpec for each listener Pod - ## For reference: https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#PodSpec - # listenerPodTemplate: - # spec: - # containers: - # # Use this section to append additional configuration to the listener container. - # # If you change the name of the container, the configuration will not be applied to the listener, - # # and it will be treated as a side-car container. - # - name: listener - # securityContext: - # runAsUser: 1000 - # # Use this section to add the configuration of a side-car container. - # # Comment it out or remove it if you don't need it. - # # Spec for this container will be applied as is without any modifications. - # - name: side-car - # image: example-sidecar - ## Resource object allows modifying resources created by the chart itself resource: # Specifies metadata that will be applied to all resources managed by ARC @@ -312,158 +295,183 @@ controllerServiceAccount: namespace: "" name: "" -## listenerMetrics are configurable metrics applied to the listener. -## In order to avoid helm merging these fields, we left the metrics commented out. -## When configuring metrics, please uncomment the listenerMetrics object below. -## You can modify the configuration to remove the label or specify custom buckets for histogram. -## -## If the buckets field is not specified, the default buckets will be applied. Default buckets are -## provided here for documentation purposes -# listenerMetrics: -# counters: -# gha_started_jobs_total: -# labels: -# ["repository", "organization", "enterprise", "job_name", "event_name", "job_workflow_ref", "job_workflow_name", "job_workflow_target"] -# gha_completed_jobs_total: -# labels: -# [ -# "repository", -# "organization", -# "enterprise", -# "job_name", -# "event_name", -# "job_result", -# "job_workflow_ref", -# "job_workflow_name", -# "job_workflow_target", -# ] -# gauges: -# gha_assigned_jobs: -# labels: ["name", "namespace", "repository", "organization", "enterprise"] -# gha_running_jobs: -# labels: ["name", "namespace", "repository", "organization", "enterprise"] -# gha_registered_runners: -# labels: ["name", "namespace", "repository", "organization", "enterprise"] -# gha_busy_runners: -# labels: ["name", "namespace", "repository", "organization", "enterprise"] -# gha_min_runners: -# labels: ["name", "namespace", "repository", "organization", "enterprise"] -# gha_max_runners: -# labels: ["name", "namespace", "repository", "organization", "enterprise"] -# gha_desired_runners: -# labels: ["name", "namespace", "repository", "organization", "enterprise"] -# gha_idle_runners: -# labels: ["name", "namespace", "repository", "organization", "enterprise"] -# histograms: -# gha_job_startup_duration_seconds: -# labels: -# ["repository", "organization", "enterprise", "job_name", "event_name","job_workflow_ref", "job_workflow_name", "job_workflow_target"] -# buckets: -# [ -# 0.01, -# 0.05, -# 0.1, -# 0.5, -# 1.0, -# 2.0, -# 3.0, -# 4.0, -# 5.0, -# 6.0, -# 7.0, -# 8.0, -# 9.0, -# 10.0, -# 12.0, -# 15.0, -# 18.0, -# 20.0, -# 25.0, -# 30.0, -# 40.0, -# 50.0, -# 60.0, -# 70.0, -# 80.0, -# 90.0, -# 100.0, -# 110.0, -# 120.0, -# 150.0, -# 180.0, -# 210.0, -# 240.0, -# 300.0, -# 360.0, -# 420.0, -# 480.0, -# 540.0, -# 600.0, -# 900.0, -# 1200.0, -# 1800.0, -# 2400.0, -# 3000.0, -# 3600.0, -# ] -# gha_job_execution_duration_seconds: -# labels: -# [ -# "repository", -# "organization", -# "enterprise", -# "job_name", -# "event_name", -# "job_result", -# "job_workflow_ref", -# "job_workflow_name", -# "job_workflow_target" -# ] -# buckets: -# [ -# 0.01, -# 0.05, -# 0.1, -# 0.5, -# 1.0, -# 2.0, -# 3.0, -# 4.0, -# 5.0, -# 6.0, -# 7.0, -# 8.0, -# 9.0, -# 10.0, -# 12.0, -# 15.0, -# 18.0, -# 20.0, -# 25.0, -# 30.0, -# 40.0, -# 50.0, -# 60.0, -# 70.0, -# 80.0, -# 90.0, -# 100.0, -# 110.0, -# 120.0, -# 150.0, -# 180.0, -# 210.0, -# 240.0, -# 300.0, -# 360.0, -# 420.0, -# 480.0, -# 540.0, -# 600.0, -# 900.0, -# 1200.0, -# 1800.0, -# 2400.0, -# 3000.0, -# 3600.0, -# ] +# listener specific configuration. This configuration is applied to the listener component of the chart. +listener: + # scaler is config applied to the kubernetes client component of the listener. + # Both values must be integers greater than 0. + scaler: + # qps is the sustained rate of requests the listener may issue to the Kubernetes API server. + qps: 50 + # burst is the number of requests the listener may issue to the Kubernetes API server in a burst. + burst: 100 + ## podTemplate is the PodSpec for each listener Pod + ## For reference: https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#PodSpec + # podTemplate: + # spec: + # containers: + # # Use this section to append additional configuration to the listener container. + # # If you change the name of the container, the configuration will not be applied to the listener, + # # and it will be treated as a side-car container. + # - name: listener + # securityContext: + # runAsUser: 1000 + # # Use this section to add the configuration of a side-car container. + # # Comment it out or remove it if you don't need it. + # # Spec for this container will be applied as is without any modifications. + # - name: side-car + # image: example-sidecar + ## metrics configuration for the listener. + ## In order to avoid helm merging these fields, we left the metrics commented out. + ## When configuring metrics, please uncomment the metrics object below. + ## You can modify the configuration to remove the label or specify custom buckets for histogram. + ## + ## If the buckets field is not specified, the default buckets will be applied. Default buckets are + ## provided here for documentation purposes + # metrics: + # counters: + # gha_started_jobs_total: + # labels: + # ["repository", "organization", "enterprise", "job_name", "event_name", "job_workflow_ref", "job_workflow_name", "job_workflow_target"] + # gha_completed_jobs_total: + # labels: + # [ + # "repository", + # "organization", + # "enterprise", + # "job_name", + # "event_name", + # "job_result", + # "job_workflow_ref", + # "job_workflow_name", + # "job_workflow_target", + # ] + # gauges: + # gha_assigned_jobs: + # labels: ["name", "namespace", "repository", "organization", "enterprise"] + # gha_running_jobs: + # labels: ["name", "namespace", "repository", "organization", "enterprise"] + # gha_registered_runners: + # labels: ["name", "namespace", "repository", "organization", "enterprise"] + # gha_busy_runners: + # labels: ["name", "namespace", "repository", "organization", "enterprise"] + # gha_min_runners: + # labels: ["name", "namespace", "repository", "organization", "enterprise"] + # gha_max_runners: + # labels: ["name", "namespace", "repository", "organization", "enterprise"] + # gha_desired_runners: + # labels: ["name", "namespace", "repository", "organization", "enterprise"] + # gha_idle_runners: + # labels: ["name", "namespace", "repository", "organization", "enterprise"] + # histograms: + # gha_job_startup_duration_seconds: + # labels: + # ["repository", "organization", "enterprise", "job_name", "event_name","job_workflow_ref", "job_workflow_name", "job_workflow_target"] + # buckets: + # [ + # 0.01, + # 0.05, + # 0.1, + # 0.5, + # 1.0, + # 2.0, + # 3.0, + # 4.0, + # 5.0, + # 6.0, + # 7.0, + # 8.0, + # 9.0, + # 10.0, + # 12.0, + # 15.0, + # 18.0, + # 20.0, + # 25.0, + # 30.0, + # 40.0, + # 50.0, + # 60.0, + # 70.0, + # 80.0, + # 90.0, + # 100.0, + # 110.0, + # 120.0, + # 150.0, + # 180.0, + # 210.0, + # 240.0, + # 300.0, + # 360.0, + # 420.0, + # 480.0, + # 540.0, + # 600.0, + # 900.0, + # 1200.0, + # 1800.0, + # 2400.0, + # 3000.0, + # 3600.0, + # ] + # gha_job_execution_duration_seconds: + # labels: + # [ + # "repository", + # "organization", + # "enterprise", + # "job_name", + # "event_name", + # "job_result", + # "job_workflow_ref", + # "job_workflow_name", + # "job_workflow_target" + # ] + # buckets: + # [ + # 0.01, + # 0.05, + # 0.1, + # 0.5, + # 1.0, + # 2.0, + # 3.0, + # 4.0, + # 5.0, + # 6.0, + # 7.0, + # 8.0, + # 9.0, + # 10.0, + # 12.0, + # 15.0, + # 18.0, + # 20.0, + # 25.0, + # 30.0, + # 40.0, + # 50.0, + # 60.0, + # 70.0, + # 80.0, + # 90.0, + # 100.0, + # 110.0, + # 120.0, + # 150.0, + # 180.0, + # 210.0, + # 240.0, + # 300.0, + # 360.0, + # 420.0, + # 480.0, + # 540.0, + # 600.0, + # 900.0, + # 1200.0, + # 1800.0, + # 2400.0, + # 3000.0, + # 3600.0, + # ] diff --git a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml index 45817de2..b75e0d76 100644 --- a/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml +++ b/charts/gha-runner-scale-set/templates/autoscalingrunnerset.yaml @@ -1,5 +1,6 @@ {{- $resourceMeta := default (dict) .Values.resourceMeta }} {{- $hasCustomResourceMeta := (and .Values.resourceMeta .Values.resourceMeta.autoscalingRunnerSet) }} +{{- /* .Values.listenerConfig is validated by values.schema.json */ -}} apiVersion: actions.github.com/v1alpha1 kind: AutoscalingRunnerSet metadata: @@ -167,6 +168,11 @@ spec: {{- toYaml . | nindent 4 }} {{- end }} + {{- with .Values.listenerConfig }} + listenerConfig: + {{- toYaml . | nindent 4 }} + {{- end }} + {{- with (index $resourceMeta "autoscalingListener") }} autoscalingListener: {{- include "gha-runner-scale-set.resourceMetaSpec" . | nindent 4 }} diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index e9aa4c9b..920bc8bf 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/utils/ptr" ) func TestTemplateRenderedGitHubSecretWithGitHubToken(t *testing.T) { @@ -141,6 +142,170 @@ func TestTemplateRenderedGitHubSecretErrorWithMissingAppInput(t *testing.T) { assert.ErrorContains(t, err, "provide .Values.githubConfigSecret.github_app_installation_id and .Values.githubConfigSecret.github_app_private_key") } +func TestTemplateListenerScalerValidation(t *testing.T) { + t.Parallel() + + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + baseValues := map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret.github_token": "gh_token12345", + "controllerServiceAccount.name": "arc", + "controllerServiceAccount.namespace": "arc-system", + } + tests := []struct { + name string + setValues map[string]string + setStrValues map[string]string + wantErrorText string + }{ + { + name: "zero qps", + setValues: map[string]string{"listenerConfig.scaler.qps": "0"}, + wantErrorText: "at '/listenerConfig/scaler/qps': minimum: got 0, want 1", + }, + { + name: "negative burst", + setValues: map[string]string{"listenerConfig.scaler.burst": "-1"}, + wantErrorText: "at '/listenerConfig/scaler/burst': minimum: got -1, want 1", + }, + { + name: "fractional qps", + setValues: map[string]string{"listenerConfig.scaler.qps": "1.5"}, + wantErrorText: "at '/listenerConfig/scaler/qps'", + }, + { + name: "string burst", + setStrValues: map[string]string{"listenerConfig.scaler.burst": "100"}, + wantErrorText: "at '/listenerConfig/scaler/burst': got string, want integer", + }, + { + name: "boolean qps", + setValues: map[string]string{"listenerConfig.scaler.qps": "true"}, + wantErrorText: "at '/listenerConfig/scaler/qps': got boolean, want integer", + }, + { + name: "scaler is not an object", + setValues: map[string]string{"listenerConfig.scaler[0]": "a"}, + wantErrorText: "at '/listenerConfig/scaler': got array, want null or object", + }, + { + name: "listenerConfig is not an object", + setStrValues: map[string]string{"listenerConfig": "foo"}, + wantErrorText: "at '/listenerConfig': got string, want null or object", + }, + { + name: "misspelled scaler key", + setValues: map[string]string{"listenerConfig.scalar.qps": "5"}, + wantErrorText: "at '/listenerConfig': additional properties 'scalar' not allowed", + }, + { + name: "unknown key under scaler", + setStrValues: map[string]string{"listenerConfig.scaler.foo": "bar"}, + wantErrorText: "at '/listenerConfig/scaler': additional properties 'foo' not allowed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setValues := make(map[string]string, len(baseValues)+len(tt.setValues)) + for key, value := range baseValues { + setValues[key] = value + } + for key, value := range tt.setValues { + setValues[key] = value + } + + options := &helm.Options{ + Logger: logger.Discard, + SetValues: setValues, + SetStrValues: tt.setStrValues, + KubectlOptions: k8s.NewKubectlOptions("", "", "test"), + } + _, err := helm.RenderTemplateContextE(t, t.Context(), options, helmChartPath, "test-runners", []string{"templates/autoscalingrunnerset.yaml"}) + require.Error(t, err) + assert.ErrorContains(t, err, tt.wantErrorText) + }) + } +} + +func TestTemplateListenerScalerConfig(t *testing.T) { + t.Parallel() + + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set") + require.NoError(t, err) + + baseValues := map[string]string{ + "githubConfigUrl": "https://github.com/actions", + "githubConfigSecret.github_token": "gh_token12345", + "controllerServiceAccount.name": "arc", + "controllerServiceAccount.namespace": "arc-system", + } + tests := []struct { + name string + setValues map[string]string + wantQPS *int + wantBurst *int + }{ + { + name: "defaults from values.yaml", + wantQPS: ptr.To(50), + wantBurst: ptr.To(100), + }, + { + name: "both overridden", + setValues: map[string]string{"listenerConfig.scaler.qps": "100", "listenerConfig.scaler.burst": "200"}, + wantQPS: ptr.To(100), + wantBurst: ptr.To(200), + }, + { + name: "qps overridden keeps default burst", + setValues: map[string]string{"listenerConfig.scaler.qps": "75"}, + wantQPS: ptr.To(75), + wantBurst: ptr.To(100), + }, + { + name: "listenerConfig disabled", + setValues: map[string]string{"listenerConfig": "null"}, + wantQPS: nil, + wantBurst: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setValues := make(map[string]string, len(baseValues)+len(tt.setValues)) + for key, value := range baseValues { + setValues[key] = value + } + for key, value := range tt.setValues { + setValues[key] = value + } + + options := &helm.Options{ + Logger: logger.Discard, + SetValues: setValues, + KubectlOptions: k8s.NewKubectlOptions("", "", "test"), + } + output := helm.RenderTemplateContext(t, t.Context(), options, helmChartPath, "test-runners", []string{"templates/autoscalingrunnerset.yaml"}) + + var ars v1alpha1.AutoscalingRunnerSet + helm.UnmarshalK8SYaml(t, output, &ars) + + if tt.wantQPS == nil && tt.wantBurst == nil { + assert.Nil(t, ars.Spec.ListenerConfig.GetScaler()) + return + } + + scaler := ars.Spec.ListenerConfig.GetScaler() + require.NotNil(t, scaler) + assert.Equal(t, tt.wantQPS, scaler.QPS) + assert.Equal(t, tt.wantBurst, scaler.Burst) + }) + } +} + func TestTemplateNotRenderedGitHubSecretWithPredefinedSecret(t *testing.T) { t.Parallel() diff --git a/charts/gha-runner-scale-set/values.schema.json b/charts/gha-runner-scale-set/values.schema.json new file mode 100644 index 00000000..13505d5e --- /dev/null +++ b/charts/gha-runner-scale-set/values.schema.json @@ -0,0 +1,32 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "gha-runner-scale-set values", + "description": "Only values that require strict validation are described here. Any value not listed is intentionally unconstrained.", + "type": "object", + "properties": { + "listenerConfig": { + "description": "Configuration for the ghalistener pod.", + "type": ["object", "null"], + "additionalProperties": false, + "properties": { + "scaler": { + "description": "Configuration for the Kubernetes client used by the ghalistener scaler.", + "type": ["object", "null"], + "additionalProperties": false, + "properties": { + "qps": { + "description": "Queries per second the listener may issue to the Kubernetes API server.", + "type": "integer", + "minimum": 1 + }, + "burst": { + "description": "Burst of queries the listener may issue to the Kubernetes API server.", + "type": "integer", + "minimum": 1 + } + } + } + } + } + } +} diff --git a/charts/gha-runner-scale-set/values.yaml b/charts/gha-runner-scale-set/values.yaml index 4b4640cf..043cde00 100644 --- a/charts/gha-runner-scale-set/values.yaml +++ b/charts/gha-runner-scale-set/values.yaml @@ -146,6 +146,12 @@ githubConfigSecret: # - name: side-car # image: example-sidecar +## listenerConfig holds configuration for the ghalistener pod. +listenerConfig: + scaler: + qps: 50 + burst: 100 + ## listenerMetrics are configurable metrics applied to the listener. ## In order to avoid helm merging these fields, we left the metrics commented out. ## When configuring metrics, please uncomment the listenerMetrics object below. diff --git a/cmd/ghalistener/config/config.go b/cmd/ghalistener/config/config.go index 69cfafb8..0726e4ce 100644 --- a/cmd/ghalistener/config/config.go +++ b/cmd/ghalistener/config/config.go @@ -32,18 +32,19 @@ type Config struct { // It is initially set to nil if VaultType is set. // Otherwise, it is populated with the GitHub App credentials from the GitHub secret. *appconfig.AppConfig - EphemeralRunnerSetNamespace string `json:"ephemeral_runner_set_namespace"` - EphemeralRunnerSetName string `json:"ephemeral_runner_set_name"` - MaxRunners int `json:"max_runners"` - MinRunners int `json:"min_runners"` - RunnerScaleSetID int `json:"runner_scale_set_id"` - RunnerScaleSetName string `json:"runner_scale_set_name"` - ServerRootCA string `json:"server_root_ca"` - LogLevel string `json:"log_level"` - LogFormat string `json:"log_format"` - MetricsAddr string `json:"metrics_addr"` - MetricsEndpoint string `json:"metrics_endpoint"` - Metrics *v1alpha1.MetricsConfig `json:"metrics"` + EphemeralRunnerSetNamespace string `json:"ephemeral_runner_set_namespace"` + EphemeralRunnerSetName string `json:"ephemeral_runner_set_name"` + MaxRunners int `json:"max_runners"` + MinRunners int `json:"min_runners"` + RunnerScaleSetID int `json:"runner_scale_set_id"` + RunnerScaleSetName string `json:"runner_scale_set_name"` + ServerRootCA string `json:"server_root_ca"` + LogLevel string `json:"log_level"` + LogFormat string `json:"log_format"` + MetricsAddr string `json:"metrics_addr"` + MetricsEndpoint string `json:"metrics_endpoint"` + Metrics *v1alpha1.MetricsConfig `json:"metrics"` + ListenerConfig *v1alpha1.ListenerConfig `json:"listener_config"` } func Read(ctx context.Context, configPath string) (*Config, error) { diff --git a/cmd/ghalistener/main.go b/cmd/ghalistener/main.go index 7dc4a17f..1d572fbf 100644 --- a/cmd/ghalistener/main.go +++ b/cmd/ghalistener/main.go @@ -120,6 +120,7 @@ func run(ctx context.Context, config *config.Config) error { EphemeralRunnerSetName: config.EphemeralRunnerSetName, MaxRunners: config.MaxRunners, MinRunners: config.MinRunners, + ScalerConfig: config.ListenerConfig.GetScaler(), }, scaler.WithLogger(logger.With("component", "worker")), ) diff --git a/cmd/ghalistener/scaler/scaler.go b/cmd/ghalistener/scaler/scaler.go index 51cb0362..7c486f54 100644 --- a/cmd/ghalistener/scaler/scaler.go +++ b/cmd/ghalistener/scaler/scaler.go @@ -30,8 +30,14 @@ type Config struct { EphemeralRunnerSetName string MaxRunners int MinRunners int + ScalerConfig *v1alpha1.ScalerConfig } +const ( + defaultQPS = 50 + defaultBurst = 100 +) + // The Scaler's role is to process the messages it receives from the listener. // It then initiates Kubernetes API requests to carry out the necessary actions. type Scaler struct { @@ -52,12 +58,22 @@ func New(config Config, options ...Option) (*Scaler, error) { targetRunners: -1, patchSeq: -1, } + for _, option := range options { + option(w) + } + if err := w.applyDefaults(); err != nil { + return nil, err + } conf, err := rest.InClusterConfig() if err != nil { return nil, err } + qps, burst := effectiveRateLimiterConfig(config.ScalerConfig, w.logger) + conf.QPS = float32(qps) + conf.Burst = burst + clientset, err := kubernetes.NewForConfig(conf) if err != nil { return nil, err @@ -65,17 +81,36 @@ func New(config Config, options ...Option) (*Scaler, error) { w.clientset = clientset - for _, option := range options { - option(w) - } - - if err := w.applyDefaults(); err != nil { - return nil, err - } - return w, nil } +func effectiveRateLimiterConfig(config *v1alpha1.ScalerConfig, logger *slog.Logger) (int, int) { + if config == nil { + logger.Debug("Listener scaler configuration is missing; using defaults", "qps", defaultQPS, "burst", defaultBurst) + return defaultQPS, defaultBurst + } + + qps := defaultQPS + if config.QPS == nil { + logger.Debug("Listener scaler qps is missing; using default", "default", defaultQPS) + } else if *config.QPS < 1 { + logger.Warn("Listener scaler qps must be greater than 0; using default", "configured", *config.QPS, "default", defaultQPS) + } else { + qps = *config.QPS + } + + burst := defaultBurst + if config.Burst == nil { + logger.Debug("Listener scaler burst is missing; using default", "default", defaultBurst) + } else if *config.Burst < 1 { + logger.Warn("Listener scaler burst must be greater than 0; using default", "configured", *config.Burst, "default", defaultBurst) + } else { + burst = *config.Burst + } + + return qps, burst +} + func (w *Scaler) applyDefaults() error { if w.logger == nil { w.logger = slog.New(slog.DiscardHandler) @@ -213,10 +248,11 @@ func (w *Scaler) HandleDesiredRunnerCount(ctx context.Context, count int) (int, Do(ctx). Into(patchedEphemeralRunnerSet) if err != nil { - return 0, fmt.Errorf("could not patch ephemeral runner set , patch JSON: %s, error: %w", string(mergePatch), err) + return 0, fmt.Errorf("could not patch ephemeral runner set, patch JSON: %s, error: %w", string(mergePatch), err) } - w.logger.Info("Ephemeral runner set scaled.", + w.logger.Info( + "Ephemeral runner set scaled.", "namespace", w.config.EphemeralRunnerSetNamespace, "name", w.config.EphemeralRunnerSetName, "replicas", patchedEphemeralRunnerSet.Spec.Replicas, diff --git a/cmd/ghalistener/scaler/scaler_test.go b/cmd/ghalistener/scaler/scaler_test.go index 7ea3e967..2bf3105c 100644 --- a/cmd/ghalistener/scaler/scaler_test.go +++ b/cmd/ghalistener/scaler/scaler_test.go @@ -1,15 +1,126 @@ package scaler import ( + "bytes" "log/slog" "math" + "strconv" "testing" + "github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1" "github.com/stretchr/testify/assert" ) var discardLogger = slog.New(slog.DiscardHandler) +func TestEffectiveRateLimiterConfig(t *testing.T) { + qps := 75 + burst := 150 + zero := 0 + negative := -1 + + tests := []struct { + name string + config *v1alpha1.ScalerConfig + wantQPS int + wantBurst int + wantLog string + wantLevel string + }{ + { + name: "uses configured values", + config: &v1alpha1.ScalerConfig{ + QPS: &qps, + Burst: &burst, + }, + wantQPS: qps, + wantBurst: burst, + }, + { + name: "defaults missing config", + wantQPS: defaultQPS, + wantBurst: defaultBurst, + wantLog: "Listener scaler configuration is missing; using defaults", + wantLevel: "DEBUG", + }, + { + name: "defaults missing qps", + config: &v1alpha1.ScalerConfig{Burst: &burst}, + wantQPS: defaultQPS, + wantBurst: burst, + wantLog: "Listener scaler qps is missing; using default", + wantLevel: "DEBUG", + }, + { + name: "defaults missing burst", + config: &v1alpha1.ScalerConfig{QPS: &qps}, + wantQPS: qps, + wantBurst: defaultBurst, + wantLog: "Listener scaler burst is missing; using default", + wantLevel: "DEBUG", + }, + { + name: "defaults zero qps", + config: &v1alpha1.ScalerConfig{QPS: &zero, Burst: &burst}, + wantQPS: defaultQPS, + wantBurst: burst, + wantLog: "Listener scaler qps must be greater than 0; using default", + wantLevel: "WARN", + }, + { + name: "defaults negative burst", + config: &v1alpha1.ScalerConfig{QPS: &qps, Burst: &negative}, + wantQPS: qps, + wantBurst: defaultBurst, + wantLog: "Listener scaler burst must be greater than 0; using default", + wantLevel: "WARN", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var logs bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logs, &slog.HandlerOptions{Level: slog.LevelDebug})) + + qps, burst := effectiveRateLimiterConfig(tt.config, logger) + + assert.Equal(t, tt.wantQPS, qps) + assert.Equal(t, tt.wantBurst, burst) + if tt.wantLog == "" { + assert.Empty(t, logs.String()) + return + } + assert.Contains(t, logs.String(), "msg="+strconv.Quote(tt.wantLog)) + // Missing values are a normal configuration, so they must not be + // logged as warnings; only out-of-range values are. + assert.Contains(t, logs.String(), "level="+tt.wantLevel) + }) + } +} + +// TestEffectiveRateLimiterConfig_QuietAtInfoLevel asserts that a listener which +// does not configure the scaler produces no output at the default log level. +func TestEffectiveRateLimiterConfig_QuietAtInfoLevel(t *testing.T) { + for _, tt := range []struct { + name string + config *v1alpha1.ScalerConfig + }{ + {name: "nil config"}, + {name: "empty config", config: &v1alpha1.ScalerConfig{}}, + } { + t.Run(tt.name, func(t *testing.T) { + var logs bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logs, &slog.HandlerOptions{Level: slog.LevelInfo})) + + qps, burst := effectiveRateLimiterConfig(tt.config, logger) + + assert.Equal(t, defaultQPS, qps) + assert.Equal(t, defaultBurst, burst) + assert.Empty(t, logs.String()) + }) + } +} + func TestSetDesiredWorkerState_MinMaxDefaults(t *testing.T) { newEmptyWorker := func() *Scaler { return &Scaler{ diff --git a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml index 29cc9bb4..820a55f1 100644 --- a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml +++ b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml @@ -123,6 +123,22 @@ spec: type: object x-kubernetes-map-type: atomic type: array + listenerConfig: + description: ListenerConfig holds configuration for the ghalistener + pod. + properties: + scaler: + description: ScalerConfig configures the Kubernetes client used + by the ghalistener scaler. + properties: + burst: + minimum: 1 + type: integer + qps: + minimum: 1 + type: integer + type: object + type: object maxRunners: minimum: 0 type: integer diff --git a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml index a2dfd46d..8b78ce24 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -145,6 +145,20 @@ spec: x-kubernetes-map-type: atomic type: object type: object + listenerConfig: + description: ListenerConfig holds configuration for the ghalistener pod. + properties: + scaler: + description: ScalerConfig configures the Kubernetes client used by the ghalistener scaler. + properties: + burst: + minimum: 1 + type: integer + qps: + minimum: 1 + type: integer + type: object + type: object listenerConfigSecretMetadata: description: ResourceMeta carries metadata common to all internal resources properties: diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 38e8d64f..497a294a 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -394,8 +394,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. labelsModified := !maps.Equal(listenerConfigSecret.Labels, desiredLabels) desiredAnnotations := r.mergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) annotationsModified := !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) + dataModified := !reflect.DeepEqual(listenerConfigSecret.Data, desiredSecret.Data) - if labelsModified || annotationsModified { + if labelsModified || annotationsModified || dataModified { updatedSecret := listenerConfigSecret.DeepCopy() if labelsModified { updatedSecret.Labels = desiredLabels @@ -403,6 +404,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. if annotationsModified { updatedSecret.Annotations = desiredAnnotations } + if dataModified { + updatedSecret.Data = desiredSecret.Data + } log.Info("Updating listener config secret", "namespace", updatedSecret.Namespace, "name", updatedSecret.Name) if err := r.Patch(ctx, updatedSecret, client.MergeFrom(&listenerConfigSecret)); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update listener config secret: %w", err) diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index 67f7ecf5..d3414872 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -423,6 +423,63 @@ var _ = Describe("Test AutoScalingListener controller", func() { ).Should(BeEquivalentTo(rulesForListenerRole([]string{updated.Spec.EphemeralRunnerSetName})), "Role should be updated") }) + It("updates listener scaler configuration and recreates the listener pod", func() { + pod := new(corev1.Pod) + Eventually( + func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod) + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(Succeed(), "Listener pod should be created") + oldPodUID := pod.UID + + current := new(v1alpha1.AutoscalingListener) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, current) + Expect(err).NotTo(HaveOccurred(), "failed to get AutoScalingListener") + + qps := 75 + burst := 150 + updated := current.DeepCopy() + updated.Spec.ListenerConfig = &v1alpha1.ListenerConfig{ + Scaler: &v1alpha1.ScalerConfig{ + QPS: &qps, + Burst: &burst, + }, + } + err = k8sClient.Patch(ctx, updated, client.MergeFrom(current)) + Expect(err).NotTo(HaveOccurred(), "failed to update listener scaler configuration") + + secret := new(corev1.Secret) + Eventually( + func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerConfigName(autoscalingListener), Namespace: autoscalingListener.Namespace}, secret) + g.Expect(err).NotTo(HaveOccurred(), "failed to get listener config Secret") + + var config ghalistenerconfig.Config + err = json.Unmarshal(secret.Data["config.json"], &config) + g.Expect(err).NotTo(HaveOccurred(), "failed to parse listener configuration file") + g.Expect(config.ListenerConfig.GetScaler()).NotTo(BeNil()) + g.Expect(config.ListenerConfig.GetScaler().QPS).NotTo(BeNil()) + g.Expect(config.ListenerConfig.GetScaler().Burst).NotTo(BeNil()) + g.Expect(*config.ListenerConfig.GetScaler().QPS).To(Equal(qps)) + g.Expect(*config.ListenerConfig.GetScaler().Burst).To(Equal(burst)) + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).Should(Succeed(), "Listener config Secret should be updated") + + Eventually( + func() (types.UID, error) { + pod := new(corev1.Pod) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingListener.Name, Namespace: autoscalingListener.Namespace}, pod) + return pod.UID, err + }, + autoscalingListenerTestTimeout, + autoscalingListenerTestInterval, + ).ShouldNot(Equal(oldPodUID), "Listener pod should be recreated with the updated configuration") + }) + It("propagates updated listener metadata to owned resources", func() { assertPropagatedMetadata := func(expected string) { Eventually( diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index a75d414c..961f7b98 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -170,6 +170,7 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. RoleMetadata: autoscalingRunnerSet.Spec.ListenerRoleMetadata, RoleBindingMetadata: autoscalingRunnerSet.Spec.ListenerRoleBindingMetadata, ConfigSecretMetadata: autoscalingRunnerSet.Spec.ListenerConfigSecretMetadata, + ListenerConfig: autoscalingRunnerSet.Spec.ListenerConfig, } labels := b.filterAndMergeLabels(autoscalingRunnerSet.Labels, map[string]string{ @@ -269,6 +270,7 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha MetricsAddr: metricsAddr, MetricsEndpoint: metricsEndpoint, Metrics: autoscalingListener.Spec.Metrics, + ListenerConfig: autoscalingListener.Spec.ListenerConfig, } vault := autoscalingListener.Spec.VaultConfig From 5e540b6c718fab35befbd3844fb9540e23c65f29 Mon Sep 17 00:00:00 2001 From: KR Ravindra Date: Wed, 9 Sep 2026 00:50:13 -0700 Subject: [PATCH 5/5] Add default and per-controller max-concurrent-reconciles flags (#4626) Signed-off-by: KR Ravindra <42912207+KR-Ravindra@users.noreply.github.com> --- .../templates/_controller_template.tpl | 16 +++++- .../controller_deployment_args_test.yaml | 51 +++++++++++++++++++ .../values.yaml | 11 +++- .../templates/deployment.yaml | 16 +++++- .../tests/template_test.go | 41 +++++++++++++-- .../values.yaml | 12 +++-- controllers/actions.github.com/options.go | 45 ++++++++++++++-- .../actions.github.com/options_test.go | 43 ++++++++++++++++ main.go | 28 +++++++--- 9 files changed, 238 insertions(+), 25 deletions(-) create mode 100644 controllers/actions.github.com/options_test.go diff --git a/charts/gha-runner-scale-set-controller-experimental/templates/_controller_template.tpl b/charts/gha-runner-scale-set-controller-experimental/templates/_controller_template.tpl index d1391d13..cab00dee 100644 --- a/charts/gha-runner-scale-set-controller-experimental/templates/_controller_template.tpl +++ b/charts/gha-runner-scale-set-controller-experimental/templates/_controller_template.tpl @@ -47,8 +47,20 @@ args: {{- with .Values.controller.manager.config.watchSingleNamespace }} - "--watch-single-namespace={{ . }}" {{- end }} -{{- with .Values.controller.manager.config.runnerMaxConcurrentReconciles }} - - "--runner-max-concurrent-reconciles={{ . }}" +{{- with .Values.controller.manager.config.defaultMaxConcurrentReconciles }} + - "--default-max-concurrent-reconciles={{ . }}" +{{- end }} +{{- with .Values.controller.manager.config.autoscalingRunnerSetMaxConcurrentReconciles }} + - "--autoscaling-runner-set-max-concurrent-reconciles={{ . }}" +{{- end }} +{{- with .Values.controller.manager.config.autoscalingListenerMaxConcurrentReconciles }} + - "--autoscaling-listener-max-concurrent-reconciles={{ . }}" +{{- end }} +{{- with .Values.controller.manager.config.ephemeralRunnerSetMaxConcurrentReconciles }} + - "--ephemeral-runner-set-max-concurrent-reconciles={{ . }}" +{{- end }} +{{- with .Values.controller.manager.config.ephemeralRunnerMaxConcurrentReconciles }} + - "--ephemeral-runner-max-concurrent-reconciles={{ . }}" {{- end }} {{- if .Values.controller.metrics }} {{- with .Values.controller.metrics }} diff --git a/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_args_test.yaml b/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_args_test.yaml index f2f6b75c..24343765 100644 --- a/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_args_test.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/tests/controller_deployment_args_test.yaml @@ -73,3 +73,54 @@ tests: - contains: path: spec.template.spec.containers[0].args content: "--listener-metrics-endpoint=/metrics" + + - it: should omit every max-concurrent-reconciles flag by default + release: + name: "test-arc" + namespace: "test-ns" + asserts: + - notContains: + path: spec.template.spec.containers[0].args + content: "--ephemeral-runner-max-concurrent-reconciles=1" + - notContains: + path: spec.template.spec.containers[0].args + content: "--default-max-concurrent-reconciles=1" + - notContains: + path: spec.template.spec.containers[0].args + content: "--autoscaling-runner-set-max-concurrent-reconciles=1" + - notContains: + path: spec.template.spec.containers[0].args + content: "--autoscaling-listener-max-concurrent-reconciles=1" + - notContains: + path: spec.template.spec.containers[0].args + content: "--ephemeral-runner-set-max-concurrent-reconciles=1" + + - it: should include every max-concurrent-reconciles flag when configured + set: + controller: + manager: + config: + defaultMaxConcurrentReconciles: 4 + autoscalingRunnerSetMaxConcurrentReconciles: 3 + autoscalingListenerMaxConcurrentReconciles: 5 + ephemeralRunnerSetMaxConcurrentReconciles: 6 + ephemeralRunnerMaxConcurrentReconciles: 20 + release: + name: "test-arc" + namespace: "test-ns" + asserts: + - contains: + path: spec.template.spec.containers[0].args + content: "--default-max-concurrent-reconciles=4" + - contains: + path: spec.template.spec.containers[0].args + content: "--autoscaling-runner-set-max-concurrent-reconciles=3" + - contains: + path: spec.template.spec.containers[0].args + content: "--autoscaling-listener-max-concurrent-reconciles=5" + - contains: + path: spec.template.spec.containers[0].args + content: "--ephemeral-runner-set-max-concurrent-reconciles=6" + - contains: + path: spec.template.spec.containers[0].args + content: "--ephemeral-runner-max-concurrent-reconciles=20" diff --git a/charts/gha-runner-scale-set-controller-experimental/values.yaml b/charts/gha-runner-scale-set-controller-experimental/values.yaml index 1c743c30..18808e13 100644 --- a/charts/gha-runner-scale-set-controller-experimental/values.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/values.yaml @@ -28,8 +28,15 @@ controller: # Defaults to watch all namespaces when unset. watchSingleNamespace: "" - # The maximum number of concurrent reconciles which can be run by the EphemeralRunner controller. - runnerMaxConcurrentReconciles: 2 + # The maximum number of concurrent reconciles applied to every controller that does not + # set its own value below. Defaults to 1 when unset. + defaultMaxConcurrentReconciles: null + + # Per-controller overrides. Each defaults to defaultMaxConcurrentReconciles when unset. + autoscalingRunnerSetMaxConcurrentReconciles: null + autoscalingListenerMaxConcurrentReconciles: null + ephemeralRunnerSetMaxConcurrentReconciles: null + ephemeralRunnerMaxConcurrentReconciles: null # List of label prefixes that should NOT be propagated to internal resources. excludeLabelPropagationPrefixes: [] diff --git a/charts/gha-runner-scale-set-controller/templates/deployment.yaml b/charts/gha-runner-scale-set-controller/templates/deployment.yaml index 961f7848..c2293039 100644 --- a/charts/gha-runner-scale-set-controller/templates/deployment.yaml +++ b/charts/gha-runner-scale-set-controller/templates/deployment.yaml @@ -67,8 +67,20 @@ spec: {{- with .Values.flags.watchSingleNamespace }} - "--watch-single-namespace={{ . }}" {{- end }} - {{- with .Values.flags.runnerMaxConcurrentReconciles }} - - "--runner-max-concurrent-reconciles={{ . }}" + {{- with .Values.flags.defaultMaxConcurrentReconciles }} + - "--default-max-concurrent-reconciles={{ . }}" + {{- end }} + {{- with .Values.flags.autoscalingRunnerSetMaxConcurrentReconciles }} + - "--autoscaling-runner-set-max-concurrent-reconciles={{ . }}" + {{- end }} + {{- with .Values.flags.autoscalingListenerMaxConcurrentReconciles }} + - "--autoscaling-listener-max-concurrent-reconciles={{ . }}" + {{- end }} + {{- with .Values.flags.ephemeralRunnerSetMaxConcurrentReconciles }} + - "--ephemeral-runner-set-max-concurrent-reconciles={{ . }}" + {{- end }} + {{- with .Values.flags.ephemeralRunnerMaxConcurrentReconciles }} + - "--ephemeral-runner-max-concurrent-reconciles={{ . }}" {{- end }} {{- if .Values.metrics }} {{- with .Values.metrics }} diff --git a/charts/gha-runner-scale-set-controller/tests/template_test.go b/charts/gha-runner-scale-set-controller/tests/template_test.go index b6ed84d4..db533faf 100644 --- a/charts/gha-runner-scale-set-controller/tests/template_test.go +++ b/charts/gha-runner-scale-set-controller/tests/template_test.go @@ -366,7 +366,6 @@ func TestTemplate_ControllerDeployment_Defaults(t *testing.T) { "--metrics-addr=0", "--listener-metrics-addr=0", "--listener-metrics-endpoint=", - "--runner-max-concurrent-reconciles=2", } assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args) @@ -518,7 +517,6 @@ func TestTemplate_ControllerDeployment_Customize(t *testing.T) { "--listener-metrics-addr=0", "--listener-metrics-endpoint=", "--metrics-addr=0", - "--runner-max-concurrent-reconciles=2", } assert.ElementsMatch(t, expectArgs, deployment.Spec.Template.Spec.Containers[0].Args) @@ -646,7 +644,6 @@ func TestTemplate_EnableLeaderElection(t *testing.T) { "--listener-metrics-addr=0", "--listener-metrics-endpoint=", "--metrics-addr=0", - "--runner-max-concurrent-reconciles=2", } assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args) @@ -687,7 +684,6 @@ func TestTemplate_ControllerDeployment_ForwardImagePullSecrets(t *testing.T) { "--listener-metrics-addr=0", "--listener-metrics-endpoint=", "--metrics-addr=0", - "--runner-max-concurrent-reconciles=2", } assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args) @@ -777,7 +773,6 @@ func TestTemplate_ControllerDeployment_WatchSingleNamespace(t *testing.T) { "--listener-metrics-addr=0", "--listener-metrics-endpoint=", "--metrics-addr=0", - "--runner-max-concurrent-reconciles=2", } assert.ElementsMatch(t, expectedArgs, deployment.Spec.Template.Spec.Containers[0].Args) @@ -796,6 +791,42 @@ func TestTemplate_ControllerDeployment_WatchSingleNamespace(t *testing.T) { assert.Equal(t, "/tmp", deployment.Spec.Template.Spec.Containers[0].VolumeMounts[0].MountPath) } +func TestTemplate_ControllerDeployment_MaxConcurrentReconciles(t *testing.T) { + t.Parallel() + + // Path to the helm chart we will test + helmChartPath, err := filepath.Abs("../../gha-runner-scale-set-controller") + require.NoError(t, err) + + releaseName := "test-arc" + namespaceName := "test-" + strings.ToLower(random.UniqueID()) + + options := &helm.Options{ + Logger: logger.Discard, + SetValues: map[string]string{ + "flags.defaultMaxConcurrentReconciles": "4", + "flags.autoscalingRunnerSetMaxConcurrentReconciles": "3", + "flags.autoscalingListenerMaxConcurrentReconciles": "5", + "flags.ephemeralRunnerSetMaxConcurrentReconciles": "6", + "flags.ephemeralRunnerMaxConcurrentReconciles": "20", + }, + KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), + } + + output := helm.RenderTemplateContext(t, t.Context(), options, helmChartPath, releaseName, []string{"templates/deployment.yaml"}) + + var deployment appsv1.Deployment + helm.UnmarshalK8SYaml(t, output, &deployment) + + assert.Len(t, deployment.Spec.Template.Spec.Containers, 1) + args := deployment.Spec.Template.Spec.Containers[0].Args + assert.Contains(t, args, "--default-max-concurrent-reconciles=4") + assert.Contains(t, args, "--autoscaling-runner-set-max-concurrent-reconciles=3") + assert.Contains(t, args, "--autoscaling-listener-max-concurrent-reconciles=5") + assert.Contains(t, args, "--ephemeral-runner-set-max-concurrent-reconciles=6") + assert.Contains(t, args, "--ephemeral-runner-max-concurrent-reconciles=20") +} + func TestTemplate_ControllerContainerEnvironmentVariables(t *testing.T) { t.Parallel() diff --git a/charts/gha-runner-scale-set-controller/values.yaml b/charts/gha-runner-scale-set-controller/values.yaml index 87fc46c9..ce119bfa 100644 --- a/charts/gha-runner-scale-set-controller/values.yaml +++ b/charts/gha-runner-scale-set-controller/values.yaml @@ -114,10 +114,16 @@ flags: ## Defaults to watch all namespaces when unset. # watchSingleNamespace: "" - ## The maximum number of concurrent reconciles which can be run by the EphemeralRunner controller. - # Increase this value to improve the throughput of the controller. + ## The maximum number of concurrent reconciles applied to every controller that does not set its own value below. + # Increase this value to improve the throughput of the controller when running many runner scale sets. # It may also increase the load on the API server and the external service (e.g. GitHub API). - runnerMaxConcurrentReconciles: 2 + # defaultMaxConcurrentReconciles: 1 + + ## Per-controller overrides. Each defaults to defaultMaxConcurrentReconciles when unset. + # autoscalingRunnerSetMaxConcurrentReconciles: 1 + # autoscalingListenerMaxConcurrentReconciles: 1 + # ephemeralRunnerSetMaxConcurrentReconciles: 1 + # ephemeralRunnerMaxConcurrentReconciles: 1 ## Defines a list of prefixes that should not be propagated to internal resources. ## This is useful when you have labels that are used for internal purposes and should not be propagated to internal resources. diff --git a/controllers/actions.github.com/options.go b/controllers/actions.github.com/options.go index bc9583de..10104aae 100644 --- a/controllers/actions.github.com/options.go +++ b/controllers/actions.github.com/options.go @@ -10,9 +10,25 @@ import ( // Options is the optional configuration for the controllers, which can be // set via command-line flags or environment variables. type Options struct { - // RunnerMaxConcurrentReconciles is the maximum number of concurrent Reconciles which can be run - // by the EphemeralRunnerController. - RunnerMaxConcurrentReconciles int + // DefaultMaxConcurrentReconciles is the maximum number of concurrent Reconciles + // applied to every controller that does not have its own value set below. + DefaultMaxConcurrentReconciles int + + // AutoscalingRunnerSetMaxConcurrentReconciles is the maximum number of concurrent Reconciles + // which can be run by the AutoscalingRunnerSetController. Zero means DefaultMaxConcurrentReconciles. + AutoscalingRunnerSetMaxConcurrentReconciles int + + // AutoscalingListenerMaxConcurrentReconciles is the maximum number of concurrent Reconciles + // which can be run by the AutoscalingListenerController. Zero means DefaultMaxConcurrentReconciles. + AutoscalingListenerMaxConcurrentReconciles int + + // EphemeralRunnerSetMaxConcurrentReconciles is the maximum number of concurrent Reconciles + // which can be run by the EphemeralRunnerSetController. Zero means DefaultMaxConcurrentReconciles. + EphemeralRunnerSetMaxConcurrentReconciles int + + // EphemeralRunnerMaxConcurrentReconciles is the maximum number of concurrent Reconciles + // which can be run by the EphemeralRunnerController. Zero means DefaultMaxConcurrentReconciles. + EphemeralRunnerMaxConcurrentReconciles int } // OptionsWithDefault returns the default options. @@ -20,10 +36,31 @@ type Options struct { // rather than having to correlate those in multiple places. func OptionsWithDefault() Options { return Options{ - RunnerMaxConcurrentReconciles: 2, + DefaultMaxConcurrentReconciles: 1, } } +// Resolve returns a copy of the options where a DefaultMaxConcurrentReconciles +// of zero or less is replaced by 1, and every per-controller +// MaxConcurrentReconciles that is left at zero is replaced by +// DefaultMaxConcurrentReconciles. +func (o Options) Resolve() Options { + if o.DefaultMaxConcurrentReconciles <= 0 { + o.DefaultMaxConcurrentReconciles = 1 + } + orDefault := func(n int) int { + if n > 0 { + return n + } + return o.DefaultMaxConcurrentReconciles + } + o.AutoscalingRunnerSetMaxConcurrentReconciles = orDefault(o.AutoscalingRunnerSetMaxConcurrentReconciles) + o.AutoscalingListenerMaxConcurrentReconciles = orDefault(o.AutoscalingListenerMaxConcurrentReconciles) + o.EphemeralRunnerSetMaxConcurrentReconciles = orDefault(o.EphemeralRunnerSetMaxConcurrentReconciles) + o.EphemeralRunnerMaxConcurrentReconciles = orDefault(o.EphemeralRunnerMaxConcurrentReconciles) + return o +} + type Option func(*controller.Options) // WithMaxConcurrentReconciles sets the maximum number of concurrent Reconciles which can be run. diff --git a/controllers/actions.github.com/options_test.go b/controllers/actions.github.com/options_test.go new file mode 100644 index 00000000..020d7594 --- /dev/null +++ b/controllers/actions.github.com/options_test.go @@ -0,0 +1,43 @@ +package actionsgithubcom + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOptionsResolve(t *testing.T) { + t.Parallel() + + t.Run("unset per-controller values fall back to the default", func(t *testing.T) { + got := OptionsWithDefault().Resolve() + assert.Equal(t, 1, got.DefaultMaxConcurrentReconciles) + assert.Equal(t, 1, got.AutoscalingRunnerSetMaxConcurrentReconciles) + assert.Equal(t, 1, got.AutoscalingListenerMaxConcurrentReconciles) + assert.Equal(t, 1, got.EphemeralRunnerSetMaxConcurrentReconciles) + assert.Equal(t, 1, got.EphemeralRunnerMaxConcurrentReconciles) + }) + + t.Run("a default of zero or less is replaced by 1", func(t *testing.T) { + for _, n := range []int{0, -1} { + opts := Options{DefaultMaxConcurrentReconciles: n} + got := opts.Resolve() + assert.Equal(t, 1, got.DefaultMaxConcurrentReconciles) + assert.Equal(t, 1, got.AutoscalingRunnerSetMaxConcurrentReconciles) + assert.Equal(t, 1, got.AutoscalingListenerMaxConcurrentReconciles) + assert.Equal(t, 1, got.EphemeralRunnerSetMaxConcurrentReconciles) + assert.Equal(t, 1, got.EphemeralRunnerMaxConcurrentReconciles) + } + }) + + t.Run("a changed default applies to every unset controller", func(t *testing.T) { + opts := OptionsWithDefault() + opts.DefaultMaxConcurrentReconciles = 4 + opts.EphemeralRunnerMaxConcurrentReconciles = 20 + got := opts.Resolve() + assert.Equal(t, 4, got.AutoscalingRunnerSetMaxConcurrentReconciles) + assert.Equal(t, 4, got.AutoscalingListenerMaxConcurrentReconciles) + assert.Equal(t, 4, got.EphemeralRunnerSetMaxConcurrentReconciles) + assert.Equal(t, 20, got.EphemeralRunnerMaxConcurrentReconciles) + }) +} diff --git a/main.go b/main.go index 5ec4a566..d6759d4d 100644 --- a/main.go +++ b/main.go @@ -151,7 +151,11 @@ func main() { flag.DurationVar(&defaultScaleDownDelay, "default-scale-down-delay", actionssummerwindnet.DefaultScaleDownDelay, "The approximate delay for a scale down followed by a scale up, used to prevent flapping (down->up->down->... loop)") flag.IntVar(&port, "port", 9443, "The port to which the admission webhook endpoint should bind") flag.DurationVar(&syncPeriod, "sync-period", 1*time.Minute, "Determines the minimum frequency at which K8s resources managed by this controller are reconciled.") - flag.IntVar(&opts.RunnerMaxConcurrentReconciles, "runner-max-concurrent-reconciles", opts.RunnerMaxConcurrentReconciles, "The maximum number of concurrent reconciles which can be run by the EphemeralRunner controller. Increase this value to improve the throughput of the controller, but it may also increase the load on the API server and the external service (e.g. GitHub API).") + flag.IntVar(&opts.DefaultMaxConcurrentReconciles, "default-max-concurrent-reconciles", opts.DefaultMaxConcurrentReconciles, "The maximum number of concurrent reconciles applied to every controller that does not set its own ---max-concurrent-reconciles flag. Increase this value to improve the throughput of the controller, but it may also increase the load on the API server and the external service (e.g. GitHub API).") + flag.IntVar(&opts.AutoscalingRunnerSetMaxConcurrentReconciles, "autoscaling-runner-set-max-concurrent-reconciles", 0, "The maximum number of concurrent reconciles which can be run by the AutoscalingRunnerSet controller. Defaults to --default-max-concurrent-reconciles.") + flag.IntVar(&opts.AutoscalingListenerMaxConcurrentReconciles, "autoscaling-listener-max-concurrent-reconciles", 0, "The maximum number of concurrent reconciles which can be run by the AutoscalingListener controller. Defaults to --default-max-concurrent-reconciles.") + flag.IntVar(&opts.EphemeralRunnerSetMaxConcurrentReconciles, "ephemeral-runner-set-max-concurrent-reconciles", 0, "The maximum number of concurrent reconciles which can be run by the EphemeralRunnerSet controller. Defaults to --default-max-concurrent-reconciles.") + flag.IntVar(&opts.EphemeralRunnerMaxConcurrentReconciles, "ephemeral-runner-max-concurrent-reconciles", 0, "The maximum number of concurrent reconciles which can be run by the EphemeralRunner controller. Defaults to --default-max-concurrent-reconciles.") flag.Var(&commonRunnerLabels, "common-runner-labels", "Runner labels in the K1=V1,K2=V2,... format that are inherited all the runners created by the controller. See https://github.com/actions/actions-runner-controller/issues/321 for more information") flag.StringVar(&namespace, "watch-namespace", "", "The namespace to watch for custom resources. Set to empty for letting it watch for all namespaces.") flag.StringVar(&watchSingleNamespace, "watch-single-namespace", "", "Restrict to watch for custom resources in a single namespace.") @@ -165,6 +169,7 @@ func main() { flag.StringVar(&workqueueRateLimiter, "workqueue-rate-limiter", "", `The workqueue rate limiter to use. Valid values are "bucket_rate_limiter" (default) and "typed_rate_limiter" (per-item only, no global token bucket).`) flag.Parse() + opts = opts.Resolve() runnerPodDefaults.RunnerImagePullSecrets = runnerImagePullSecrets log, err := logging.NewLogger(logLevel, logFormat) @@ -174,7 +179,13 @@ func main() { } c.Log = &log - log.Info("Using options", "runner-max-concurrent-reconciles", opts.RunnerMaxConcurrentReconciles) + log.Info("Using options", + "default-max-concurrent-reconciles", opts.DefaultMaxConcurrentReconciles, + "autoscaling-runner-set-max-concurrent-reconciles", opts.AutoscalingRunnerSetMaxConcurrentReconciles, + "autoscaling-listener-max-concurrent-reconciles", opts.AutoscalingListenerMaxConcurrentReconciles, + "ephemeral-runner-set-max-concurrent-reconciles", opts.EphemeralRunnerSetMaxConcurrentReconciles, + "ephemeral-runner-max-concurrent-reconciles", opts.EphemeralRunnerMaxConcurrentReconciles, + ) if !autoScalingRunnerSetOnly { ghClient, err = c.NewClient() @@ -324,6 +335,7 @@ func main() { os.Exit(1) } + autoscalingRunnerSetOpts := append(controllerOpts, actionsgithubcom.WithMaxConcurrentReconciles(opts.AutoscalingRunnerSetMaxConcurrentReconciles)) if err = (&actionsgithubcom.AutoscalingRunnerSetReconciler{ Client: mgr.GetClient(), Log: log.WithName("AutoscalingRunnerSet").WithValues("version", build.Version), @@ -332,33 +344,35 @@ func main() { DefaultRunnerScaleSetListenerImage: managerImage, DefaultRunnerScaleSetListenerImagePullSecrets: autoScalerImagePullSecrets, ResourceBuilder: rb, - }).SetupWithManager(mgr, controllerOpts...); err != nil { + }).SetupWithManager(mgr, autoscalingRunnerSetOpts...); err != nil { log.Error(err, "unable to create controller", "controller", "AutoscalingRunnerSet") os.Exit(1) } - runnerOpts := append(controllerOpts, actionsgithubcom.WithMaxConcurrentReconciles(opts.RunnerMaxConcurrentReconciles)) + ephemeralRunnerOpts := append(controllerOpts, actionsgithubcom.WithMaxConcurrentReconciles(opts.EphemeralRunnerMaxConcurrentReconciles)) if err = (&actionsgithubcom.EphemeralRunnerReconciler{ Client: mgr.GetClient(), Log: log.WithName("EphemeralRunner").WithValues("version", build.Version), Scheme: mgr.GetScheme(), PublishMetrics: metricsAddr != "0", ResourceBuilder: rb, - }).SetupWithManager(mgr, runnerOpts...); err != nil { + }).SetupWithManager(mgr, ephemeralRunnerOpts...); err != nil { log.Error(err, "unable to create controller", "controller", "EphemeralRunner") os.Exit(1) } + ephemeralRunnerSetOpts := append(controllerOpts, actionsgithubcom.WithMaxConcurrentReconciles(opts.EphemeralRunnerSetMaxConcurrentReconciles)) if err = (&actionsgithubcom.EphemeralRunnerSetReconciler{ Client: mgr.GetClient(), Log: log.WithName("EphemeralRunnerSet").WithValues("version", build.Version), Scheme: mgr.GetScheme(), ResourceBuilder: rb, - }).SetupWithManager(mgr, controllerOpts...); err != nil { + }).SetupWithManager(mgr, ephemeralRunnerSetOpts...); err != nil { log.Error(err, "unable to create controller", "controller", "EphemeralRunnerSet") os.Exit(1) } + autoscalingListenerOpts := append(controllerOpts, actionsgithubcom.WithMaxConcurrentReconciles(opts.AutoscalingListenerMaxConcurrentReconciles)) if err = (&actionsgithubcom.AutoscalingListenerReconciler{ Client: mgr.GetClient(), Log: log.WithName("AutoscalingListener").WithValues("version", build.Version), @@ -366,7 +380,7 @@ func main() { ListenerMetricsAddr: listenerMetricsAddr, ListenerMetricsEndpoint: listenerMetricsEndpoint, ResourceBuilder: rb, - }).SetupWithManager(mgr, controllerOpts...); err != nil { + }).SetupWithManager(mgr, autoscalingListenerOpts...); err != nil { log.Error(err, "unable to create controller", "controller", "AutoscalingListener") os.Exit(1) }