Add default and per-controller max-concurrent-reconciles flags (#4626)

Signed-off-by: KR Ravindra <42912207+KR-Ravindra@users.noreply.github.com>

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 }}

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"

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: []

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 }}

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()
 

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.