diff --git a/scripts/fleet-role.jq b/scripts/fleet-role.jq index 7b1db03b..8e6ce583 100644 --- a/scripts/fleet-role.jq +++ b/scripts/fleet-role.jq @@ -70,6 +70,7 @@ $config as $c voter: ($labelled and condition_true(voter_condition) and ((.metadata.annotations // {})[removed_annotation] // "") == ""), eligible: ($labels[$c.eligible_label] == "true"), pinned: ($labels[$c.pinned_label] == "true"), + cordoned: (.spec.unschedulable // false), removed: (((.metadata.annotations // {})[removed_annotation] // "") != ""), regrade_level: $labels[$c.level_label]}] as $all | ($all | map(select(.voter)) | sort_by(.name)) as $voters @@ -192,6 +193,7 @@ $config as $c role: (if $node.voter then "voter" else "standby" end), eligible: $node.eligible, pinned: $node.pinned, + cordoned: $node.cordoned, verdict: (if ($gaps | length) > 0 then "unknown" elif ($faults | length) > 0 then "unfit" else "fit" end), reasons: (if ($gaps | length) > 0 then $gaps else $faults end), link: $m.link, @@ -274,7 +276,7 @@ $config as $c # The swap. A slow link makes both of its ends unfit, since a voter is unfit when its round trip to any other voter is, so the voter to replace is the one at fault: the unfit voter the most hosts' unfit verdicts are attributed to, the worst-ranked among those with as many. It goes out only if it 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. The best fit eligible standby that ranks above it comes in, judged against the voters that would remain. | ([$hosts[] | select(.role == "voter" and .verdict == "unfit") | . + {accounts_for: ($attribution[.name] // [] | length)}] | sort_by(.rank) | reverse | sort_by(-.accounts_for) | first) as $culprit | (if $culprit != null and $culprit.eligible and ($culprit.pinned | not) then $culprit else null end) as $out -| ([$hosts[] | select(.role == "standby" and .eligible) +| ([$hosts[] | select(.role == "standby" and .eligible and (.cordoned | not)) | (if $out == null then .m else (.m | without($out.name)) end) as $m | select((gaps($m) | length) == 0 and (faults($m) | length) == 0) | . + {rank: rank($m)}] | sort_by(.rank)) as $fit_candidates diff --git a/tests/unit/fleet_role_support.py b/tests/unit/fleet_role_support.py index 5471304e..7df64045 100644 --- a/tests/unit/fleet_role_support.py +++ b/tests/unit/fleet_role_support.py @@ -364,6 +364,7 @@ def node( learner: bool = False, eligible: bool = False, pinned: bool = False, + cordoned: bool = False, ready: bool = True, ready_for: float = 24 * 3600, level: str | None = None, @@ -387,6 +388,7 @@ def node( conditions.append({"type": VOTER_CONDITION, "status": "True", "lastTransitionTime": transition_time(ready_for)}) return { "metadata": {"name": name, "labels": labels}, + "spec": {"unschedulable": True} if cordoned else {}, "status": { "conditions": conditions, "addresses": [{"type": "InternalIP", "address": address or "127.0.0.1"}, {"type": "Hostname", "address": name}], diff --git a/tests/unit/test_fleet_role.py b/tests/unit/test_fleet_role.py index da13f3c0..0249124f 100644 --- a/tests/unit/test_fleet_role.py +++ b/tests/unit/test_fleet_role.py @@ -55,6 +55,8 @@ class Host: learner: bool = False eligible: bool = False pinned: bool = False + # An operator's cordon: the host takes no new work, so it is never the voter that comes in. + cordoned: bool = False ready: bool = True ready_for: float = 24 * 3600 level: str | None = None @@ -96,7 +98,7 @@ def populate(cluster: Cluster, hosts: list[Host]) -> None: """Gives the cluster these Nodes and, for each host, a probe entry of its last `samples` samples ending now (less stale_by).""" voters = [h.name for h in hosts if h.voter] cluster.nodes = [ - node(h.name, voter=h.voter, learner=h.learner, eligible=h.eligible, pinned=h.pinned, ready=h.ready, ready_for=h.ready_for, level=h.level) for h in hosts + node(h.name, voter=h.voter, learner=h.learner, eligible=h.eligible, pinned=h.pinned, cordoned=h.cordoned, ready=h.ready, ready_for=h.ready_for, level=h.level) for h in hosts ] entries = {} for h in hosts: @@ -389,6 +391,13 @@ def test_within_a_link_type_the_lower_round_trip_ranks_first_then_the_lower_fsyn hosts += [Host("e", eligible=True, **{**base, **overrides["e"]}), Host("f", eligible=True, **{**base, **overrides["f"]})] self.assertEqual(self.pass_(hosts)["decision"]["swap"], {"out": "a", "in": expected}) + def test_a_cordoned_standby_is_never_the_voter_that_comes_in(self) -> None: + hosts = healthy(a={"tcp": UNFIT_RTT_MS + 50}, d={"cordoned": True}) + status = self.pass_(hosts) + self.assertEqual(status["decision"]["action"], "none") + self.assertEqual(status["decision"]["because"][0], "voter a is unfit and no eligible standby is fit and ranks above it") + self.assertTrue(status["hosts"]["d"]["cordoned"]) + def test_a_candidate_must_rank_above_the_voter_it_replaces(self) -> None: # The unfit voter is wired; the only fit standby is on a link the cluster cannot see, which ranks below wired. hosts = healthy(a={"tcp": UNFIT_RTT_MS + 50}, d={"link": "unknown"})