Retry GCP metadata writes on an operation-level fingerprint conflict - #349
Merged
Merged
Conversation
GCP keeps labels for networks, routers, firewalls and key pairs in the project-wide common instance metadata, and every write re-uploads that document under an optimistic fingerprint. gcp_metadata_save_op retries when the fingerprint is stale, but its predicate only recognised the conflict as an HttpError. A concurrent writer produces it differently: the upload is accepted and the resulting operation completes with CONDITION_NOT_MET, which wait_for_operation raised as a plain Exception the retry never matched. Under parallel use every collision therefore failed on the first attempt - the recurring test_crud_* failures in the GCP live suite, which runs five workers against one project. Raise GCPOperationError from wait_for_operation, a ProviderInternalException carrying the operation's error payload and codes, and have the save predicate retry on CONDITION_NOT_MET. The save already re-fetches the metadata (and a fresh fingerprint) on each attempt, so the existing retry is correct once it fires. The exception message is unchanged; callers that catch CloudBridgeBaseException see the typed error instead of the middleware's generic wrapper.
nuwang
had a problem deploying
to
cloud-integration
September 17, 2026 11:07 — with 
GitHub Actions
Failure
GitHub Actions
Failure
nuwang
had a problem deploying
to
cloud-integration
September 17, 2026 13:15 — with
GitHub Actions
Failure
GitHub Actions
Failure
nuwang
had a problem deploying
to
cloud-integration
September 17, 2026 15:37 — with
GitHub Actions
Failure
GitHub Actions
Failure
The CB_TEST_TRACE files were emitted by a command placed after pytest in tox's commands list, and tox stops at the first failing command - so the trace was printed only for passing runs, and a failing run, the one whose trace is worth reading, lost it. Emit it from commands_post, which tox runs regardless of the outcome; the environment still reports the failure.
Retrying on CONDITION_NOT_MET turned out not to be enough. Instrumenting the live suite showed why: with five workers labelling resources in one project, GCP accepts several setCommonInstanceMetadata requests against the same fingerprint while an earlier one is still pending (one fingerprint was accepted eleven times), and a later one can complete as DONE with its change absent from the document - 11 of 56 writes in a single run, none of them turning up later. The pending window was wide because the document had grown to ~1,700 orphaned test entries, making each write take 10-260 seconds, but the behaviour is GCP's and any concurrent writers can hit it. So a write now counts as done only when the callback's changes can be read back, and is redone on fresh metadata otherwise; after the retries are exhausted it raises MetadataWriteNotApplied. A callback that changes nothing sends no write, since every write re-uploads the whole document and moves the fingerprint for everyone else. add_metadata_item checks the fetched metadata for the key itself and raises DuplicateResourceException - a key already holding this write's value counts as done, so a retry after a lost write never appends a second copy - with GCP's own duplicate-key rejection kept as the backstop. remove_metadata_item returns False when there was nothing to remove, which its callers had always tested for; it returned True unconditionally. The tests drive the real save operation and wait_for_operation against a fake compute client holding a server-side document whose operations can apply, conflict, or complete without applying.
nuwang
force-pushed
the
gcp-metadata-conflict-retry
branch
from
September 17, 2026 17:20
c5e6348 to
878a991
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause of the recurring GCP live-suite failures (
test_crud_network,test_crud_router,test_crud_vm_firewall*,test_crud_key_pair_service), most recently both attempts of the #348 run and earlier onmain.What was happening
GCP keeps labels for networks, routers, firewalls and key pairs in the project-wide
commonInstanceMetadata, and every label write re-uploads that whole document under an optimistic fingerprint. The live suite runs-n 5, so five workers race on one document.gcp_metadata_save_ophas a tenacity retry for exactly this — but its predicate only recognises the conflict as anHttpErrorcarrying the message. A concurrent writer produces it differently:setCommonInstanceMetadatareturns 200 with an operation, the operation then completes withCONDITION_NOT_MET, andwait_for_operationraised that as a plainException(result['error']). The predicate never fired, so every collision was fatal on the first attempt. Whether a run passed depended on whether xdist's schedule happened to collide — and since that schedule is deterministic for a given test set, the same five tests failed identically on both attempts.Fix
wait_for_operationraisesGCPOperationError, aProviderInternalExceptioncarrying the operation's error payload with acodesproperty.CONDITION_NOT_METfrom it, alongside the existing HTTP form. The save already re-fetches the metadata (and a fresh fingerprint) on each attempt, so the retry is correct once it fires.The exception message is unchanged. Callers catching
CloudBridgeBaseExceptionnow see the typed error instead of the middleware's generic wrapper; nothing in the codebase inspected the old one.Tests
tests/test_gcp_metadata_save.pydrives the realgcp_metadata_save_opand the realwait_for_operationagainst a fake compute client, offline:CONDITION_NOT_METis retried, the metadata is re-fetched, and the second upload carries the new fingerprint;GCPOperationErrorwith its codes.The GCP live job on this PR is the end-to-end check.
Changelog under
4.4.2 - unreleased— same heading as #348, so whichever merges second needs a trivial changelog rebase.