feat(role): judge voters by etcd's own vote and let a holder renew the regrade Lease

README.md

@@ -208,7 +208,7 @@ The controller reads the Nodes and the probes' samples every sample period and p
 - A window is 100 samples, the fewest that give the 99th percentile a sample of its own, so one outlier never decides; a host with fewer is `unknown`, as is one whose probe has not reported for three sample periods. Readings from before the last change of the voter set are not counted.
 - Hosts that pass are ranked wired before Wi-Fi before unknown, then by round trip, then by disk. The link ranks hosts and is never a gate; the status gives each host's link and where it was read (`link` and `link_source`), and the log names it beside the verdict.
 
-A slow link makes both of its ends unfit, since a voter is unfit when its round trip to any other voter is, so every unfit verdict is attributed to the node at fault for it (`attributed_to` on each host, and `attribution`, the hosts each node accounts for). A host's own disk, memory, readiness or restarts are its own fault. Its round trip is the link to its worst peer, and the end at fault for a slow link is the one whose links to the other voters are slower too, compared by the best of them, since a node on a poor path is slow to every peer and a node on a good one is fast to its others; when the two ends' other links are as fast as each other's, or one end has none, the link is attributed to both. The voter to replace is the unfit voter the most hosts' unfit verdicts are attributed to, the worst-ranked among those with as many, and the log names the attribution beside each unfit verdict. A swap is reported only when that voter is eligible and not pinned (when it is not, replacing a voter that shares a slow link with it would fix nothing, so nothing is swapped) and a fit eligible standby ranks above it. A candidate is judged against the voters that would remain, since a slow link to the very voter it would replace says nothing against it. It is held back, with the reason in the status and the log, while the voter count is not `github_runner_arc_role_controller_voters`, any voter is below `full`, is not Ready, has no leader, is a learner or knows a member that is not a voter, is not connected to every other voter, fewer hosts are eligible than the target, or the cooldown after a change of the voter set has not passed (k3s's five minutes to remove a stalled learner plus one window). The target must be odd, at least 3, and at most `github_runner_cluster_max_servers` minus one, which leaves a swap one place of headroom; the role refuses anything else. A host is eligible, and a voter pinned, by a Node label with the value `true` (`github_runner_arc_role_controller_eligible_label` and `_pinned_label`), which the cluster role publishes from `github_runner_cluster_voter_eligible` and `github_runner_cluster_voter_pinned` per host (and the target from `github_runner_cluster_voters`, refused unless odd, at least 3 and below `github_runner_cluster_max_servers`; see the cluster role's README); a voter not marked eligible is never swapped out, since nothing has prepared it to come back.
+A slow link makes both of its ends unfit, since a voter is unfit when its round trip to any other voter is, so every unfit verdict is attributed to the node at fault for it (`attributed_to` on each host, and `attribution`, the hosts each node accounts for). A host's own disk, memory, readiness or restarts are its own fault. Its round trip is the link to its worst peer, and the end at fault for a slow link is the one whose links to the other voters are slower too, compared by the best of them, since a node on a poor path is slow to every peer and a node on a good one is fast to its others; when the two ends' other links are as fast as each other's, or one end has none, the link is attributed to both. The voter to replace is the unfit voter the most hosts' unfit verdicts are attributed to, the worst-ranked among those with as many, and the log names the attribution beside each unfit verdict. A swap is reported only when that voter is eligible and not pinned (when it is not, replacing a voter that shares a slow link with it would fix nothing, so nothing is swapped) and a fit eligible standby ranks above it. A candidate is judged against the voters that would remain, since a slow link to the very voter it would replace says nothing against it. A voter is a node that carries k3s's etcd label and whose `EtcdIsVoter` condition is True: k3s labels a server the moment it joins etcd as a learner and never removes the label, so a labelled node without the condition is a member that does not vote: it is not counted as a voter and holds a swap back (`member but not a voter` in the status). It is held back, with the reason in the status and the log, while the voter count is not `github_runner_arc_role_controller_voters`, any voter is below `full`, is not Ready, has no leader, is a learner or knows a member that is not a voter, is not connected to every other voter, fewer hosts are eligible than the target, or the cooldown after a change of the voter set has not passed (k3s's five minutes to remove a stalled learner plus one window). The target must be odd, at least 3, and at most `github_runner_cluster_max_servers` minus one, which leaves a swap one place of headroom; the role refuses anything else. A host is eligible, and a voter pinned, by a Node label with the value `true` (`github_runner_arc_role_controller_eligible_label` and `_pinned_label`), which the cluster role publishes from `github_runner_cluster_voter_eligible` and `github_runner_cluster_voter_pinned` per host (and the target from `github_runner_cluster_voters`, refused unless odd, at least 3 and below `github_runner_cluster_max_servers`; see the cluster role's README); a voter not marked eligible is never swapped out, since nothing has prepared it to come back.
 
 The controller and the probe run one image, `github_runner_arc_role_controller_image`, pulled without a pull Secret, so the published package must be public.
 

roles/github_runner_cluster/files/regrade/fleet-regrade-core.sh

@@ -249,6 +249,25 @@ release_lease() {
   jq '.spec.holderIdentity = ""' <<< "$current" | kubectl_ replace -f - > /dev/null
 }
 
+# Extends the Lease its holder $1 already has, or fails. The holder, acquire time, duration and annotations are kept and the renew time becomes now, and the replacement fails if anyone has changed the Lease since it was read, so a Lease that has passed to another holder, been released or never existed is refused and left as it was. A holder whose Lease has run out but that nobody has taken since may still renew it. Used by a holder that works for longer than one duration, such as the role controller; a step of the regrade core keeps its sized duration and never renews.
+renew_lease() {
+  local holder=${1:?renew_lease needs the holder} current now
+  now=$(now_epoch)
+  if ! current=$(kubectl_ get lease "$LEASE_NAME" --namespace "$LEASE_NAMESPACE" --output json 2>&1); then
+    log "cannot renew the lease: it cannot be read: $current"
+    return 1
+  fi
+  if [ "$(jq -r '.spec.holderIdentity // ""' <<< "$current")" != "$holder" ]; then
+    log "cannot renew the lease: it is not held by $holder"
+    return 1
+  fi
+  if jq --arg renewed "$(micro_time "$now")" '.spec.renewTime = $renewed' <<< "$current" | kubectl_ replace -f - > /dev/null; then
+    return 0
+  fi
+  log "cannot renew the lease: it changed while it was being renewed"
+  return 1
+}
+
 # Succeeds when every other etcd-voting node is Ready. A node whose state is unknown, that has no Ready condition, or that cannot be listed counts as not ready: a step never starts while a peer might be down.
 peers_ready() {
   local nodes not_ready

scripts/fleet-role.jq

@@ -1,7 +1,7 @@
 # The role controller's decision (see scripts/fleet-role.sh, which gathers the inputs and publishes the result): which hosts are fit to hold an etcd vote, and the swap the controller would make. Pure: nothing here reads the cluster or the clock.
 #
 # Inputs, as jq named arguments:
-# - $config: the controller's settings (see scripts/fleet-role.sh for each), with `now` the time of the pass in epoch seconds.
+# - $config: the controller's settings (see scripts/fleet-role.sh for each), with `now` the time of the pass in epoch seconds and `expected_voters` the number of voters the cluster should hold at this moment: the target, or one more while a swap holds a new voter beside the old.
 # - $nodes_file: the output of `kubectl get nodes -o json`, as --slurpfile gives it (a one-element array).
 # - $probes_file: each node's probe entry (scripts/fleet-role-probe.sh), by node name, likewise.
 # - $previous_file: the status this program published on the last pass, or null on the first, likewise.
@@ -29,12 +29,17 @@ def stale_after_samples: 3;
 # Linux's byte count of one kibibyte, MemAvailable being reported in kB that are kibibytes.
 def kib: 1024;
 
+# The Node condition k3s sets True on a member of etcd that votes (setEtcdStatusCondition in pkg/etcd/etcd.go). k3s labels a server as an etcd node when it joins, as a learner that does not vote until the leader promotes it, and does not remove the label when a member is removed, so the label alone is no evidence of a vote.
+def voter_condition: "EtcdIsVoter";
+
 def link_rank: {"wired": 0, "wifi": 1, "unknown": 2};
 
 def round2: if . == null then null else (. * 100 | round) / 100 end;
 
 def ready_condition: ((.status.conditions // []) | map(select(.type == "Ready")) | .[0]);
 
+def condition_true($type): ((.status.conditions // []) | map(select(.type == $type)) | .[0].status) == "True";
+
 # The pair figure of one observer towards one peer over a window: the worse of the TCP connect tail and etcd's own round trip tail (only a voter has the latter, keyed by the peer's etcd id), in milliseconds, with how many TCP readings it rests on.
 def pair($window; $peer; $peer_id):
   ([$window[] | .tcp[$peer] | select(. != null)]) as $tcp
@@ -59,11 +64,13 @@ $config as $c
 # The nodes, with what the decision reads off each.
 | [$nodes.items[]
    | (.metadata.labels // {}) as $labels
+   | ($labels[$c.voter_label] == "true") as $labelled
    | ready_condition as $ready
    | {name: .metadata.name,
       ready: ($ready.status == "True"),
       ready_since: ($ready.lastTransitionTime | fromdateiso8601? // null),
-      voter: ($labels[$c.voter_label] == "true"),
+      labelled: $labelled,
+      voter: ($labelled and condition_true(voter_condition)),
       eligible: ($labels[$c.eligible_label] == "true"),
       pinned: ($labels[$c.pinned_label] == "true"),
       regrade_level: $labels[$c.level_label]}] as $all
@@ -232,7 +239,8 @@ $config as $c
 
 # Why no swap may start now whatever the hosts' fitness: the epic's conditions on the cluster as a whole.
 | ([
-    (if ($voters | length) != $c.target then "there are \($voters | length) voters, not the target \($c.target)" else empty end),
+    (if ($voters | length) != $c.expected_voters then "there are \($voters | length) voters, not the \(if $c.expected_voters == $c.target then "target \($c.target)" else "expected \($c.expected_voters) (the target \($c.target) and the new voter a swap holds)" end)" else empty end),
+    ($all[] | select(.labelled and (.voter | not)) | "node \(.name) is a member but not a voter: it carries the etcd label without \(voter_condition)=True"),
     ($voter_nodes[] | select(.regrade_level != null) | "voter \(.name) is at regrade level \(.regrade_level), below full"),
     ($voter_nodes[] | select(.ready | not) | "voter \(.name) is not Ready"),
     ($voter_names[] as $v | $probes[$v].samples[-1] as $last | $last.etcd as $e

scripts/fleet-role.sh

@@ -60,13 +60,14 @@ kubectl get nodes -o json > "$work/nodes.json"
 kubectl get configmap "$probe_configmap" -n "$namespace" -o json | jq -c '(.data // {}) | map_values(fromjson)' > "$work/probes.json"
 kubectl get configmap "$status_configmap" -n "$namespace" -o json | jq -c '(.data["status.json"] // "") | if . == "" then null else fromjson end' > "$work/previous.json"
 
+# A pass holds the cluster at the target: this controller changes no membership, so no swap holds a new voter beside the old and the expected count is the target.
 config="$(jq -nc \
   --argjson now "$now" --argjson target "$target" --argjson max_servers "$max_servers" \
   --argjson election_timeout_ms "$election_timeout_ms" --argjson sample_seconds "$sample_seconds" \
   --argjson pressure_threshold "$pressure_threshold" --argjson available_floor_percent "$available_floor_percent" \
   --argjson default_quota_bytes "$default_quota_bytes" \
   --arg eligible_label "$eligible_label" --arg pinned_label "$pinned_label" --arg level_label "$level_label" \
-  '{now: $now, target: $target, max_servers: $max_servers, election_timeout_ms: $election_timeout_ms, sample_seconds: $sample_seconds,
+  '{now: $now, target: $target, expected_voters: $target, max_servers: $max_servers, election_timeout_ms: $election_timeout_ms, sample_seconds: $sample_seconds,
     pressure_threshold: $pressure_threshold, available_floor_percent: $available_floor_percent, default_quota_bytes: $default_quota_bytes,
     voter_label: "node-role.kubernetes.io/etcd", eligible_label: $eligible_label, pinned_label: $pinned_label, level_label: $level_label}')"
 

tests/regrade/regrade-lease.sh

@@ -104,6 +104,57 @@ write_lease other-node "$((NOW - 10))"
 release_lease
 check "release leaves a lease that has passed to another node alone" test "$(holder)" = other-node
 
+# Renewing: the holder keeps the Lease past its duration by writing a later renew time, and nobody else may.
+renews() { renew_lease "$1" > /dev/null 2>&1; }
+# Writes a Lease held by $1, acquired at epoch $2, last renewed at epoch $3, carrying an annotation.
+write_annotated_lease() {
+  build_lease "$1" "$2" "$3" 7 | jq '.metadata.annotations = {"example.test/nodes": "node-a node-b"}' > "$lease"
+}
+
+reset_cluster
+write_lease fleet-role "$((NOW - 500))"
+check "renew: the holder renews its own lease" renews fleet-role
+check "renew: the holder is unchanged" test "$(holder)" = fleet-role
+check "renew: the renew time is now" test "$(jq -r '.spec.renewTime' "$lease")" = "$(micro_time "$NOW")"
+check "renew: the lease is replaced, not created" test "$(jq -r '.metadata.resourceVersion' "$lease")" = 8
+
+reset_cluster
+write_annotated_lease fleet-role "$((NOW - 800))" "$((NOW - 500))"
+renews fleet-role || true
+check "renew: the acquire time is kept" test "$(jq -r '.spec.acquireTime' "$lease")" = "$(micro_time "$((NOW - 800))")"
+check "renew: the duration is kept" test "$(jq -r '.spec.leaseDurationSeconds' "$lease")" = 600
+check "renew: annotations survive" test "$(jq -r '.metadata.annotations["example.test/nodes"]' "$lease")" = "node-a node-b"
+
+reset_cluster
+write_lease fleet-role "$((NOW - 599))"
+renews fleet-role || true
+past_original_expiry() { [ "$(lease_state "$(cat "$lease")" fleet-role "$((NOW + 599))")" = mine ]; }
+check "renew: the lease then outlasts the expiry it had before" past_original_expiry
+
+reset_cluster
+write_annotated_lease other-node "$((NOW - 800))" "$((NOW - 100))"
+cp "$lease" "$work/before.json"
+check_not "renew: refused when another holds it" renews fleet-role
+check "renew: a refusal leaves the lease as it was" cmp -s "$lease" "$work/before.json"
+
+reset_cluster
+write_lease '' "$((NOW - 100))"
+check_not "renew: refused when the lease was released" renews fleet-role
+
+reset_cluster
+check_not "renew: refused when there is no lease" renews fleet-role
+
+reset_cluster
+write_lease fleet-role "$((NOW - 100))"
+export FAKE_REPLACE_CONFLICT=1
+check_not "renew: refused when the lease changed while it was being renewed" renews fleet-role
+
+reset_cluster
+write_lease fleet-role "$((NOW - 100))"
+export FAKE_LEASE_READ_ERROR=1
+check_not "renew: refused when the lease cannot be read" renews fleet-role
+unset FAKE_LEASE_READ_ERROR
+
 # The other etcd-voting nodes.
 reset_cluster
 check "peers: all Ready" peers_ready

tests/unit/fleet_role_support.py

@@ -25,6 +25,8 @@
 LINK_LABEL = "github-runner.exadev.com/link"
 LEVEL_LABEL = "github-runner.exadev.com/regrade-level"
 VOTER_LABEL = "node-role.kubernetes.io/etcd"
+# The Node condition k3s sets True on a voting etcd member and not on a learner (setEtcdStatusCondition in pkg/etcd/etcd.go).
+VOTER_CONDITION = "EtcdIsVoter"
 
 # The settings the tests give the controller; the thresholds below derive from these the way the controller derives them, so a test names the rule it holds the program to and not a bare figure.
 ELECTION_TIMEOUT_MS = 1000
@@ -196,6 +198,25 @@ def run(self, script: str, **env: str) -> subprocess.CompletedProcess[str]:
 }
 
 
+def controller_config(*, expected_voters: int = TARGET) -> dict[str, Any]:
+    """The settings object scripts/fleet-role.sh gives the decision program (scripts/fleet-role.jq), for a test that runs the program alone."""
+    return {
+        "now": int(time.time()),
+        "target": TARGET,
+        "expected_voters": expected_voters,
+        "max_servers": MAX_SERVERS,
+        "election_timeout_ms": ELECTION_TIMEOUT_MS,
+        "sample_seconds": SAMPLE_SECONDS,
+        "pressure_threshold": PRESSURE_THRESHOLD,
+        "available_floor_percent": AVAILABLE_FLOOR_PERCENT,
+        "default_quota_bytes": ETCD_QUOTA_BYTES,
+        "voter_label": VOTER_LABEL,
+        "eligible_label": ELIGIBLE_LABEL,
+        "pinned_label": PINNED_LABEL,
+        "level_label": LEVEL_LABEL,
+    }
+
+
 def transition_time(seconds_ago: float) -> str:
     return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime(time.time() - seconds_ago))
 
@@ -204,6 +225,7 @@ def node(
     name: str,
     *,
     voter: bool = False,
+    learner: bool = False,
     eligible: bool = False,
     pinned: bool = False,
     ready: bool = True,
@@ -212,9 +234,9 @@ def node(
     address: str | None = None,
     link: str | None = None,
 ) -> dict[str, Any]:
-    """A Node as `kubectl get nodes -o json` lists it; ready_for is how long ago its Ready condition last changed, and link the medium the cluster role labelled it with, none when it has no such label."""
+    """A Node as `kubectl get nodes -o json` lists it; a voter carries the etcd label and the EtcdIsVoter condition, a learner the label alone (k3s labels a member when it joins and sets the condition when it votes), ready_for is how long ago its Ready condition last changed, and link the medium the cluster role labelled it with, none when it has no such label."""
     labels: dict[str, str] = {}
-    if voter:
+    if voter or learner:
         labels[VOTER_LABEL] = "true"
     if eligible:
         labels[ELIGIBLE_LABEL] = "true"
@@ -224,10 +246,13 @@ def node(
         labels[LEVEL_LABEL] = level
     if link is not None:
         labels[LINK_LABEL] = link
+    conditions = [{"type": "Ready", "status": "True" if ready else "Unknown", "lastTransitionTime": transition_time(ready_for)}]
+    if voter:
+        conditions.append({"type": VOTER_CONDITION, "status": "True", "lastTransitionTime": transition_time(ready_for)})
     return {
         "metadata": {"name": name, "labels": labels},
         "status": {
-            "conditions": [{"type": "Ready", "status": "True" if ready else "Unknown", "lastTransitionTime": transition_time(ready_for)}],
+            "conditions": conditions,
             "addresses": [{"type": "InternalIP", "address": address or "127.0.0.1"}, {"type": "Hostname", "address": name}],
         },
     }