From 581c9b9261b8556e691465880e9fcb6ef3387d76 Mon Sep 17 00:00:00 2001 From: stxkxs <139715017+stxkxs@users.noreply.github.com> Date: Fri, 4 Sep 2026 22:27:15 -0700 Subject: [PATCH 1/2] Derive the burn-rate budget figure from the expression that spends it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A ticket-tier rule's summary claimed the whole error budget where its own expression spends a tenth of it. ─── The defect ─── dashboards/base/alerting/agent-operator.yaml, the factor-1 / 3d tier: summary: operator reconcile latency budget burning (100% over 3d) Burn factor 1 spends the budget at exactly the rate that exhausts it over the SLO window, so 3d of it consumes 3/30 = 10%. The expression is correct; only the sentence is wrong, which is why every rule-level validation passed it. A summary is a free-text annotation and nothing parsed it. The tier carries severity: ticket, the least urgent the file ships, and its title described total budget exhaustion — the most urgent condition an SLO has. A summary is the alert TITLE: the sentence that reaches a human first and often the only one they read. Escalated, it is an outage that is not happening. Recognised as wrong, it teaches on-call that these titles are not to be trusted, which spends the credibility of the three tiers that are right. Fixed to `budget burning slowest (10% in 3d)`. The 1d and 3d tiers both consume 10% — 3 x 1d and 1 x 3d are the same spend — so the adverb separates them, not the figure. ─── scripts/check-burn-rate-budgets.py ─── Fixing the string closes the instance. A figure that agrees with the standard today and is compared to nothing is the same defect waiting, so the gate derives it rather than listing it: budget consumed = burn factor x long window / SLO window Every term is read from a different place in the tree. The factor and the long window come from the rule's own expression. The SLO window comes from the dashboard panel measuring the same metric selector over its longest explicit range — dashboards/base/platform/agent-operator.yaml computes the objective over 30d on the same controller_runtime_reconcile_time_seconds_bucket the rules burn against, and portal and fleet-vend pair the same way. That panel is edited by different work than an alert summary, so it cannot agree with a wrong figure by construction. Arithmetic is exact (fractions.Fraction), so the comparison is an equality rather than a tolerance. A tolerance loose enough for the factors this catalog uses is loose enough to miss a one-step drift. Rules enter the corpus on their EXPRESSION, never on their summary. Selecting on the sentence would let a rule leave the corpus by having its claim deleted, and that is the edit most likely to accompany a wrong one — so a burn rule stating no figure is a finding rather than a skip. The burn factor is the comparison against the budget-normalised ratio, `/ > bool `. Matching `> bool` alone also matches the traffic guard four of these rules carry — `sum(rate(...)) > bool 0.0167`, roughly one request a minute, there to stop an idle service alerting on a ratio computed from no traffic. Read as a factor it makes four correct rules look like they each state two contradictory rates of spend. The dashboard walk is imported from check-athena-panel-columns.py rather than copied, so the two gates cannot come to disagree about which dashboards exist. ─── Registration ─── CI runs it in the dashboards job beside the alert-coverage gate. controls.py plants the original figure and requires a rejection naming the file; reverify-gates.sh runs it green and plants the same defect, floor 37 -> 39 against 42 run lines. empty-corpus.py picks it up by discovery and classifies its refusal as one about a derivation input rather than its corpus. ─── scripts/tests/run.py ─── MAX_UNCOVERED_GATES 11 -> 10, not 9. The run reports 9 of 25 against a baseline of 10 of 24, but two things moved: this gate arrived with tests, and check-athena-panel-columns.py flipped to covered because this gate imports it. Only the first is coverage. The comment names the third import-covered file beside the two it already named, and says the number moves by what a change covered rather than by what the run reports. ─── .github/workflows/ci.yml ─── The alert-coverage step's comment carried its last sentence twice. --- .github/workflows/ci.yml | 15 +- CLAUDE.md | 9 + dashboards/base/alerting/agent-operator.yaml | 2 +- scripts/check-burn-rate-budgets.py | 350 +++++++++++++++++++ scripts/tests/controls.py | 17 + scripts/tests/reverify-gates.sh | 18 +- scripts/tests/run.py | 25 +- scripts/tests/test_burn_rate_budgets.py | 311 ++++++++++++++++ 8 files changed, 734 insertions(+), 13 deletions(-) create mode 100755 scripts/check-burn-rate-budgets.py create mode 100644 scripts/tests/test_burn_rate_budgets.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24da9a1..ed3841e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -112,12 +112,21 @@ jobs: # rules can each be individually correct and leave the state reported by # nobody. kubeconform SKIPs the Grafana CRDs and no gate executes the # PromQL, so an alert that can never fire looks exactly like one that has - # not fired yet. kubeconform SKIPs the Grafana CRDs and no gate - # executes the PromQL, so an alert that can never fire looks exactly like - # one that has not fired yet. + # not fired yet. - name: Excluded alert states are somebody's job run: ./scripts/check-alert-coverage.py + # A burn-rate summary is the alert TITLE — the sentence that reaches a + # human first and often the only one they read. The figure in it is prose + # that nothing parses, so a rule can name a budget its own expression does + # not spend and pass every rule-level validation, because the expression is + # correct and only the sentence is wrong. This derives the figure from the + # rule's factor and window against the objective the dashboards measure, so + # there is no constant here to agree with the standard today and be + # compared to nothing tomorrow. + - name: Burn-rate summaries state the budget their expression spends + run: ./scripts/check-burn-rate-budgets.py + # ── Kyverno policy unit tests ──────────────────────────────────────── # `kyverno test` proves each policy rule PASSES a compliant resource and FAILS # a violating one (policies/kyverno/tests). Without this, a policy could be diff --git a/CLAUDE.md b/CLAUDE.md index 054ca41..d7088e1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -166,6 +166,15 @@ aggregate, so a clean `task validate` is necessary but not sufficient: `scripts/check-catalog-revision.py` (CI job `catalog-revision`) - **Alert coverage** — `scripts/check-alert-coverage.py`, which runs inside the `dashboards` job alongside the locally-available dashboard and Athena gates +- **Burn-rate budget claims** — `scripts/check-burn-rate-budgets.py`, in the same + `dashboards` job. A burn-rate summary is the alert title, and the budget figure + in it is prose nothing parses, so a rule can name a figure its own expression + does not spend while every rule-level validation passes. The figure is derived + rather than listed: burn factor times long window over the SLO window, with the + factor and window read from the rule's expression and the SLO window from the + dashboard panel measuring the same selector. A rule whose selectors no panel + measures is a finding, because a figure compared to nothing is how the last + wrong one survived - **Image vulnerabilities** — `scripts/check-image-vulnerabilities.py` (CI job `image-vulnerabilities`). The one with a target of its own, `task validate:image-vulnerabilities`, kept out of the aggregate because it diff --git a/dashboards/base/alerting/agent-operator.yaml b/dashboards/base/alerting/agent-operator.yaml index 3ac96dd..402d7ba 100644 --- a/dashboards/base/alerting/agent-operator.yaml +++ b/dashboards/base/alerting/agent-operator.yaml @@ -175,7 +175,7 @@ spec: service: eks-agent-platform component: operator annotations: - summary: operator reconcile latency budget burning (100% over 3d) + summary: operator reconcile latency budget burning slowest (10% in 3d) description: > The fraction of reconciles over 1s exceeds 1x the 99% latency objective over both the 3d and 6h windows — the budget is being spent at exactly the diff --git a/scripts/check-burn-rate-budgets.py b/scripts/check-burn-rate-budgets.py new file mode 100755 index 0000000..e8d99d6 --- /dev/null +++ b/scripts/check-burn-rate-budgets.py @@ -0,0 +1,350 @@ +#!/usr/bin/env python3 +"""The budget a burn-rate alert claims is the one its own expression spends. + +WHY THIS EXISTS + +A multi-window burn-rate rule fires on a rate of spend, and its summary is the +alert TITLE — the sentence that reaches a human first and often the only one +they read. One ticket-tier rule said: + + summary: operator reconcile latency budget burning (100% over 3d) + +for an expression comparing against a burn factor of 1 over a 3d window. Factor +1 spends the budget at exactly the rate that exhausts it over the SLO window, so +three days of it consumes three days' worth: 10% of a 30d budget, not 100%. The +title described total exhaustion — the most urgent condition an SLO has — on the +least urgent tier the file ships. + +Both readings of that title are damaging. Escalated, it is an outage that is not +happening. Recognised as wrong, it teaches on-call that these titles are not to +be trusted, which spends the credibility of the three tiers that are right. + +Nothing could see it. The expression is correct; only the sentence describing it +is wrong, so every rule-level validation passes. kubeconform SKIPs the +grafana.integreatly.org CRDs, no gate executes PromQL, and a summary is +free-text that nothing parses. + +WHAT IT CHECKS + +The claim is checked against arithmetic, not against a table of expected +figures. For a burn rule with factor `f` over long window `w`, against an SLO +window `W`: + + budget consumed = f * w / W + +Every term comes from the tree: + + * `f` and `w` from the rule's own expression — the `> bool f` threshold and + the longest range selector in it; + * `W` from the dashboard panel measuring the same metric selector over its + longest explicit range. That is the panel that displays the objective this + rule burns against, in a different file, edited by different work. + +So there is no constant here to agree with the standard today and be compared to +nothing tomorrow. A figure that drifts fails against the expression that +produces it, and an SLO window that moves fails against the panel that measures +it. + +Every rule whose expression is a burn-rate expression must carry the claim. A +burn-rate title that does not say how much budget is at stake tells the reader +nothing they can act on, and leaving the claim out would otherwise remove the +rule from this gate's corpus without removing the rule. + +WHAT IT DOES NOT CHECK + + * Whether the SLO objective is the right one. `W` is read from the panel that + measures it; that the business wants 30d rather than 28d is not a fact in + this repository. + * Whether the factors implement the alerting policy correctly. That 14.4 over + 1h is the intended fast tier is a design decision; this asserts only that + the sentence and the expression describe the same spend. + * Whether the rule fires. The expression is read, never executed. + * The `description` under each summary, which states the same thing in prose + and in a different shape per file. The summary is checked because it is the + title — the sentence that reaches a human whether or not they open the rule. + * Agreement with the org SLO standard. The standard is the authority for these + figures and it is not in this repository, so what is asserted here is that + the catalog agrees with ITSELF: the sentence, the expression, and the panel + measuring the objective all describe one spend. A window that moved in the + standard and nowhere else would leave this green. +""" + +from __future__ import annotations + +import importlib.util +import pathlib +import re +import sys +from fractions import Fraction + +_here = pathlib.Path(__file__).resolve().parent + +_gl = _here / "gatelib.py" +_gs = importlib.util.spec_from_file_location("gatelib", _gl) +assert _gs and _gs.loader, f"{_gl} is not loadable as a module" +gatelib = importlib.util.module_from_spec(_gs) +sys.modules["gatelib"] = gatelib +_gs.loader.exec_module(gatelib) + +# The dashboard walk lives in the gate that already owns it. Copying it here +# would give this gate a second walk to keep in step with the first, and a +# dashboard silently dropped from either corpus reports the same as a clean run. +_ap = _here / "check-athena-panel-columns.py" +_as = importlib.util.spec_from_file_location("check_athena_panel_columns", _ap) +assert _as and _as.loader, f"{_ap} is not loadable as a module" +panels = importlib.util.module_from_spec(_as) +_as.loader.exec_module(panels) + +ROOT = _here.parent +ALERT_DIR = ROOT / "dashboards" / "base" / "alerting" +RULE_GROUP = "GrafanaAlertRuleGroup" + +UNIT_SECONDS = {"s": 1, "m": 60, "h": 3600, "d": 86400, "w": 604800} + +# `metric{labels}[range]` — the labels are kept because they are part of what a +# panel has to match to be measuring the same thing. +SELECTOR = re.compile( + r"(?P[a-zA-Z_:][a-zA-Z0-9_:]*(?:\{[^}]*\})?)\[(?P\d+[smhdw])\]") +# The burn factor, and what separates it from every other `> bool` in a rule. +# A burn comparison is made against the error ratio NORMALISED by the budget: +# ` / > bool `. Matching `> bool` alone also +# matches the traffic guard these rules carry — `sum(rate(...)) > bool 0.0167`, +# roughly one request a minute, there to stop an idle service alerting on a +# ratio computed from nothing. Read as a factor it makes a rule look like two +# contradictory claims about one rate of spend. +FACTOR = re.compile( + r"/\s*(?P0\.\d+)\s*>\s*bool\s+(?P\d+(?:\.\d+)?)") +# The claim in the summary: a percentage of budget over a window. +CLAIM = re.compile( + r"(?P\d+(?:\.(?P\d+))?)%\s+(?:in|over)\s+(?P\d+[smhdw])") + + +def seconds(duration: str) -> int: + """A PromQL duration in seconds. The grammar here is a single unit, which + is what every range in this catalog uses; a compound `1h30m` would not match + SELECTOR and so would never reach this.""" + return int(duration[:-1]) * UNIT_SECONDS[duration[-1]] + + +def human(total: int) -> str: + """Seconds as the largest whole unit that divides them, for messages that a + reader compares against a duration written in the file.""" + for unit in ("w", "d", "h", "m"): + size = UNIT_SECONDS[unit] + if total >= size and total % size == 0: + return f"{total // size}{unit}" + return f"{total}s" + + +def rule_expr(rule: dict) -> str: + """The rule's query, as one string. + + A Grafana rule carries a list of `data` stages; the burn arithmetic is in + the datasource query, and the later stages are threshold expressions over + its result. + """ + for stage in rule.get("data") or []: + if not isinstance(stage, dict): + continue + expr = (stage.get("model") or {}).get("expr") + if isinstance(expr, str) and expr.strip(): + return expr + return "" + + +def burn_rules(alert_dir: pathlib.Path): + """(path, group, rule, expr) for every rule whose query is a burn-rate one. + + Selected on the EXPRESSION rather than on the summary. Selecting on the + sentence would let a rule leave this gate's corpus by having its claim + deleted, which is the one edit most likely to accompany a wrong figure. + """ + for path in sorted(alert_dir.glob("*.yaml")): + for doc in gatelib.read_yaml_all(path): + if not isinstance(doc, dict) or doc.get("kind") != RULE_GROUP: + continue + group = (doc.get("metadata") or {}).get("name", path.stem) + for rule in (doc.get("spec") or {}).get("rules") or []: + if not isinstance(rule, dict): + continue + expr = rule_expr(rule) + if FACTOR.search(expr) and SELECTOR.search(expr): + yield path, str(group), rule, expr + + +def long_window(expr: str) -> tuple[int, set[str]]: + """The longest range in `expr`, and the selectors carrying it. + + A dual-window burn rule ANDs a long window against a short confirmation + window. The long one is the period the claim is about; the short one exists + to stop the alert firing on a spike that has already stopped. + """ + ranges: dict[int, set[str]] = {} + for m in SELECTOR.finditer(expr): + ranges.setdefault(seconds(m.group("range")), set()).add( + re.sub(r"\s+", "", m.group("sel"))) + longest = max(ranges) + return longest, ranges[longest] + + +def factors(expr: str) -> set[Fraction]: + """Every burn factor the expression compares against. + + A set, because a dual-window rule states the same factor twice and a rule + stating two different ones is not a single claim about a rate of spend. + """ + return {Fraction(m.group("factor")) for m in FACTOR.finditer(expr)} + + +def objective_windows() -> dict[str, int]: + """selector -> the longest explicit range a dashboard panel applies to it. + + The SLO window, read from the panel that displays the objective. A burn + window is by construction shorter than the window it burns against, so the + longest range a panel measures this selector over is the objective's. + """ + out: dict[str, int] = {} + for _path, dash in panels.dashboards(): + for expr in panel_exprs(dash): + for m in SELECTOR.finditer(expr): + sel = re.sub(r"\s+", "", m.group("sel")) + out[sel] = max(out.get(sel, 0), seconds(m.group("range"))) + return out + + +def panel_exprs(node) -> list[str]: + """Every `expr` under a dashboard, rows and nested panels included.""" + found: list[str] = [] + if isinstance(node, dict): + expr = node.get("expr") + if isinstance(expr, str) and expr.strip(): + found.append(expr) + for value in node.values(): + found.extend(panel_exprs(value)) + elif isinstance(node, list): + for item in node: + found.extend(panel_exprs(item)) + return found + + +def decimal(value: Fraction) -> str: + """A Fraction as the decimal a reader will find in the expression. + + Exact arithmetic is what lets the comparison be an equality rather than a + tolerance, but `72/5` is not the string anyone can search the file for. + """ + if value.denominator == 1: + return str(value.numerator) + return f"{float(value):g}" + + +def as_percent(value: Fraction, places: int) -> str: + """`value` as a percentage string with `places` decimals, no float.""" + scaled = value * 100 + if places == 0: + return str(round(scaled)) + quantum = Fraction(10) ** places + return f"{float(round(scaled * quantum) / quantum):.{places}f}" + + +def main() -> int: + if not ALERT_DIR.is_dir(): + print(f"Cannot run: {ALERT_DIR.relative_to(ROOT)} does not exist, so the " + f"burn-rate rules whose claims this gate checks are not there.") + print("This gate examined nothing, which is not the same as finding nothing.") + return gatelib.CANNOT_RUN + + rules = list(burn_rules(ALERT_DIR)) + if not rules: + print(f"Cannot run: no burn-rate rule found under " + f"{ALERT_DIR.relative_to(ROOT)}. A run over no rule reports what a " + f"catalog of correct claims reports.") + return gatelib.CANNOT_RUN + + windows = objective_windows() + failures: list[str] = [] + checked = 0 + + for path, group, rule, expr in rules: + title = str(rule.get("title") or rule.get("uid") or "") + where = f"{path.name}: {group}/{title}" + + seen = factors(expr) + if len(seen) != 1: + failures.append( + f"{where} compares against {len(seen)} different burn factors " + f"({', '.join(decimal(f) for f in sorted(seen))}), so there is no one " + f"rate of spend for its summary to be describing.") + continue + factor = seen.pop() + + window, selectors = long_window(expr) + + summary = str((rule.get("annotations") or {}).get("summary") or "") + claim = CLAIM.search(summary) + if not claim: + failures.append( + f"{where} is a burn-rate rule and its summary states no budget " + f"figure: {summary!r}. The summary is the alert title, and a burn " + f"title that does not say how much budget is at stake gives the " + f"reader nothing to act on.") + continue + + stated_window = seconds(claim.group("window")) + if stated_window != window: + failures.append( + f"{where} says {claim.group(0)!r} and its expression measures over " + f"{human(window)}. The sentence and the query describe different " + f"periods, so one of them is about an alert that does not exist.") + continue + + objective = {windows[sel] for sel in selectors if sel in windows} + if not objective: + failures.append( + f"{where} claims {claim.group(0)!r} and no dashboard panel measures " + f"{' or '.join(sorted(selectors))} over an explicit range. The " + f"figure is compared to nothing, which is how the last wrong one " + f"survived.") + continue + if len(objective) > 1: + failures.append( + f"{where} burns against selectors whose panels measure them over " + f"{', '.join(human(w) for w in sorted(objective))}. Which of those " + f"is the SLO window decides the figure, and the tree states both.") + continue + slo = objective.pop() + + places = len(claim.group("frac") or "") + derived = factor * Fraction(window, slo) + stated = Fraction(claim.group("pct")) / 100 + if as_percent(derived, places) != as_percent(stated, places): + failures.append( + f"{where} claims {claim.group(0)!r} and its expression spends " + f"{as_percent(derived, places)}% over {human(window)}: burn factor " + f"{decimal(factor)} against a {human(slo)} objective is " + f"{decimal(factor)} x {human(window)} / {human(slo)}. This " + f"sentence is the alert title.") + continue + checked += 1 + + if failures: + print(f"{len(failures)} burn-rate budget claim(s) the expression does not " + f"support:\n") + for f in failures: + print(f" {f}") + return 1 + + objectives = sorted({human(windows[sel]) + for _p, _g, _r, e in rules + for sel in long_window(e)[1] if sel in windows}) + print(f"✓ every burn-rate summary states the budget its expression spends: " + f"{checked} rule(s) across {len({g for _p, g, _r, _e in rules})} group(s), " + f"each derived from its own factor and window against the " + f"{', '.join(objectives)} objective(s) the dashboards measure") + print(" the objective itself, the choice of factors, and whether any rule " + "fires are outside this claim") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/controls.py b/scripts/tests/controls.py index ed1d26d..e224a62 100755 --- a/scripts/tests/controls.py +++ b/scripts/tests/controls.py @@ -489,6 +489,21 @@ def m_alert_coverage(root): marker='lastRunAt\\"} == 0') +def m_burn_rate_budgets(root): + """Put back the figure this gate was written for. + + A ticket-tier rule whose expression compares against a burn factor of 1 over + 3d, claiming the budget is fully consumed. Factor 1 spends at exactly the + rate that exhausts the budget over the SLO window, so 3d of it is 10% of a + 30d budget. The expression stays correct and only the sentence changes, + which is what every rule-level validation passes. + """ + return _sub(root, "dashboards/base/alerting/agent-operator.yaml", + "budget burning slowest (10% in 3d)", + "budget burning (100% over 3d)", + marker="100% over 3d") + + def m_chart_deprecation(root): """A recorded chart that nothing pins, which the offline gate must reject.""" import json @@ -562,6 +577,8 @@ def m_env_coverage(root): "check-policy-validity.py": ("a structurally invalid ClusterPolicy", m_policy_validity), "check-serviceaccount-bindings.py": ("a pod naming an absent ServiceAccount", m_serviceaccount_bindings), "check-alert-coverage.py": ("an alert on an unexported KSM field", m_alert_coverage), + "check-burn-rate-budgets.py": ("a summary claiming a budget its expression " + "does not spend", m_burn_rate_budgets), } diff --git a/scripts/tests/reverify-gates.sh b/scripts/tests/reverify-gates.sh index 17d0a77..23a0588 100755 --- a/scripts/tests/reverify-gates.sh +++ b/scripts/tests/reverify-gates.sh @@ -157,6 +157,7 @@ run 0 "image-pins: a helm reporting OCI pulls on stdout" \ run 0 "check-renovate-coverage.py" ./scripts/check-renovate-coverage.py run 0 "check-ai-config.py" ./scripts/check-ai-config.py run 0 "check-env-coverage.py" ./scripts/check-env-coverage.py +run 0 "check-burn-rate-budgets.py" ./scripts/check-burn-rate-budgets.py run 0 "check-named-things.py" ./scripts/check-named-things.py run 0 "check-policy-validity.py" ./scripts/check-policy-validity.py run 0 "no-placeholders.sh" ./scripts/no-placeholders.sh @@ -352,6 +353,21 @@ PY run nonzero "ai-config: global. geo prefix" ./scripts/check-ai-config.py res $F +# The number an on-call reads first. The expression stays correct and only the +# sentence changes, which is every rule-level validation's blind spot. +F=dashboards/base/alerting/agent-operator.yaml; mut $F +python3 - "$F" <<'PYX' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace("budget burning slowest (10% in 3d)", "budget burning (100% over 3d)", 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" claimed the 3d tier consumes the whole budget") +PYX +run nonzero "burn-rate-budgets: a summary claiming a budget its expression does not spend" \ + ./scripts/check-burn-rate-budgets.py +res $F + F=addons/bootstrap/cert-manager/values-hub.yaml; mut $F; rm -f $F run nonzero "env-coverage: deleted hub delta" ./scripts/check-env-coverage.py res $F @@ -464,7 +480,7 @@ echo "RESULT pass=$pass fail=$fail" # The harness owes the same assertion it demands of the gates: with every `run` # line deleted it would report pass=0 fail=0 and exit 0, which is a green run # over nothing checked. -MIN_CHECKS=37 +MIN_CHECKS=39 total=$((pass + fail)) if [ "$total" -lt "$MIN_CHECKS" ]; then echo "FAIL ran $total check(s), under the floor of $MIN_CHECKS — this harness" diff --git a/scripts/tests/run.py b/scripts/tests/run.py index eeebf8c..48df237 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -61,6 +61,9 @@ # The floors that keep an emptied corpus from reading as a clean one, # asserted apart from the gates that carry them. "test_corpus_floors", + # The arithmetic behind a figure an on-call reads at the worst moment, and + # the anchoring that keeps it from being a constant compared to nothing. + "test_burn_rate_budgets", ) # A floor well under the real count. It catches "discovery found almost nothing", @@ -144,16 +147,22 @@ # Ratchets downward only. Adding a gate without tests fails here rather than # diluting the combined figure by a percentage point nobody notices. # -# Two of the files this counts as covered are covered by IMPORT, not by tests. +# Some of the files this counts as covered are covered by IMPORT, not by tests. # check-image-vulnerabilities.py loads check-image-pins.py by path so the two # cannot disagree about what the fleet is, and that loads render-addons.py in -# turn; a test module importing the first executes the module bodies of all -# three. Coverage cannot tell that from a test, so both read as non-zero while -# carrying no assertions of their own. The ceiling is lowered anyway, because a -# ratchet that declines to move is one nobody can regress against — but it is -# the weaker half of this file's claim, and the per-gate floors and the controls -# in scripts/tests/controls.py are where the real one lives. -MAX_UNCOVERED_GATES = 11 +# turn; check-burn-rate-budgets.py loads check-athena-panel-columns.py for the +# dashboard walk, for the same reason. A test module importing any of those +# executes the module bodies of the ones underneath. Coverage cannot tell that +# from a test, so they read as non-zero while carrying no assertions of their +# own. The ceiling is lowered anyway, because a ratchet that declines to move is +# one nobody can regress against — but it is the weaker half of this file's +# claim, and the per-gate floors and the controls in scripts/tests/controls.py +# are where the real one lives. +# +# So this number moves by what a change actually covered, not by what the run +# reports. A gate arriving with its own tests lowers it; a gate that starts +# reading as covered because something new imports it does not. +MAX_UNCOVERED_GATES = 10 PER_GATE_FLOORS = { "scripts/check-named-things.py": 35, diff --git a/scripts/tests/test_burn_rate_budgets.py b/scripts/tests/test_burn_rate_budgets.py new file mode 100644 index 0000000..3be0925 --- /dev/null +++ b/scripts/tests/test_burn_rate_budgets.py @@ -0,0 +1,311 @@ +"""Unit tests for the burn-rate budget gate. + +The defect this gate exists for was a number that agreed with nothing: a +summary claiming 100% budget consumed where its own expression spends 10%. So +the cases here concentrate on the two ways a gate like this reports a false +pass — reading the figure from somewhere that cannot disagree with it, and +quietly dropping a rule out of the corpus so its claim is never read. + +Every term of the arithmetic comes from a different place in the tree, and each +of those places gets a fixture that moves it. +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import tempfile +import unittest +from fractions import Fraction + +import yaml +from gateloader import load + +gate = load("check-burn-rate-budgets") + +ROOT = pathlib.Path(__file__).resolve().parent.parent.parent + + +def burn_expr(budget, factor, long_window, short_window, + guard=None, numerator=None, denominator=None): + """A dual-window burn expression in the shape this catalog writes them. + + Two metric selectors by default, because the real rules divide one by + another and a rule whose two selectors disagree about the objective is one + of the cases below. + """ + num = numerator or NUMERATOR + den = denominator or DENOMINATOR + + def half(window): + return (f"(sum(rate({num}[{window}])) / " + f"clamp_min(sum(rate({den}[{window}])), 0.001) / " + f"{budget} > bool {factor})") + + expr = half(long_window) + " * " + half(short_window) + if guard is not None: + expr += f" * (sum(rate({den}[{long_window}])) > bool {guard})" + return expr + + +def rule(title, summary, expr): + return {"uid": title.lower(), "title": title, + "annotations": {"summary": summary}, + "data": [{"refId": "A", "model": {"refId": "A", "expr": expr}}, + {"refId": "B", "model": {"refId": "B", "expression": "A"}}]} + + +def group(name, *rules): + return {"apiVersion": "grafana.integreatly.org/v1beta1", + "kind": "GrafanaAlertRuleGroup", + "metadata": {"name": name}, + "spec": {"rules": list(rules)}} + + +def dashboard(name, *exprs): + return {"apiVersion": "grafana.integreatly.org/v1beta1", + "kind": "GrafanaDashboard", + "metadata": {"name": name}, + "spec": {"json": json.dumps( + {"title": name, + "panels": [{"type": "timeseries", "title": f"p{i}", + "targets": [{"expr": e}]} + for i, e in enumerate(exprs)]})}} + + +NUMERATOR = 'svc_request_duration_seconds_bucket{job="svc",le="1"}' +DENOMINATOR = 'svc_request_duration_seconds_count{job="svc"}' +PANELS = (f"sum(rate({NUMERATOR}[30d])) / sum(rate({DENOMINATOR}[30d]))",) + + +class ReadingTheExpression(unittest.TestCase): + """What the rule itself states, before anything is compared to it.""" + + def test_the_burn_factor_is_the_one_divided_by_the_budget(self): + expr = burn_expr("0.01", "14.4", "1h", "5m") + self.assertEqual(gate.factors(expr), {Fraction("14.4")}) + + def test_a_traffic_guard_is_not_a_burn_factor(self): + """`sum(rate(...)) > bool 0.0167` keeps an idle service from alerting on + a ratio computed from no requests. Counted as a factor it makes a rule + look like two contradictory claims about one rate of spend, and the + rules carrying one are the majority of this catalog.""" + expr = burn_expr("0.001", "14.4", "1h", "5m", guard="0.0167") + self.assertEqual(gate.factors(expr), {Fraction("14.4")}) + + def test_the_long_window_is_the_period_the_claim_is_about(self): + expr = burn_expr("0.01", "6", "6h", "30m") + window, selectors = gate.long_window(expr) + self.assertEqual(window, 6 * 3600) + self.assertIn(NUMERATOR, selectors) + + def test_the_short_confirmation_window_is_not_taken_for_it(self): + """The short window exists to stop a spike that has already stopped from + firing; a claim about it would be a claim about a different alert.""" + expr = burn_expr("0.01", "14.4", "1h", "5m") + self.assertEqual(gate.long_window(expr)[0], 3600) + + def test_a_rule_with_no_burn_comparison_is_not_in_the_corpus(self): + found = list(gate.burn_rules(self.planted())) + self.assertEqual([r["title"] for _p, _g, r, _e in found], ["Burn"]) + + def planted(self): + d = pathlib.Path(tempfile.mkdtemp()) + (d / "g.yaml").write_text(yaml.safe_dump(group( + "g", + rule("Burn", "burning (2% in 1h)", + burn_expr("0.01", "14.4", "1h", "5m")), + rule("Rate", "error rate above 5%", + f"sum(rate({DENOMINATOR}[5m])) > 0.05")))) + return d + + +class TheDurationGrammar(unittest.TestCase): + def test_units_convert(self): + for text, want in (("5m", 300), ("1h", 3600), ("1d", 86400), + ("3d", 259200), ("30d", 2592000), ("1w", 604800)): + with self.subTest(duration=text): + self.assertEqual(gate.seconds(text), want) + + def test_seconds_render_as_the_unit_a_file_would_write(self): + for total, want in ((2592000, "30d"), (259200, "3d"), (3600, "1h"), + (1800, "30m"), (604800, "1w")): + with self.subTest(seconds=total): + self.assertEqual(gate.human(total), want) + + def test_a_factor_renders_as_the_decimal_in_the_expression(self): + """Exact arithmetic is what makes the comparison an equality rather than + a tolerance, but `72/5` is not a string anyone can search the file for.""" + self.assertEqual(gate.decimal(Fraction("14.4")), "14.4") + self.assertEqual(gate.decimal(Fraction(1)), "1") + self.assertEqual(gate.decimal(Fraction(6)), "6") + + +class TheVerdict(unittest.TestCase): + """main() over a planted alerting directory and dashboard corpus.""" + + def verdict(self, rules, panel_exprs=PANELS): + root = pathlib.Path(tempfile.mkdtemp()) + alerts = root / "dashboards" / "base" / "alerting" + boards = root / "dashboards" / "base" / "platform" + alerts.mkdir(parents=True) + boards.mkdir(parents=True) + (alerts / "g.yaml").write_text(yaml.safe_dump(group("slo", *rules))) + (boards / "d.yaml").write_text( + yaml.safe_dump(dashboard("d", *panel_exprs))) + saved = (gate.ROOT, gate.ALERT_DIR, + gate.panels.ROOT, gate.panels.DASHBOARD_DIR) + gate.ROOT, gate.ALERT_DIR = root, alerts + gate.panels.ROOT, gate.panels.DASHBOARD_DIR = root, root / "dashboards" + try: + with contextlib.redirect_stdout(io.StringIO()) as out: + rc = gate.main() + finally: + (gate.ROOT, gate.ALERT_DIR, + gate.panels.ROOT, gate.panels.DASHBOARD_DIR) = saved + return rc, out.getvalue() + + def tiers(self): + """The four-tier ladder, each claim the one its expression spends + against a 30d objective.""" + return [ + rule("Fast", "burning fast (2% in 1h)", + burn_expr("0.01", "14.4", "1h", "5m")), + rule("Slow", "burning (5% in 6h)", + burn_expr("0.01", "6", "6h", "30m")), + rule("Ticket1d", "burning slowly (10% in 1d)", + burn_expr("0.01", "3", "1d", "2h")), + rule("Ticket3d", "burning slowest (10% in 3d)", + burn_expr("0.01", "1", "3d", "6h")), + ] + + def test_a_consistent_ladder_passes(self): + """The control. Without it every case below could be failing for a + reason the case did not plant.""" + rc, out = self.verdict(self.tiers()) + self.assertEqual(rc, 0, out) + + def test_the_defect_this_gate_was_written_for(self): + """Factor 1 over 3d against a 30d objective spends 10%, not 100%. The + expression is right and only the sentence is wrong, which is why every + rule-level validation passed it.""" + tiers = self.tiers() + tiers[3]["annotations"]["summary"] = "burning (100% over 3d)" + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("claims '100% over 3d'", out) + self.assertIn("spends 10% over 3d", out) + self.assertIn("1 x 3d / 30d", out) + + def test_a_factor_that_drifts_from_its_claim_is_reported(self): + tiers = self.tiers() + tiers[0]["data"][0]["model"]["expr"] = burn_expr( + "0.01", "7.2", "1h", "5m") + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("burn factor 7.2", out) + + def test_an_objective_window_that_moves_on_the_dashboard_is_reported(self): + """The term that lives in another file. Nothing in the alerting + directory changes here, and every claim in it becomes wrong.""" + rc, out = self.verdict( + self.tiers(), + panel_exprs=(f"sum(rate({NUMERATOR}[60d])) / " + f"sum(rate({DENOMINATOR}[60d]))",)) + self.assertEqual(rc, 1, out) + self.assertIn("60d objective", out) + self.assertEqual(out.count("claims"), 4) + + def test_a_claim_compared_to_nothing_is_reported(self): + """No panel measures what the rule burns against, so the figure agrees + with whatever it says. That is the state the defect survived in.""" + rc, out = self.verdict( + self.tiers(), panel_exprs=('sum(rate(other_metric{job="x"}[30d]))',)) + self.assertEqual(rc, 1, out) + self.assertIn("compared to nothing", out) + + def test_a_burn_rule_stating_no_figure_is_reported(self): + """Otherwise deleting the claim removes the rule from this gate's + corpus, and that is the edit most likely to accompany a wrong one.""" + tiers = self.tiers() + tiers[0]["annotations"]["summary"] = "budget burning fast" + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("states no budget figure", out) + + def test_a_summary_naming_another_window_is_reported(self): + tiers = self.tiers() + tiers[0]["annotations"]["summary"] = "burning fast (2% in 2h)" + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("measures over 1h", out) + + def test_two_burn_factors_in_one_rule_are_reported(self): + tiers = self.tiers() + tiers[0]["data"][0]["model"]["expr"] = ( + f"(sum(rate({NUMERATOR}[1h])) / 0.01 > bool 14.4)" + f" * (sum(rate({NUMERATOR}[5m])) / 0.01 > bool 6)") + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("2 different burn factors", out) + + def test_two_objective_windows_for_one_rule_are_reported(self): + """Which panel states the objective decides the figure. A tree stating + both has not decided, and picking one here would be this gate choosing + the answer it checks against.""" + rc, out = self.verdict(self.tiers(), panel_exprs=( + f"sum(rate({NUMERATOR}[30d]))", + f"sum(rate({DENOMINATOR}[7d]))", + )) + self.assertEqual(rc, 1, out) + self.assertIn("which of those", out.lower()) + + def test_no_burn_rule_at_all_cannot_run(self): + """Exit 2. A directory with no burn rule reports what a directory of + correct claims reports.""" + rc, out = self.verdict([rule("Rate", "error rate above 5%", + f"sum(rate({DENOMINATOR}[5m])) > 0.05")]) + self.assertEqual(rc, gate.gatelib.CANNOT_RUN, out) + self.assertIn("no burn-rate rule", out) + + +class TheShippedCatalog(unittest.TestCase): + """Over the tree, so a rule added with a wrong figure fails here.""" + + def test_the_catalog_passes(self): + with contextlib.redirect_stdout(io.StringIO()) as out: + self.assertEqual(gate.main(), 0, out.getvalue()) + + def test_the_ladder_carries_more_than_one_factor(self): + """A catalog whose tiers all shared a factor would pass a gate that + derived the figure from any one of them, so the corpus this runs over is + asserted rather than assumed.""" + seen = set() + for _p, _g, _r, expr in gate.burn_rules(gate.ALERT_DIR): + seen |= gate.factors(expr) + self.assertGreaterEqual(len(seen), 3, seen) + + def test_every_burn_rule_is_anchored_to_a_measured_objective(self): + """The figure is only as good as the thing it is compared against.""" + windows = gate.objective_windows() + for path, group_name, r, expr in gate.burn_rules(gate.ALERT_DIR): + with self.subTest(rule=f"{path.name}:{r.get('title')}"): + selectors = gate.long_window(expr)[1] + self.assertTrue( + [s for s in selectors if s in windows], + f"{group_name}/{r.get('title')} burns against " + f"{selectors} and no dashboard panel measures any of them") + + def test_the_objective_is_read_from_the_dashboards_not_declared_here(self): + """A constant agreeing with the standard today and compared to nothing + tomorrow is the defect this gate was written for, one level up.""" + source = (ROOT / "scripts" / "check-burn-rate-budgets.py").read_text() + self.assertNotIn("2592000", source) + self.assertNotIn('"30d"', source) + self.assertIn("panels.dashboards()", source) + + +if __name__ == "__main__": + unittest.main() From f2b56e5a3eb45c23c75ccba897963c26aa9473d3 Mon Sep 17 00:00:00 2001 From: stxkxs <139715017+stxkxs@users.noreply.github.com> Date: Fri, 4 Sep 2026 23:16:45 -0700 Subject: [PATCH 2/2] Read the objective from the panel a cluster receives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The burn-rate figure has no independent existence. It is not a number stored anywhere and corrected when wrong: it is the product of three terms, each read from a different place, and the summary is checked against what they produce. A reader looking for the value will not find one to edit. budget consumed = burn factor x long window / SLO window That is the point of the gate, and the reason a drift now fails: there is no longer a figure to drift FROM. A wrong summary disagrees with the expression that produces it; a moved SLO window disagrees with the panel that measures it; a panel that stops shipping anchors nothing and says so. ─── What the merge exposed ─── The routing gate that landed on main asks what a cluster RECEIVES: it renders dashboards/base and refuses an alerting object present in the directory and absent from the render. This gate's third term still asked what files EXIST — the SLO window came from a walk over dashboards/**.yaml on disk. Those are different questions, and the gap was reachable with one deleted line: $ sed -i '/platform\/agent-operator.yaml/d' dashboards/base/kustomization.yaml $ kustomize build dashboards/base | grep -c 'name: agent-operator$' -> 0 $ kustomize build dashboards/base | grep -c 'name: agent-operator-slo' -> 1 The rules ship and burn. The panel measuring the objective they burn against does not. Every figure in the file is then anchored to a document no cluster receives, and both gates report green. ─── scripts/check-burn-rate-budgets.py ─── `delivered_dashboards()` renders the kustomization and reads the GrafanaDashboard JSON out of the render. KUSTOMIZE_ROOT is ALERT_DIR.parent — the same derivation the routing gate uses, so the rules and the objective cannot be read out of different trees. A dashboard on disk and not in `resources` anchors nothing, and the existing "compared to nothing" verdict fires, reworded to say delivered. A render that fails and a kustomize that is not installed are both exit 2: a kustomization that does not build says nothing about which panels a cluster receives, and the dashboards on disk say nothing about it either. This drops the check-athena-panel-columns.py import, which existed to avoid a second walk of the dashboards directory. There is no walk of that directory now. ─── The agreement is asserted, not assumed ─── The corpus here is still the alerting directory on disk. This gate does not take the render reading itself, because the routing gate already refuses a rule group the same root does not render — so the corpus is the delivered one by way of that gate. test_the_rules_this_reads_are_rules_the_catalog_delivers asserts both halves of what that leans on: the two gates render the same root, and ALERTING_KINDS still contains GrafanaAlertRuleGroup. Narrow either and this gate returns to reading files no cluster has, with nothing to say so. ─── scripts/tests/reverify-gates.sh ─── A second planted defect for this gate: the panel dropped from the kustomization. The dashboard is unedited and still renders; it is simply not delivered. 47 `run` lines, floor 44. ─── scripts/tests/run.py ─── MAX_UNCOVERED_GATES stays 10, and it now reads 10 of 26 rather than 9. Dropping the import returned check-athena-panel-columns.py to uncovered, which is the honest reading: it never had tests, only a module body executed by somebody else's import. The comment no longer names it as import-covered. --- CLAUDE.md | 8 ++- scripts/check-burn-rate-budgets.py | 85 ++++++++++++++++++++----- scripts/tests/reverify-gates.sh | 20 +++++- scripts/tests/run.py | 19 +++--- scripts/tests/test_burn_rate_budgets.py | 84 +++++++++++++++++++++--- 5 files changed, 177 insertions(+), 39 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d7088e1..3ffd0ca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -172,9 +172,11 @@ aggregate, so a clean `task validate` is necessary but not sufficient: does not spend while every rule-level validation passes. The figure is derived rather than listed: burn factor times long window over the SLO window, with the factor and window read from the rule's expression and the SLO window from the - dashboard panel measuring the same selector. A rule whose selectors no panel - measures is a finding, because a figure compared to nothing is how the last - wrong one survived + dashboard panel measuring the same selector — out of the render, so it is a + panel a cluster receives rather than a file on disk. A rule whose selectors no + delivered panel measures is a finding, because a figure compared to nothing is + how the last wrong one survived. The figure has no independent existence: + there is no constant to correct and none in the summary that anything trusts - **Image vulnerabilities** — `scripts/check-image-vulnerabilities.py` (CI job `image-vulnerabilities`). The one with a target of its own, `task validate:image-vulnerabilities`, kept out of the aggregate because it diff --git a/scripts/check-burn-rate-budgets.py b/scripts/check-burn-rate-budgets.py index e8d99d6..b8ad8bc 100755 --- a/scripts/check-burn-rate-budgets.py +++ b/scripts/check-burn-rate-budgets.py @@ -37,13 +37,20 @@ * `f` and `w` from the rule's own expression — the `> bool f` threshold and the longest range selector in it; * `W` from the dashboard panel measuring the same metric selector over its - longest explicit range. That is the panel that displays the objective this + longest explicit range — read out of the RENDER, not off disk, so it is the + panel a cluster receives. That is the panel displaying the objective this rule burns against, in a different file, edited by different work. +The figure therefore has no independent existence. There is no number here to +correct, and none in the summary that anything trusts: the sentence is checked +against the arithmetic that produces it, every term of which is read from a +place that would have to be edited in step for a wrong figure to survive. + So there is no constant here to agree with the standard today and be compared to nothing tomorrow. A figure that drifts fails against the expression that -produces it, and an SLO window that moves fails against the panel that measures -it. +produces it; an SLO window that moves fails against the panel that measures it; +and a panel that stops being delivered stops anchoring anything, which is a +finding rather than a silent fallback to whatever the summary says. Every rule whose expression is a burn-rate expression must carry the claim. A burn-rate title that does not say how much budget is at stake tells the reader @@ -72,11 +79,15 @@ from __future__ import annotations import importlib.util +import json import pathlib import re +import subprocess import sys from fractions import Fraction +import yaml + _here = pathlib.Path(__file__).resolve().parent _gl = _here / "gatelib.py" @@ -86,18 +97,14 @@ sys.modules["gatelib"] = gatelib _gs.loader.exec_module(gatelib) -# The dashboard walk lives in the gate that already owns it. Copying it here -# would give this gate a second walk to keep in step with the first, and a -# dashboard silently dropped from either corpus reports the same as a clean run. -_ap = _here / "check-athena-panel-columns.py" -_as = importlib.util.spec_from_file_location("check_athena_panel_columns", _ap) -assert _as and _as.loader, f"{_ap} is not loadable as a module" -panels = importlib.util.module_from_spec(_as) -_as.loader.exec_module(panels) - ROOT = _here.parent ALERT_DIR = ROOT / "dashboards" / "base" / "alerting" +# The kustomization delivering both the rules and the panels they burn against. +# Derived from the alerting directory rather than named again, so the rules and +# the objective cannot come to be read out of different trees. +KUSTOMIZE_ROOT = ALERT_DIR.parent RULE_GROUP = "GrafanaAlertRuleGroup" +DASHBOARD = "GrafanaDashboard" UNIT_SECONDS = {"s": 1, "m": 60, "h": 3600, "d": 86400, "w": 604800} @@ -119,6 +126,14 @@ r"(?P\d+(?:\.(?P\d+))?)%\s+(?:in|over)\s+(?P\d+[smhdw])") +def under_root(path: pathlib.Path) -> str: + """`path` relative to the repository when it is inside it, else as given.""" + try: + return str(path.relative_to(ROOT)) + except ValueError: + return str(path) + + def seconds(duration: str) -> int: """A PromQL duration in seconds. The grammar here is a single unit, which is what every range in this catalog uses; a compound `1h30m` would not match @@ -196,15 +211,54 @@ def factors(expr: str) -> set[Fraction]: return {Fraction(m.group("factor")) for m in FACTOR.finditer(expr)} +def delivered_dashboards(root: pathlib.Path) -> list[dict]: + """The dashboard JSON of every GrafanaDashboard `root` renders. + + Read from the render rather than off disk, because `resources` is an + explicit list and a panel can stop being delivered without moving, being + edited, or failing to render. A dashboard the catalog no longer ships is + not a second source: the rules would keep burning against an objective + nobody can see, and this gate would keep certifying their figures against + a document no cluster receives. + """ + gatelib.require("kustomize") + proc = subprocess.run(["kustomize", "build", str(root)], + capture_output=True, text=True) + if proc.returncode != 0: + print(f"Cannot run: kustomize build {under_root(root)} failed, so " + f"which panels this catalog delivers could not be read. The " + f"dashboards on disk say nothing about that on their own.") + print(proc.stderr.strip()[:500]) + sys.exit(gatelib.CANNOT_RUN) + out: list[dict] = [] + for doc in yaml.safe_load_all(proc.stdout): + if not isinstance(doc, dict) or doc.get("kind") != DASHBOARD: + continue + raw = (doc.get("spec") or {}).get("json") + if not isinstance(raw, str): + continue + try: + out.append(json.loads(raw)) + except json.JSONDecodeError as err: + # Refusing to skip a dashboard that will not parse. A gate that + # silently drops its subject reports success over nothing. + name = (doc.get("metadata") or {}).get("name", "") + print(f"Cannot run: {DASHBOARD}/{name} renders spec.json that is not " + f"valid JSON, so the objective it measures could not be read: " + f"{err}") + sys.exit(gatelib.CANNOT_RUN) + return out + + def objective_windows() -> dict[str, int]: - """selector -> the longest explicit range a dashboard panel applies to it. + """selector -> the longest explicit range a DELIVERED panel applies to it. The SLO window, read from the panel that displays the objective. A burn window is by construction shorter than the window it burns against, so the longest range a panel measures this selector over is the objective's. """ out: dict[str, int] = {} - for _path, dash in panels.dashboards(): + for dash in delivered_dashboards(KUSTOMIZE_ROOT): for expr in panel_exprs(dash): for m in SELECTOR.finditer(expr): sel = re.sub(r"\s+", "", m.group("sel")) @@ -301,7 +355,8 @@ def main() -> int: objective = {windows[sel] for sel in selectors if sel in windows} if not objective: failures.append( - f"{where} claims {claim.group(0)!r} and no dashboard panel measures " + f"{where} claims {claim.group(0)!r} and no dashboard delivered " + f"by {under_root(KUSTOMIZE_ROOT)} measures " f"{' or '.join(sorted(selectors))} over an explicit range. The " f"figure is compared to nothing, which is how the last wrong one " f"survived.") diff --git a/scripts/tests/reverify-gates.sh b/scripts/tests/reverify-gates.sh index 9fad8ca..42b708c 100755 --- a/scripts/tests/reverify-gates.sh +++ b/scripts/tests/reverify-gates.sh @@ -419,6 +419,24 @@ run nonzero "burn-rate-budgets: a summary claiming a budget its expression does ./scripts/check-burn-rate-budgets.py res $F +# The other term, and the one that lives outside the alerting directory. The +# dashboard is unedited and still renders; it is simply no longer in the +# kustomization's resources. The rules ship and burn, the panel measuring what +# they burn against does not, and every figure is then anchored to a document no +# cluster receives. +F=dashboards/base/kustomization.yaml; mut $F +python3 - "$F" <<'PYZ' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace(" - platform/agent-operator.yaml\n", "", 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" dropped the panel measuring the objective from the kustomization") +PYZ +run nonzero "burn-rate-budgets: the panel measuring the objective stops shipping" \ + ./scripts/check-burn-rate-budgets.py +res $F + F=addons/bootstrap/cert-manager/values-hub.yaml; mut $F; rm -f $F run nonzero "env-coverage: deleted hub delta" ./scripts/check-env-coverage.py res $F @@ -531,7 +549,7 @@ echo "RESULT pass=$pass fail=$fail" # The harness owes the same assertion it demands of the gates: with every `run` # line deleted it would report pass=0 fail=0 and exit 0, which is a green run # over nothing checked. -MIN_CHECKS=43 +MIN_CHECKS=44 total=$((pass + fail)) if [ "$total" -lt "$MIN_CHECKS" ]; then echo "FAIL ran $total check(s), under the floor of $MIN_CHECKS — this harness" diff --git a/scripts/tests/run.py b/scripts/tests/run.py index 18498cf..959549a 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -148,21 +148,20 @@ # Ratchets downward only. Adding a gate without tests fails here rather than # diluting the combined figure by a percentage point nobody notices. # -# Some of the files this counts as covered are covered by IMPORT, not by tests. +# Two of the files this counts as covered are covered by IMPORT, not by tests. # check-image-vulnerabilities.py loads check-image-pins.py by path so the two # cannot disagree about what the fleet is, and that loads render-addons.py in -# turn; check-burn-rate-budgets.py loads check-athena-panel-columns.py for the -# dashboard walk, for the same reason. A test module importing any of those -# executes the module bodies of the ones underneath. Coverage cannot tell that -# from a test, so they read as non-zero while carrying no assertions of their -# own. The ceiling is lowered anyway, because a ratchet that declines to move is -# one nobody can regress against — but it is the weaker half of this file's -# claim, and the per-gate floors and the controls in scripts/tests/controls.py -# are where the real one lives. +# turn; a test module importing the first executes the module bodies of all +# three. Coverage cannot tell that from a test, so both read as non-zero while +# carrying no assertions of their own. The ceiling is lowered anyway, because a +# ratchet that declines to move is one nobody can regress against — but it is +# the weaker half of this file's claim, and the per-gate floors and the controls +# in scripts/tests/controls.py are where the real one lives. # # So this number moves by what a change actually covered, not by what the run # reports. A gate arriving with its own tests lowers it; a gate that starts -# reading as covered because something new imports it does not. +# reading as covered because something new imports it does not, and an import +# removed later would give that coverage back with nothing lost. MAX_UNCOVERED_GATES = 10 PER_GATE_FLOORS = { diff --git a/scripts/tests/test_burn_rate_budgets.py b/scripts/tests/test_burn_rate_budgets.py index 3be0925..55b770f 100644 --- a/scripts/tests/test_burn_rate_budgets.py +++ b/scripts/tests/test_burn_rate_budgets.py @@ -16,6 +16,7 @@ import io import json import pathlib +import shutil import tempfile import unittest from fractions import Fraction @@ -27,6 +28,12 @@ ROOT = pathlib.Path(__file__).resolve().parent.parent.parent +# The objective is read out of the render, so the planted fixtures are rendered +# too rather than compared against a stub. The job running these installs +# kustomize; a checkout without it skips rather than aborting the runner on the +# gate's exit-2 refusal. +HAS_KUSTOMIZE = shutil.which("kustomize") is not None + def burn_expr(budget, factor, long_window, short_window, guard=None, numerator=None, denominator=None): @@ -143,28 +150,39 @@ def test_a_factor_renders_as_the_decimal_in_the_expression(self): self.assertEqual(gate.decimal(Fraction(6)), "6") +@unittest.skipUnless(HAS_KUSTOMIZE, "kustomize is not on PATH") class TheVerdict(unittest.TestCase): """main() over a planted alerting directory and dashboard corpus.""" - def verdict(self, rules, panel_exprs=PANELS): + def verdict(self, rules, panel_exprs=PANELS, ship_dashboard=True): + """main() over a planted tree, rendered the way the real one is. + + `ship_dashboard=False` writes the dashboard and leaves it out of the + kustomization's `resources`, which is what a panel that stops being + delivered looks like from disk. + """ root = pathlib.Path(tempfile.mkdtemp()) - alerts = root / "dashboards" / "base" / "alerting" - boards = root / "dashboards" / "base" / "platform" + base = root / "dashboards" / "base" + alerts = base / "alerting" + boards = base / "platform" alerts.mkdir(parents=True) boards.mkdir(parents=True) (alerts / "g.yaml").write_text(yaml.safe_dump(group("slo", *rules))) (boards / "d.yaml").write_text( yaml.safe_dump(dashboard("d", *panel_exprs))) - saved = (gate.ROOT, gate.ALERT_DIR, - gate.panels.ROOT, gate.panels.DASHBOARD_DIR) - gate.ROOT, gate.ALERT_DIR = root, alerts - gate.panels.ROOT, gate.panels.DASHBOARD_DIR = root, root / "dashboards" + resources = [" - alerting/g.yaml"] + if ship_dashboard: + resources.append(" - platform/d.yaml") + (base / "kustomization.yaml").write_text( + "apiVersion: kustomize.config.k8s.io/v1beta1\n" + "kind: Kustomization\nresources:\n" + "\n".join(resources) + "\n") + saved = (gate.ROOT, gate.ALERT_DIR, gate.KUSTOMIZE_ROOT) + gate.ROOT, gate.ALERT_DIR, gate.KUSTOMIZE_ROOT = root, alerts, base try: with contextlib.redirect_stdout(io.StringIO()) as out: rc = gate.main() finally: - (gate.ROOT, gate.ALERT_DIR, - gate.panels.ROOT, gate.panels.DASHBOARD_DIR) = saved + gate.ROOT, gate.ALERT_DIR, gate.KUSTOMIZE_ROOT = saved return rc, out.getvalue() def tiers(self): @@ -262,6 +280,32 @@ def test_two_objective_windows_for_one_rule_are_reported(self): self.assertEqual(rc, 1, out) self.assertIn("which of those", out.lower()) + def test_a_panel_that_stops_being_delivered_is_reported(self): + """The dashboard is on disk, unedited, and renders fine — it is simply + no longer in the kustomization's `resources`. The rules still ship and + still burn; the panel measuring what they burn against does not. A + figure anchored to a document no cluster receives is anchored to + nothing.""" + rc, out = self.verdict(self.tiers(), ship_dashboard=False) + self.assertEqual(rc, 1, out) + self.assertIn("no dashboard delivered by", out) + + def test_a_render_that_fails_cannot_run(self): + """Exit 2. A kustomization that does not build says nothing about which + panels a cluster receives, and the dashboards on disk say nothing about + it either.""" + root = pathlib.Path(tempfile.mkdtemp()) + base = root / "dashboards" / "base" + base.mkdir(parents=True) + (base / "kustomization.yaml").write_text( + "apiVersion: kustomize.config.k8s.io/v1beta1\n" + "kind: Kustomization\nresources:\n - platform/gone.yaml\n") + with self.assertRaises(SystemExit) as caught, \ + contextlib.redirect_stdout(io.StringIO()) as out: + gate.delivered_dashboards(base) + self.assertEqual(caught.exception.code, gate.gatelib.CANNOT_RUN) + self.assertIn("could not be read", out.getvalue()) + def test_no_burn_rule_at_all_cannot_run(self): """Exit 2. A directory with no burn rule reports what a directory of correct claims reports.""" @@ -298,13 +342,33 @@ def test_every_burn_rule_is_anchored_to_a_measured_objective(self): f"{group_name}/{r.get('title')} burns against " f"{selectors} and no dashboard panel measures any of them") + def test_the_rules_this_reads_are_rules_the_catalog_delivers(self): + """This gate reads the alerting directory; a cluster receives what the + kustomization renders. It does not take the second reading itself, + because check-alert-severity-routes.py already refuses a rule group on + disk that the same root does not render — so the corpus here is the + delivered one by way of that gate. + + Asserted rather than assumed. The two gates have to agree on the root + and on the kind, and a narrowing on either side would otherwise leave + this one reading files no cluster has, silently. + """ + routes = load("check-alert-severity-routes") + self.assertEqual(routes.KUSTOMIZE_ROOT, gate.KUSTOMIZE_ROOT, + "the two alerting gates render different roots, so one " + "of them is asserting over a tree the other does not") + self.assertIn(gate.RULE_GROUP, routes.ALERTING_KINDS, + f"{routes.__name__} no longer refuses an unrendered " + f"{gate.RULE_GROUP}, so this gate's corpus is files on " + f"disk rather than what a cluster receives") + def test_the_objective_is_read_from_the_dashboards_not_declared_here(self): """A constant agreeing with the standard today and compared to nothing tomorrow is the defect this gate was written for, one level up.""" source = (ROOT / "scripts" / "check-burn-rate-budgets.py").read_text() self.assertNotIn("2592000", source) self.assertNotIn('"30d"', source) - self.assertIn("panels.dashboards()", source) + self.assertIn('subprocess.run(["kustomize", "build"', source) if __name__ == "__main__":