From 82022427cd5aeb693adf33b03f818bec7d152b32 Mon Sep 17 00:00:00 2001 From: Pigbibi <20649888+Pigbibi@users.noreply.github.com> Date: Fri, 6 Nov 2026 13:00:57 +0800 Subject: [PATCH] fix: Normalize generated container names in warm recovery Co-Authored-By: Codex --- docs/ibkr-runtime-rollout.md | 7 ++- scripts/cloud_run_warm_recovery.py | 19 +++++++- tests/test_cloud_run_warm_recovery.py | 70 ++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/docs/ibkr-runtime-rollout.md b/docs/ibkr-runtime-rollout.md index be5af3fa..e7454612 100644 --- a/docs/ibkr-runtime-rollout.md +++ b/docs/ibkr-runtime-rollout.md @@ -621,7 +621,12 @@ an earlier operation. Any later attempt needs a fresh owner card and readback. The baseline must use an explicit warm-100 desired traffic row, with no tags or latest-revision indirection. The existing template must already carry the approved application source label, a single application container, an untagged -image repository and minimum zero. The template spec must equal the verified warm spec. Configuration and Secret +image repository and minimum zero. The template spec must equal the verified warm +spec, except for a revision-only container name generated for a single unnamed +container. The same comparison applies to replacement readbacks. Explicit +container names and every other field must match; frozen revision fingerprints +still include the generated name. See the [Cloud Run container name reference](https://docs.cloud.google.com/run/docs/reference/yaml/v1). +Configuration and Secret references are copied without synchronization; runtime target continuity remains checked by the common verifier. No Secret value, IAM policy, Scheduler, execution switch or risk setting is changed. diff --git a/scripts/cloud_run_warm_recovery.py b/scripts/cloud_run_warm_recovery.py index fbb84fe5..25d90977 100644 --- a/scripts/cloud_run_warm_recovery.py +++ b/scripts/cloud_run_warm_recovery.py @@ -50,6 +50,21 @@ def fingerprint(value: dict) -> dict: 'imageDigest': value.get('status', {}).get('imageDigest')} +def template_matches_revision(template: dict, revision: dict) -> bool: + if template == revision: + return True + containers = template.get('containers', []) + observed = revision.get('containers', []) + if (len(containers) != 1 or len(observed) != 1 or 'name' in containers[0] + or not isinstance(observed[0].get('name'), str) or not observed[0]['name']): + return False + # Cloud Run may generate a revision-only name for an unnamed container. + # Normalize only this comparison; frozen snapshots remain byte-for-byte bound. + normalized = copy.deepcopy(revision) + normalized['containers'][0].pop('name') + return template == normalized + + def healthy(value: dict, commit: str) -> None: # The common verifier prints provider conditions; keep those private here. with contextlib.redirect_stderr(io.StringIO()), contextlib.redirect_stdout(io.StringIO()): @@ -177,7 +192,7 @@ def check_replacement(self, timeout=30): require(value['metadata'].get('name') == self.card['replacement']) require(str(value['metadata'].get('annotations', {}).get(MINIMUM, '0')) == '0') template = self.expected['spec']['template'] - require(value['spec'] == template['spec']) + require(template_matches_revision(template['spec'], value['spec'])) for key in ('labels', 'annotations'): for name, item in template['metadata'].get(key, {}).items(): require(value['metadata'].get(key, {}).get(name) == item) @@ -299,7 +314,7 @@ def execute(self): current = self.stable([(self.warm, 100)]) self.valid() spec = copy.deepcopy(current['spec']) - require(spec['template']['spec'] == self.card['warm']['spec']) + require(template_matches_revision(spec['template']['spec'], self.card['warm']['spec'])) template = spec['template'] template['metadata']['name'] = self.card['replacement'] template['metadata'].setdefault('annotations', {})[MINIMUM] = '0' diff --git a/tests/test_cloud_run_warm_recovery.py b/tests/test_cloud_run_warm_recovery.py index d023bf64..cd3dfcd4 100644 --- a/tests/test_cloud_run_warm_recovery.py +++ b/tests/test_cloud_run_warm_recovery.py @@ -7,7 +7,7 @@ import pytest from scripts.cloud_run_candidate_warmup import MINIMUM -from scripts.cloud_run_warm_recovery import API, Recovery, RecoveryError, healthy +from scripts.cloud_run_warm_recovery import API, Recovery, RecoveryError, healthy, template_matches_revision DIGEST = 'sha256:' + 'a' * 64 COMMIT = 'b' * 40 @@ -265,6 +265,74 @@ def test_template_must_preserve_the_verified_warm_configuration(tmp_path): assert cloud.writes == [] +def test_warm_only_generated_container_name_preserves_template(tmp_path): + approval = card() + approval['warm']['spec']['containers'][0]['name'] = 'generated-warm' + obj, cloud = run(tmp_path, approval=approval) + obj.execute() + assert len(cloud.writes) == 3 + assert 'name' not in cloud.writes[0]['spec']['template']['spec']['containers'][0] + assert obj.card['warm']['spec']['containers'][0]['name'] == 'generated-warm' + assert json.loads(obj.attempt.read_text())['status'] == 'SUCCEEDED' + + +def test_replacement_only_generated_container_name_is_still_frozen(tmp_path): + obj, cloud = run(tmp_path) + original = cloud.__call__ + def api(kind, name='', body=None, timeout=30): + value = original(kind, name, body, timeout) + if kind == 'revision' and name == obj.card['replacement']: + value['spec']['containers'][0]['name'] = 'generated-replacement' + return value + obj.api = api + obj.execute() + assert len(cloud.writes) == 3 + assert obj.replacement['spec']['containers'][0]['name'] == 'generated-replacement' + assert json.loads(obj.attempt.read_text())['status'] == 'SUCCEEDED' + + +@pytest.mark.parametrize('field,value', [ + ('image', 'registry.invalid/unit/other'), ('env', []), ('resources', {'limits': {'cpu': '2'}}), + ('startupProbe', {}), ('name', ''), ('name', None)]) +def test_generated_name_never_hides_other_container_drift(tmp_path, field, value): + approval = card() + approval['warm']['spec']['containers'][0]['name'] = 'generated-warm' + approval['warm']['spec']['containers'][0][field] = value + obj, cloud = run(tmp_path, approval=approval) + with pytest.raises(RecoveryError): obj.execute() + assert cloud.writes == [] + + +def test_configured_names_and_multiple_containers_are_not_normalized(): + template = card()['warm']['spec'] + observed = copy.deepcopy(template) + observed['containers'][0]['name'] = 'generated' + original = copy.deepcopy(observed) + assert template_matches_revision(template, observed) + assert observed == original + template['containers'][0]['name'] = 'configured' + assert not template_matches_revision(template, observed) + assert not template_matches_revision(observed, {'containers': []}) + template['containers'].append(copy.deepcopy(template['containers'][0])) + observed['containers'].append(copy.deepcopy(observed['containers'][0])) + assert not template_matches_revision(template, observed) + + +def test_generated_name_change_after_freezing_triggers_one_compensation(tmp_path): + obj, cloud = run(tmp_path) + original = cloud.__call__ + def api(kind, name='', body=None, timeout=30): + value = original(kind, name, body, timeout) + if kind == 'revision' and name == obj.card['replacement']: + value['spec']['containers'][0]['name'] = 'first' if len(cloud.writes) == 1 else 'changed' + return value + obj.api = api + with pytest.raises(RecoveryError): obj.execute() + assert len(cloud.writes) == 3 + assert cloud.service['spec']['traffic'] == [{'revisionName': obj.warm, 'percent': 100}] + assert json.loads(obj.attempt.read_text())['status'] == 'FAILED_WARM_RESTORED' + + def test_control_plane_convergence_never_accepts_a_late_read(tmp_path): obj, cloud = run(tmp_path) original = cloud.__call__