diff --git a/roles/github_runner_arc/README.md b/roles/github_runner_arc/README.md index ab5c730..6472e66 100644 --- a/roles/github_runner_arc/README.md +++ b/roles/github_runner_arc/README.md @@ -39,6 +39,7 @@ Helm 4 and server-side apply. The role reads `helm version --short` and, on Helm - `github_runner_arc_controller_replicas`: above 1, the chart turns on leader election and the role adds a preferred pod anti-affinity across nodes. `github_runner_arc_controller_extra_values` is merged over the role's controller values. - `github_runner_arc_metrics_enabled` (and the `_addr`/`_endpoint` settings): Prometheus metrics for the controller and every listener; the 0.14 chart only enables them together. - `github_runner_arc_listener_probes_enabled`: readiness and liveness probes on each listener's metrics endpoint, so a listener that starts and then fails its GitHub authentication is not counted ready during a rollout. Needs metrics on. +- `github_runner_arc_control_pods_priority_enabled`: runs the controller and every listener under the platform's PriorityClass (`github_runner_arc_platform_priority_class_name`, created by the role whether or not the platform is installed). A listener is one pod per scale set that ARC cannot replicate, and while it waits to be scheduled no new jobs are assigned; with the class a rescheduled one can take a full node's room from a runner pod, whose job ARC retries, instead of waiting behind it. Off by default, as it changes what a full node evicts. - `github_runner_arc_node_label_key`, `github_runner_arc_node_label_value`: when the key is set, the controller, listeners and runner pods are restricted to nodes carrying the label, and the autoscaler reads memory pressure from those nodes only. The role labels the nodes named in `github_runner_arc_labelled_nodes`. - `github_runner_arc_ghcr_username`, `github_runner_arc_ghcr_token`, `github_runner_arc_heartbeat_gh_token`: secrets, as plain variables. The pull credential is needed only for a static pull Secret. - `github_runner_arc_manage_secrets`: `false` stops the role writing the App Secrets and `heartbeat-gh-token`, and instead checks before any change that they already exist, and that each App Secret holds `github_app_id`, `github_app_installation_id` and `github_app_private_key`. The role then needs no `private_key` or `installation_id`, and no `app_id` either: it reads the App's id from the Secret where it needs it. An `app_id` that is set must match the Secret's `github_app_id`. diff --git a/roles/github_runner_arc/defaults/main.yml b/roles/github_runner_arc/defaults/main.yml index 36bf42c..88d4141 100644 --- a/roles/github_runner_arc/defaults/main.yml +++ b/roles/github_runner_arc/defaults/main.yml @@ -90,6 +90,9 @@ github_runner_arc_listener_metrics_endpoint: "/metrics" github_runner_arc_listener_probes_enabled: false github_runner_arc_listener_probe_port: 8080 +# Runs the controller and every listener under the platform's PriorityClass (github_runner_arc_platform_priority_class_name), which the role creates whether or not the platform is installed. A listener is one pod per scale set that ARC cannot replicate, and while it waits to be scheduled no new jobs are assigned, so one that was rescheduled on a full node waits behind runner pods at the default priority. With the class it can take a node's room from a runner pod, whose job ARC retries. An upgrade also reschedules every controller replica together. Off by default, as it changes what a full node evicts. +github_runner_arc_control_pods_priority_enabled: false + # Node eligibility by label. When the key is set, the controller, every listener and every runner pod get a nodeSelector for key=value, the autoscaler reads memory pressure only from nodes carrying it, and the role labels each node named in github_runner_arc_labelled_nodes with it (nodes labelled some other way need not be listed). A label rather than a taint keeps the cluster's other workloads unaffected. A profile's own node_selector is merged over this. github_runner_arc_node_label_key: "" github_runner_arc_node_label_value: "true" diff --git a/roles/github_runner_arc/meta/argument_specs.yml b/roles/github_runner_arc/meta/argument_specs.yml index 492e4b9..ba09728 100644 --- a/roles/github_runner_arc/meta/argument_specs.yml +++ b/roles/github_runner_arc/meta/argument_specs.yml @@ -166,6 +166,10 @@ argument_specs: type: int default: 8080 description: Port the listener probes poll. + github_runner_arc_control_pods_priority_enabled: + type: bool + default: false + description: Runs the controller and every listener under the platform's PriorityClass (github_runner_arc_platform_priority_class_name), so a rescheduled listener can take a full node's room from a runner pod instead of waiting behind it. github_runner_arc_node_label_key: type: str default: "" diff --git a/roles/github_runner_arc/templates/controller-values.yaml.j2 b/roles/github_runner_arc/templates/controller-values.yaml.j2 index 38614e4..c57c369 100644 --- a/roles/github_runner_arc/templates/controller-values.yaml.j2 +++ b/roles/github_runner_arc/templates/controller-values.yaml.j2 @@ -13,6 +13,9 @@ affinity: app.kubernetes.io/name: gha-rs-controller app.kubernetes.io/instance: {{ github_runner_arc_controller_release_name | to_json }} {% endif %} +{% if github_runner_arc_control_pods_priority_enabled | bool %} +priorityClassName: {{ github_runner_arc_platform_priority_class_name | to_json }} +{% endif %} {% if github_runner_arc_node_label_key | length > 0 %} nodeSelector: {{ github_runner_arc_node_label_key | to_json }}: {{ github_runner_arc_node_label_value | string | to_json }} diff --git a/roles/github_runner_arc/templates/scale-set-overlay.yaml.j2 b/roles/github_runner_arc/templates/scale-set-overlay.yaml.j2 index 03a611f..8d7ed67 100644 --- a/roles/github_runner_arc/templates/scale-set-overlay.yaml.j2 +++ b/roles/github_runner_arc/templates/scale-set-overlay.yaml.j2 @@ -23,12 +23,15 @@ template: # The backstop for a pod that outlives its job: the kubelet stops it this long after it starts. activeDeadlineSeconds: {{ github_runner_arc_runner_active_deadline_seconds | int }} {% endif %} -{% if github_runner_arc_node_label_key | length > 0 or github_runner_arc_listener_probes_enabled | bool %} -# The chart puts no nodeSelector and no probe on the listener. The AutoscalingRunnerSet needs containers whenever listenerTemplate.spec is set, and a container named listener is merged into the real listener rather than added as a sidecar. +{% if github_runner_arc_node_label_key | length > 0 or github_runner_arc_listener_probes_enabled | bool or github_runner_arc_control_pods_priority_enabled | bool %} +# The chart puts no nodeSelector, no probe and no priority class on the listener. The AutoscalingRunnerSet needs containers whenever listenerTemplate.spec is set, and a container named listener is merged into the real listener rather than added as a sidecar. listenerTemplate: spec: {% if github_runner_arc_node_label_key | length > 0 %} nodeSelector: {{ eligibility | to_json }} +{% endif %} +{% if github_runner_arc_control_pods_priority_enabled | bool %} + priorityClassName: {{ github_runner_arc_platform_priority_class_name | to_json }} {% endif %} containers: - name: listener diff --git a/tests/scale_set_overlay/render_controller.yml b/tests/scale_set_overlay/render_controller.yml new file mode 100644 index 0000000..59b01c3 --- /dev/null +++ b/tests/scale_set_overlay/render_controller.yml @@ -0,0 +1,16 @@ +--- +# Renders the github_runner_arc role's controller values (templates/controller-values.yaml.j2) with the role's defaults and constants, for tests/unit/test_scale_set_overlay.py. Run with -e overlay_output=, plus any role variable to override. +- name: Render the controller values + hosts: localhost + connection: local + gather_facts: false + vars_files: + - ../../roles/github_runner_arc/defaults/main.yml + - ../../roles/github_runner_arc/vars/main.yml + tasks: + - name: Write the values + ansible.builtin.copy: + # With every option at its default the template renders to nothing, which copy rejects as no content, so an empty mapping stands for it. + content: "{{ (lookup('ansible.builtin.template', playbook_dir ~ '/../../roles/github_runner_arc/templates/controller-values.yaml.j2') | trim) or '{}' }}" + dest: "{{ overlay_output }}" + mode: "0644" diff --git a/tests/unit/test_scale_set_overlay.py b/tests/unit/test_scale_set_overlay.py index 7f1d6b8..bdfabc7 100644 --- a/tests/unit/test_scale_set_overlay.py +++ b/tests/unit/test_scale_set_overlay.py @@ -16,6 +16,7 @@ ROOT = Path(__file__).resolve().parents[2] PLUGIN = ROOT / "plugins" / "filter" / "arc.py" RENDER = ROOT / "tests" / "scale_set_overlay" / "render.yml" +RENDER_CONTROLLER = ROOT / "tests" / "scale_set_overlay" / "render_controller.yml" _spec = importlib.util.spec_from_file_location("arc_filters", PLUGIN) assert _spec is not None and _spec.loader is not None arc = importlib.util.module_from_spec(_spec) @@ -47,6 +48,25 @@ def render(profile: dict[str, Any], **role_vars: Any) -> dict[str, Any]: return rendered +def render_controller(**role_vars: Any) -> dict[str, Any]: + """Render the controller values with ansible-playbook and return them parsed, an empty dict when nothing is set.""" + with tempfile.TemporaryDirectory() as work: + output = Path(work) / "controller.yaml" + extra = Path(work) / "extra.json" + extra.write_text(json.dumps({"overlay_output": str(output), **role_vars})) + env = {**os.environ, "ANSIBLE_COLLECTIONS_PATH": str(ROOT / "playbooks" / "collections"), "ANSIBLE_LOCALHOST_WARNING": "false", "ANSIBLE_INVENTORY_UNPARSED_WARNING": "false"} + subprocess.run([os.environ.get("ANSIBLE_PLAYBOOK", "ansible-playbook"), str(RENDER_CONTROLLER), "-e", f"@{extra}"], check=True, env=env, cwd=work, capture_output=True, text=True) + rendered: dict[str, Any] | None = yaml.safe_load(output.read_text()) + return rendered or {} + + +class ControllerTest(unittest.TestCase): + def test_the_controller_runs_under_the_platform_priority_class_only_when_asked(self) -> None: + self.assertEqual(render_controller(github_runner_arc_control_pods_priority_enabled=True)["priorityClassName"], "github-runner-platform") + self.assertEqual(render_controller(github_runner_arc_control_pods_priority_enabled=True, github_runner_arc_platform_priority_class_name="ci-control")["priorityClassName"], "ci-control") + self.assertNotIn("priorityClassName", render_controller()) + + class OverlayTest(unittest.TestCase): def test_without_burst_the_runner_pods_keep_the_eligibility_node_selector(self) -> None: overlay = render(expanded({"sizing": SIZING, "node_selector": {"kubernetes.io/arch": "amd64"}}), github_runner_arc_node_label_key=ELIGIBLE, github_runner_arc_node_label_value="true") @@ -54,6 +74,14 @@ def test_without_burst_the_runner_pods_keep_the_eligibility_node_selector(self) self.assertEqual(overlay["maxRunners"], 24) self.assertEqual(overlay["listenerTemplate"]["spec"]["nodeSelector"], {ELIGIBLE: "true"}) + def test_the_listener_runs_under_the_platform_priority_class_only_when_asked(self) -> None: + profile = expanded({"sizing": SIZING}) + asked = render(profile, github_runner_arc_control_pods_priority_enabled=True) + self.assertEqual(asked["listenerTemplate"]["spec"]["priorityClassName"], "github-runner-platform") + self.assertEqual(render(profile, github_runner_arc_control_pods_priority_enabled=True, github_runner_arc_platform_priority_class_name="ci-control")["listenerTemplate"]["spec"]["priorityClassName"], "ci-control") + self.assertNotIn("listenerTemplate", render(profile)) + self.assertNotIn("priorityClassName", render(profile, github_runner_arc_node_label_key=ELIGIBLE, github_runner_arc_node_label_value="true")["listenerTemplate"]["spec"]) + def test_burst_places_the_runner_pods_by_affinity_and_adds_the_burst_runners(self) -> None: overlay = render(expanded({"sizing": SIZING, "burst": BURST}), github_runner_arc_node_label_key=ELIGIBLE, github_runner_arc_node_label_value="true") spec = overlay["template"]["spec"]