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