fix(arc): check orgs per cluster and refuse a scale set registered from two clusters

plugins/filter/arc.py

@@ -609,7 +609,7 @@ def _expand_profile(org_name: str, app_secret: str, index: int, profile: Any, er
 def arc_profiles(orgs: Sequence[Any], require_app_id: bool = True) -> dict[str, Any]:
     """Expand github_runner_arc_orgs entries into one record per scale-set profile, and check them.
 
-    Pass every org configured anywhere in the inventory, not one host's, so that the cross-host checks (an org configured twice, two profiles sharing a namespace, more than one autoscaled profile setting its own sizing) see the whole fleet.
+    Pass every org that installs into one cluster, not one host's, so that the cross-host checks (an org configured twice, two profiles sharing a namespace, more than one autoscaled profile setting its own sizing) see the whole cluster; arc_cluster_profiles gathers them from the inventory.
 
     Args:
         require_app_id: whether each org must set app_id. The role needs it only when it writes the App Secret; otherwise the App's id is in the existing App Secret, and an app_id that is set is only checked against it.
@@ -640,7 +640,7 @@ def arc_profiles(orgs: Sequence[Any], require_app_id: bool = True) -> dict[str,
         if org.get("private_key") is not None and org.get("private_key_op_reference") is not None:
             errors.append(f"{org_name}: set private_key or private_key_op_reference, not both (the 1Password adapter fills private_key from the reference)")
         if org_name.lower() in seen_orgs:
-            errors.append(f"Org '{org_name}' is configured more than once in github_runner_arc_orgs across the inventory (also as '{seen_orgs[org_name.lower()]}'). Each org's Helm releases must be installed from exactly one host; remove the duplicate.")
+            errors.append(f"Org '{org_name}' is configured more than once in github_runner_arc_orgs (also as '{seen_orgs[org_name.lower()]}'). Each org's Helm releases in a cluster must be installed from one entry on one host; remove the duplicate.")
             continue
         seen_orgs[org_name.lower()] = org_name
         org_profiles = org.get("scale_set_profiles")
@@ -671,6 +671,61 @@ def arc_profiles(orgs: Sequence[Any], require_app_id: bool = True) -> dict[str,
     return {"errors": errors, "profiles": profiles, "autoscaled": autoscaled}
 
 
+def _is_list(value: Any) -> bool:
+    """Return whether value is a list in the YAML sense: a sequence that is not a string."""
+    return isinstance(value, Sequence) and not isinstance(value, (str, bytes))
+
+
+def arc_cluster_profiles(orgs_by_host: Mapping[str, Any], cluster: Sequence[str] | None = None) -> dict[str, Any]:
+    """Expand and check the orgs of every host that installs into one cluster, and check their scale sets against the hosts of every other cluster.
+
+    The hosts of one cluster share its controller, namespaces and autoscaled pool, so an org is installed into a cluster from one host, and arc_profiles' checks apply across all of them. A host of another cluster (another cluster group, with a controller of its own) may install the same org, since each cluster's releases and namespaces are its own, but not the same scale set: GitHub gives an org's scale set name to one controller at a time, so a second cluster's listener would contend with the first for it.
+
+    Args:
+        orgs_by_host: each inventory host's github_runner_arc_orgs, by host name, in inventory order. cluster: the names of the hosts that install into the same cluster as the host being checked, or None when that is not known, which treats every host as one cluster.
+
+    Returns:
+        arc_profiles' result for the orgs of the cluster's hosts (see arc_profiles; app_id is not required, since only each host's own check knows whether it writes the App Secret), with errors that also name an org configured on two of the cluster's hosts and a scale set registered from both this cluster and another.
+    """
+    members = set(orgs_by_host) if cluster is None else set(cluster)
+    errors: list[str] = []
+    cluster_orgs: list[Any] = []
+    installers: dict[str, tuple[str, str]] = {}
+    for host, orgs in orgs_by_host.items():
+        if host not in members:
+            continue
+        if not _is_list(orgs):
+            errors.append(f"{host}: github_runner_arc_orgs must be a list")
+            continue
+        for org in orgs:
+            name = _text(org, "name") if isinstance(org, Mapping) else ""
+            installer = installers.get(name.lower()) if name else None
+            # The same org twice on one host is arc_profiles' own check, so it is passed on for that.
+            if installer is not None and installer[0] != host:
+                errors.append(f"Org '{name}' is configured more than once in github_runner_arc_orgs for one cluster, on {installer[0]} (as '{installer[1]}') and on {host}. Each org's Helm releases in a cluster must be installed from one host; move its scale_set_profiles onto one of them.")
+                continue
+            if name:
+                installers.setdefault(name.lower(), (host, name))
+            cluster_orgs.append(org)
+    result = arc_profiles(cluster_orgs, require_app_id=False)
+    # Another cluster's hosts are checked in full by that cluster's own validation, so only the scale sets that expand are compared here.
+    registered: dict[tuple[str, str], tuple[str, str]] = {}
+    elsewhere: list[tuple[str, dict[str, Any]]] = []
+    for host, orgs in orgs_by_host.items():
+        if not _is_list(orgs):
+            continue
+        for profile in arc_profiles(orgs, require_app_id=False)["profiles"]:
+            if host in members:
+                registered.setdefault((profile["org_name"].lower(), profile["release"]), (host, profile["label"]))
+            else:
+                elsewhere.append((host, profile))
+    for host, profile in elsewhere:
+        match = registered.get((profile["org_name"].lower(), profile["release"]))
+        if match is not None:
+            errors.append(f"{match[1]} on {match[0]} and {profile['label']} on {host}, which installs into another cluster, both register scale set '{profile['release']}' for org '{profile['org_name']}'. GitHub gives a scale set name to one controller at a time; uninstall it from one cluster before installing it in the other, or give one of them a different suffix or release_name.")
+    return {**result, "errors": errors + result["errors"]}
+
+
 def _pinned_image_errors(where: str, image: Any) -> list[str]:
     """Return what is wrong with a capability's image reference: it must be a non-empty reference pinned to a version tag or a digest."""
     if not isinstance(image, str) or not image:
@@ -1458,6 +1513,7 @@ def filters(self) -> dict[str, Any]:
         """Return the filters this plugin provides."""
         return {
             "arc_profiles": arc_profiles,
+            "arc_cluster_profiles": arc_cluster_profiles,
             "arc_max_runners": arc_max_runners,
             "arc_measured_max_runners": arc_measured_max_runners,
             "arc_upgrade_blocker": arc_upgrade_blocker,

roles/github_runner_arc/README.md

@@ -6,7 +6,9 @@ It runs in two ways. Inside `playbooks/site.yml`, against a k3s server host that
 
 ## Validation
 
-`tasks/validate.yml` checks every input that needs neither secrets nor a cluster (a measured sizing's node figures are read later, when the role installs the profile): each org's fields, profile names (derived or overridden) that would collide or make invalid Kubernetes names, two of an org's profiles sharing a release name, a `max_runners` that is not a whole number, the controller's release name and namespace, the same org configured on two hosts, more than one autoscaled profile, values files that do not exist, the autoscaler's pool settings, sizing inputs, burst settings, and the controller, probe and node label settings. Both playbooks run it in a play of its own before anything changes; the role runs it itself when included some other way. It also bootstraps the heartbeat gist when `github_runner_arc_heartbeat_bootstrap_gist` is on and no host in the inventory has one: it creates the gist with the control node's `gh` session and stops, printing the id to record.
+`tasks/validate.yml` checks every input that needs neither secrets nor a cluster (a measured sizing's node figures are read later, when the role installs the profile): each org's fields, profile names (derived or overridden) that would collide or make invalid Kubernetes names, two of an org's profiles sharing a release name, a `max_runners` that is not a whole number, the controller's release name and namespace, the same org configured on two hosts of one cluster, a scale set registered from two clusters (see below), more than one autoscaled profile, values files that do not exist, the autoscaler's pool settings, sizing inputs, burst settings, and the controller, probe and node label settings. Both playbooks run it in a play of its own before anything changes; the role runs it itself when included some other way. It also bootstraps the heartbeat gist when `github_runner_arc_heartbeat_bootstrap_gist` is on and no host in the inventory has one: it creates the gist with the control node's `gh` session and stops, printing the id to record.
+
+The checks that span hosts compare the hosts of one cluster: the hosts in this host's cluster group (`github_runner_cluster_group`), which share a controller, its namespaces and the autoscaled pool, so an org is installed into a cluster from one host. A host of another cluster group may install the same org, for example while its scale sets move one at a time to a second cluster beside the first, but not the same scale set, since GitHub gives an org's scale set name to one controller at a time. A host outside its cluster group, such as the control node running `playbooks/arc.yml`, counts every host in the inventory as one cluster.
 
 Before touching the cluster the role then checks the secrets it is about to write (each org's App key looks like a PEM key, the pull credential and heartbeat token are set) and proves the pull credential can read every image it installs from `github_runner_arc_image_pull_registry`, by getting a pull-scoped registry token and fetching each image's manifest. With an App-sourced pull Secret (see below) it mints each org's token and proves it the same way, before installing anything.
 
@@ -21,7 +23,7 @@ Before touching the cluster the role then checks the secrets it is about to writ
   - `runs_on_label`: the extra `scaleSetLabels` entry the runners carry. Default `<org>-runners`, which pools every profile of the org under one `runs-on` label; set a different one to keep a profile out of that pool.
   - `scale_set_labels`: the whole `scaleSetLabels` list, in place of `runs_on_label`. An empty list sets no `scaleSetLabels`, so jobs target the scale set by its release name.
   - `tolerations`: Kubernetes tolerations for the profile's runner pods, for nodes that carry both the eligibility label and a `NoSchedule` or `NoExecute` taint that keeps other workloads off them, such as machines the controller and listener must not run on. Measured sizing counts a node whose only repelling taints these tolerations match; without them it leaves a tainted node out, because it takes runner pods to tolerate nothing. A taint Kubernetes adds for a passing condition (not ready, unreachable, cordoned) still counts as before. With `burst` they come before `burst.tolerations`.
-  - `autoscale`: `true` on at most one profile in the whole inventory makes it the scale set the autoscaler manages.
+  - `autoscale`: `true` makes the profile's scale set one of those the autoscaler manages, pooled against one memory budget; at most one autoscaled profile in a cluster may set `sizing`.
   - `sizing`: derive a runner ceiling from node capacity, given as uniform figures or measured from each eligible node (see Sizing). On an ordinary profile without `max_runners` it sets `maxRunners`; on the autoscaled profile it bounds the ceiling the autoscaler works out from the nodes, leaving `maxRunners` as the floor.
   - `capabilities`: names of entries in `github_runner_arc_capabilities` that the profile's runner pods carry, in order (see [Capabilities](#capabilities)).
   - `burst`: let the runner pods overflow onto nodes a cluster autoscaler adds on demand, and count those nodes' runners into `maxRunners` (see [Burst nodes](#burst-nodes)). Needs `sizing`, and no `max_runners` or `autoscale`; with `exclusive: true` the pods run on burst nodes only and `burst.max_runners` is the whole ceiling, with no `sizing`.

roles/github_runner_arc/tasks/install_scale_set_profile.yml

@@ -109,7 +109,7 @@
       data: "{{ github_runner_arc_job_started_data }}"
 
 - name: Install/upgrade the scale-set release for {{ profile.label }}
-  # runnerScaleSetName is left to default to the release name: it is the scale set's registration identity on GitHub, so two releases sharing it make the second one's listener crash-loop registering a duplicate. scaleSetLabels is the pooling mechanism instead: an extra label every profile's runners carry alongside their own unique one, so runs-on: <label> matches every profile sharing it. A profile sets its own runs_on_label to stay out of the shared pool, or scale_set_labels to choose the whole list (empty sets none, leaving jobs to target the release name).
+  # runnerScaleSetName is set to the release name in github_runner_arc_scale_set_set_values, over any values file: it is the scale set's registration identity on GitHub, so two releases sharing it make the second one's listener crash-loop registering a duplicate. scaleSetLabels is the pooling mechanism instead: an extra label every profile's runners carry alongside their own unique one, so runs-on: <label> matches every profile sharing it. A profile sets its own runs_on_label to stay out of the shared pool, or scale_set_labels to choose the whole list (empty sets none, leaving jobs to target the release name).
   kubernetes.core.helm:
     name: "{{ profile.release }}"
     chart_ref: oci://ghcr.io/actions/actions-runner-controller-charts/gha-runner-scale-set

roles/github_runner_arc/tasks/validate.yml

@@ -1,17 +1,20 @@
 ---
 # Checks every ARC input that can be checked without secrets or a cluster, so that playbooks/site.yml can run it in a pre-flight play before k3s is started or changed. Reads nothing from the target host and changes nothing, apart from bootstrapping the heartbeat gist on GitHub (see below). Cross-host checks read github_runner_arc_orgs from every host in the inventory, which --limit does not narrow, and from this host however it was set.
 
-# This host's orgs come from github_runner_arc_orgs as this play resolves it, which includes play and role variables and -e; another host's can only be read from hostvars, which holds its inventory variables and facts. Taking this host's from hostvars too would miss orgs passed as a play variable, and every check and lookup built on the fleet (the scale sets' namespaces that measured sizing leaves out, the autoscaled profile) would then not see them. An implicit localhost is not in groups['all'] at all.
-- name: Expand every org configured anywhere in the inventory into its scale-set profiles
+# This host's orgs come from github_runner_arc_orgs as this play resolves it, which includes play and role variables and -e; another host's can only be read from hostvars, which holds its inventory variables and facts. Taking this host's from hostvars too would miss orgs passed as a play variable, and every check and lookup built on the fleet (the scale sets' namespaces that measured sizing leaves out, the autoscaled profile) would then not see them. An implicit localhost is not in groups['all'] at all. The fleet is this host's cluster: the hosts of its cluster group (github_runner_cluster_group, as the playbooks resolve it), each of which belongs to one cluster. Another cluster's hosts have a controller of their own, so they are compared only for scale set names, which GitHub gives to one controller at a time. A host outside its cluster group, such as the control node running playbooks/arc.yml, cannot tell which hosts share its cluster, so every host counts as one cluster.
+- name: Expand every org configured in this host's cluster into its scale-set profiles
   ansible.builtin.set_fact:
-    # Whether an org needs app_id depends on whether its host writes the App Secret, which only that host's own check knows, so the fleet-wide expansion (for the cross-host checks) does not ask for it.
+    # Whether an org needs app_id depends on whether its host writes the App Secret, which only that host's own check knows, so the cluster-wide expansion (for the cross-host checks) does not ask for it.
     github_runner_arc_fleet: >-
       {{
-        ((groups['all'] | reject('equalto', inventory_hostname) | map('extract', hostvars) | map(attribute='github_runner_arc_orgs', default=[]) | flatten)
-         + github_runner_arc_orgs)
-        | exadev.github_runner.arc_profiles(require_app_id=false)
+        dict(github_runner_arc_other_hosts | zip(github_runner_arc_other_hosts | map('extract', hostvars) | map(attribute='github_runner_arc_orgs', default=[])))
+        | combine({inventory_hostname: github_runner_arc_orgs})
+        | exadev.github_runner.arc_cluster_profiles(groups[github_runner_arc_cluster_group] if inventory_hostname in groups.get(github_runner_arc_cluster_group, []) else none)
       }}
     github_runner_arc_local: "{{ github_runner_arc_orgs | exadev.github_runner.arc_profiles(require_app_id=github_runner_arc_manage_secrets | bool) }}"
+  vars:
+    github_runner_arc_other_hosts: "{{ groups['all'] | reject('equalto', inventory_hostname) | list }}"
+    github_runner_arc_cluster_group: "{{ github_runner_cluster_group | default('github_runner_cluster') }}"
 
 - name: Fail on invalid orgs or scale-set profiles
   ansible.builtin.fail:
@@ -151,7 +154,7 @@
     label: "{{ item.msg }}"
   when: item.failed | bool
 
-# The autoscaler runs from whichever host installs the platform, which need not be the host that installs the autoscaled profile, so these are checked on the platform host against the whole fleet's profiles.
+# The autoscaler runs from whichever host installs the platform, which need not be the host that installs the autoscaled profile, so these are checked on the platform host against the profiles of every host in its cluster.
 - name: Fail when the autoscaler's pool settings are missing or inconsistent
   ansible.builtin.fail:
     msg: >-