Skip to content

Commit 5cfa8af

Browse files
fix(ci): accept tagged no-traffic Cloud Run candidate with omitted percent (#10267)
## Summary Candidate-mode `verify_backend_release_vector.py` rejected a valid no-traffic Cloud Run candidate because it read an omitted/`null` traffic `percent` (a tagged, zero-allocation candidate) as positive pre-promotion traffic. This blocked the automatic dev deploy at the "Accept no-traffic Cloud Run candidate" gate for all four backend services. One-line-classification fix plus the regression test that reproduces the exact failed-development shape. Failure-Class: FC-nonrecoverable-promotion ## Failure mechanism Cloud Run serializes a tagged, no-allocation candidate revision with `percent: null` in `status.traffic` — the revision exists and is `Ready`, is reachable via its tag for acceptance probing, but carries 0% of default-domain traffic until promotion (the prior serving revision keeps 100%). This is the intended pre-promotion state the candidate gate is supposed to *accept*. The verifier's candidate check was: ```python if not require_serving_traffic and any(_mapping(entry).get('percent') != 0 for entry in expected_traffic): errors.append('... expected revision carries traffic before promotion') ``` For an omitted/null percent, `_mapping(entry).get('percent')` is `None`, and `None != 0` is `True`, so a Ready zero-allocation candidate was falsely flagged as "carries traffic before promotion". Sibling scripts already handle this correctly (`cloud_run_traffic_snapshot.py` requires `isinstance(percent, int)`; `deploy_status_report.py`/`repair_cloud_run_traffic.py` use `int(... or 0)`), so the verifier was the lone outlier — the fix is localized to it. Exact failing run (development, SHA `2904ea7`): https://github.com/BasedHardware/omi/actions/runs/29891331283 — each of the four services printed `percent: null` for the candidate entry and `percent: 100` for the prior serving revision, then failed with "expected revision carries traffic before promotion". ## Root cause confirmed Cloud Run allocation semantics, distinguished before patching: - Explicit numeric `percent` → that exact allocation. - Omitted/`null` `percent` on a `revisionName`-pinned target → 0% (tagged-only, no default-domain traffic). This is the candidate shape. - Omitted `percent` on a `latestRevision: true` target → receives the remainder (may be > 0). These never reach this check: `expected_traffic` is filtered by `revisionName == expected_revision`, and `latestRevision` targets carry no literal `revisionName`, so they cannot match the candidate name. So treating omitted percent as 0% **only for revision-pinned candidate entries** is semantically correct — not a blanket `int(... or 0)` coercion. ## Change `backend/scripts/verify_backend_release_vector.py` - New `_effective_candidate_traffic_percent(percent)`: `None` → `0.0`; a real non-negative number → its allocation; bool/string/negative → `None` (ambiguous, must not be trusted as zero). - Candidate-mode loop now: reject only a positive allocation ("carries traffic before promotion"); reject ambiguous shapes with a distinct "candidate traffic allocation is ambiguous" error; accept `None`/`0`. `backend/tests/unit/test_verify_backend_release_vector.py` - `test_candidate_evaluation_accepts_a_tagged_candidate_with_omitted_percent`: RED→GREEN regression reproducing run 29891331283 (prior 100%, candidate `percent: None`, Ready=True) → candidate passes; serving mode still rejects it until promoted to 100%. - `test_candidate_evaluation_rejects_positive_or_ambiguous_candidate_traffic`: positive (5, 100) → "carries traffic before promotion"; ambiguous (`"0"`, `True`, `-1`) → "ambiguous". Proves the safety guard stays mutation-sensitive. ## TDD evidence - RED (before fix): the omitted-percent acceptance test failed with the exact production error; the ambiguous cases mis-reported as "carries traffic". - GREEN (after fix): `tests/unit/test_verify_backend_release_vector.py` → 48 passed; production boundary test → 3 passed; `tests/unit/test_workflow_contracts.py` → 21 passed. - Re-ran the verifier against a sanitized fixture equivalent to the failed dev shape → candidate PASS; flipped to serving mode → still fails until 100%. ## Safety invariants (unchanged) - Serving/post-promotion check is untouched: it still requires the expected revision's traffic entry at exactly `percent == 100` and rejects everything else (`None`, mixed, ambiguous). The fix touches only the `not require_serving_traffic` branch. - Candidate readiness, latest-created-revision, immutable image, timeout, and `OMI_ENV_STAGE` checks are unchanged. - No new dependency, no workflow redesign, no refactor. ## Rollback impact Reverting restores the false-positive candidate rejection (auto-dev stays blocked on the same gate) but does not weaken any promotion/serving gate — a revert cannot let traffic move or weaken the final 100% vector check. ## Why production behavior is not weakened Production serving verification always runs in serving mode (`require_serving_traffic=True`), whose check is byte-for-byte unchanged. Candidate mode is used only for the pre-promotion acceptance gate, which this fix makes correctly recognize the intended no-traffic candidate. The `FC-nonrecoverable-promotion` contract — preserve pre-promotion traffic state until the serving vector is proven by accepting an immutable no-traffic candidate — is what this fix restores. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/BasedHardware/omi/pull/10267?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
2 parents 2904ea7 + 1da5151 commit 5cfa8af

2 files changed

Lines changed: 83 additions & 2 deletions

File tree

backend/scripts/verify_backend_release_vector.py

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,13 @@ def evaluate_cloud_run_service(
271271
errors.append(f'cloud_run/{service}: template image is not {expected_image}')
272272
if require_serving_traffic and (not expected_traffic or _mapping(expected_traffic[0]).get('percent') != 100):
273273
errors.append(f'cloud_run/{service}: expected revision does not receive 100% traffic')
274-
if not require_serving_traffic and any(_mapping(entry).get('percent') != 0 for entry in expected_traffic):
275-
errors.append(f'cloud_run/{service}: expected revision carries traffic before promotion')
274+
if not require_serving_traffic:
275+
for entry in expected_traffic:
276+
allocation = _effective_candidate_traffic_percent(_mapping(entry).get('percent'))
277+
if allocation is None:
278+
errors.append(f'cloud_run/{service}: candidate traffic allocation is ambiguous')
279+
elif allocation > 0:
280+
errors.append(f'cloud_run/{service}: expected revision carries traffic before promotion')
276281
timeout = template_spec.get('timeoutSeconds')
277282
if not isinstance(timeout, int) or timeout < MIN_CLOUD_RUN_TIMEOUT_SECONDS:
278283
errors.append(f'cloud_run/{service}: timeoutSeconds must be at least {MIN_CLOUD_RUN_TIMEOUT_SECONDS}')
@@ -432,6 +437,23 @@ def _container_env(containers: Sequence[Any]) -> dict[str, str]:
432437
}
433438

434439

440+
def _effective_candidate_traffic_percent(percent: Any) -> float | None:
441+
"""Effective default-domain allocation for a candidate traffic target.
442+
443+
Cloud Run omits ``percent`` (serialized as ``null``) on a tagged, no-allocation
444+
candidate revision, which carries 0% of serving traffic until promotion. A real
445+
non-negative numeric percent is its allocation; any other shape (bool, string,
446+
negative) is ambiguous and must not be trusted as zero, so it returns ``None``
447+
for the caller to reject. Only ``revisionName``-pinned targets reach this path;
448+
``latestRevision`` remainder targets never match the candidate name.
449+
"""
450+
if percent is None:
451+
return 0.0
452+
if isinstance(percent, bool) or not isinstance(percent, (int, float)) or percent < 0:
453+
return None
454+
return float(percent)
455+
456+
435457
def main() -> int:
436458
parser = argparse.ArgumentParser(description=__doc__)
437459
parser.add_argument('--commit-sha', required=True)

backend/tests/unit/test_verify_backend_release_vector.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,65 @@ def test_candidate_evaluation_accepts_ready_revision_before_traffic_promotion()
678678
assert 'cloud_run/backend: expected revision does not receive 100% traffic' in serving_errors
679679

680680

681+
def test_candidate_evaluation_accepts_a_tagged_candidate_with_omitted_percent() -> None:
682+
"""Regression for run 29891331283: Cloud Run serializes a tagged, no-allocation
683+
candidate revision with ``percent: null`` (effective 0%). The verifier must read
684+
an omitted/null percent as zero, not as pre-promotion traffic. Mirrors the exact
685+
failed-development shape: prior serving revision at 100% plus the Ready candidate
686+
with an omitted percent.
687+
"""
688+
expectation = _expectation()
689+
documents = _documents(expectation)
690+
for service, revision in expectation.revisions.items():
691+
documents[f'cloud_run/{service}']['status'].update(
692+
{
693+
'latestCreatedRevisionName': revision,
694+
'latestReadyRevisionName': f'{service}-old',
695+
'traffic': [
696+
{'revisionName': f'{service}-old', 'percent': 100},
697+
{'revisionName': revision, 'tag': 'candidate', 'percent': None},
698+
],
699+
}
700+
)
701+
documents[f'cloud_run_revision/{service}'] = _cloud_run_revision_document(
702+
image=expectation.image,
703+
ready='True',
704+
reason='Retired',
705+
)
706+
707+
errors = verifier.evaluate(expectation, documents, require_serving_traffic=False)
708+
709+
assert errors == []
710+
711+
# Strict serving verification still rejects the no-traffic candidate until it is promoted to 100%.
712+
serving_errors = verifier.evaluate(expectation, documents)
713+
assert 'cloud_run/backend: expected revision does not receive 100% traffic' in serving_errors
714+
715+
716+
@pytest.mark.parametrize(
717+
('percent', 'expected_error'),
718+
(
719+
(5, 'cloud_run/backend: expected revision carries traffic before promotion'),
720+
(100, 'cloud_run/backend: expected revision carries traffic before promotion'),
721+
('0', 'cloud_run/backend: candidate traffic allocation is ambiguous'),
722+
(True, 'cloud_run/backend: candidate traffic allocation is ambiguous'),
723+
(-1, 'cloud_run/backend: candidate traffic allocation is ambiguous'),
724+
),
725+
)
726+
def test_candidate_evaluation_rejects_positive_or_ambiguous_candidate_traffic(percent, expected_error: str) -> None:
727+
expectation = _expectation()
728+
documents = _documents(expectation)
729+
for service in expectation.revisions:
730+
documents[f'cloud_run/{service}']['status']['traffic'] = [
731+
{'revisionName': f'{service}-old', 'percent': 100},
732+
{'revisionName': expectation.revisions[service], 'percent': percent},
733+
]
734+
735+
errors = verifier.evaluate(expectation, documents, require_serving_traffic=False)
736+
737+
assert expected_error in errors
738+
739+
681740
@pytest.mark.parametrize(
682741
('mutate', 'expected_error'),
683742
(

0 commit comments

Comments
 (0)