Преглед изворни кода

Retry GCP metadata writes on an operation-level fingerprint conflict

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.
Nuwan Goonasekera пре 6 дана
родитељ
комит
88f637dd9b

+ 18 - 0
CHANGELOG.rst

@@ -1,3 +1,21 @@
+4.4.2 - unreleased
+------------------
+
+## Fixes
+* **GCP metadata writes now retry when a concurrent writer invalidates the
+  fingerprint.** Labels for networks, routers, firewalls and key pairs live in
+  the project-wide common instance metadata, which every write re-uploads
+  under an optimistic fingerprint. The retry for a stale fingerprint only
+  recognised the conflict as an HTTP error, but 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 ignored. So under parallel use every
+  collision failed on the first attempt - the cause of the recurring
+  ``test_crud_*`` failures in the GCP live suite. ``wait_for_operation`` now
+  raises ``GCPOperationError`` (a ``ProviderInternalException`` carrying the
+  operation's error payload and ``codes``), and the metadata save retries on
+  it with freshly fetched metadata, as it always did for the HTTP form.
+
 4.4.1 - August 21, 2026 (sha 093ef669598d9f324be28d400a851396739cf1d8)
 ----------------------------------------------------------------------
 

+ 24 - 1
cloudbridge/providers/gcp/helpers.py

@@ -19,6 +19,22 @@ if TYPE_CHECKING:
     from .provider import GCPCloudProvider
 
 
+class GCPOperationError(ProviderInternalException):
+    """A GCP operation completed with an error.
+
+    Raised by ``wait_for_operation`` with the operation's ``error`` payload,
+    so callers can act on the error codes rather than parse the message.
+    """
+
+    def __init__(self, error: Any) -> None:
+        super().__init__(error)
+        self.error = error
+
+    @property
+    def codes(self) -> list[str]:
+        return [e.get('code', '') for e in self.error.get('errors', [])]
+
+
 def gcp_projects(provider: "GCPCloudProvider") -> Any:
     return provider.gcp_compute.projects()
 
@@ -44,7 +60,14 @@ def get_common_metadata(provider: "GCPCloudProvider") -> Any:
 
 
 def __if_fingerprint_differs(e: BaseException) -> bool:
-    # return True if the CloudError exception is due to subnet being in use
+    """Whether ``e`` is GCP rejecting a metadata write on a stale fingerprint.
+
+    The conflict surfaces in two shapes: an HTTP error on the request itself,
+    or a successfully submitted operation that then completes with
+    ``CONDITION_NOT_MET`` - which is what a concurrent writer produces.
+    """
+    if isinstance(e, GCPOperationError):
+        return 'CONDITION_NOT_MET' in e.codes
     if isinstance(e, HttpError):
         expected_message = 'Supplied fingerprint does not match current ' \
                            'metadata fingerprint.'

+ 2 - 1
cloudbridge/providers/gcp/provider.py

@@ -30,6 +30,7 @@ from cloudbridge.interfaces.services import NetworkingService
 from cloudbridge.interfaces.services import SecurityService
 from cloudbridge.interfaces.services import StorageService
 
+from .helpers import GCPOperationError
 from .services import GCPComputeService
 from .services import GCPDnsService
 from .services import GCPNetworkingService
@@ -401,7 +402,7 @@ class GCPCloudProvider(BaseCloudProvider):
             result = operations.get(**args).execute()
             if result['status'] == 'DONE':
                 if 'error' in result:
-                    raise Exception(result['error'])
+                    raise GCPOperationError(result['error'])
                 return result
 
             time.sleep(0.5)

+ 119 - 0
tests/test_gcp_metadata_save.py

@@ -0,0 +1,119 @@
+"""Retrying GCP common-metadata writes on a fingerprint conflict.
+
+GCP keeps labels and key pairs in the project-wide common instance metadata,
+which every write re-uploads under an optimistic fingerprint. A concurrent
+writer makes the upload's *operation* fail with ``CONDITION_NOT_MET``; that
+is a different path from an HTTP-level error, and the write has to be retried
+with freshly fetched metadata on either. No SDK is involved: the compute
+client is a fake and the provider's real ``wait_for_operation`` polls it.
+"""
+
+import unittest
+from unittest import mock
+
+import tenacity
+
+from cloudbridge.providers.gcp.helpers import GCPOperationError
+from cloudbridge.providers.gcp.helpers import gcp_metadata_save_op
+from cloudbridge.providers.gcp.provider import GCPCloudProvider
+
+FINGERPRINT_CONFLICT = {
+    'errors': [{'code': 'CONDITION_NOT_MET',
+                'message': 'Supplied fingerprint does not match current '
+                           'metadata fingerprint.'}]}
+OTHER_FAILURE = {
+    'errors': [{'code': 'RESOURCE_NOT_FOUND',
+                'message': "The resource 'projects/p' was not found"}]}
+
+
+class _Call:
+    def __init__(self, result):
+        self._result = result
+
+    def execute(self):
+        return self._result
+
+
+class _FakeCompute:
+    """Enough of the compute client for a metadata save: each save yields an
+    operation whose outcome is the next entry in ``operation_results``."""
+
+    def __init__(self, operation_results):
+        self.operation_results = list(operation_results)
+        self.fetches = 0
+        self.saved_bodies = []
+
+    # projects().get() / projects().setCommonInstanceMetadata()
+    def projects(self):
+        return self
+
+    def get(self, project):
+        self.fetches += 1
+        return _Call({'commonInstanceMetadata': {
+            'fingerprint': f'fp-{self.fetches}', 'items': []}})
+
+    def setCommonInstanceMetadata(self, project, body):
+        self.saved_bodies.append(body)
+        return _Call({'name': f'op-{len(self.saved_bodies)}'})
+
+    # globalOperations().get() - polled by wait_for_operation
+    def globalOperations(self):
+        return self
+
+    def get_operation(self, project, operation):
+        outcome = self.operation_results.pop(0)
+        result = {'status': 'DONE'}
+        if outcome is not None:
+            result['error'] = outcome
+        return _Call(result)
+
+
+class _FakeProvider:
+    project_name = 'p'
+    wait_for_operation = GCPCloudProvider.wait_for_operation
+
+    def __init__(self, operation_results):
+        self.gcp_compute = _FakeCompute(operation_results)
+        # wait_for_operation calls operations.get(**args); the fake's
+        # get() is taken by projects().get(project=), so route it.
+        self.gcp_compute.get = self._route_get
+
+    def _route_get(self, **kwargs):
+        if 'operation' in kwargs:
+            return self.gcp_compute.get_operation(**kwargs)
+        return _FakeCompute.get(self.gcp_compute, **kwargs)
+
+
+def _save(provider, callback):
+    # The production wait between attempts is exponential backoff; the test
+    # is about whether a retry happens, not how long it waits.
+    return gcp_metadata_save_op.retry_with(
+        wait=tenacity.wait_none())(provider, callback)
+
+
+class GCPMetadataSaveTestCase(unittest.TestCase):
+
+    def test_fingerprint_conflict_is_retried_with_fresh_metadata(self):
+        provider = _FakeProvider([FINGERPRINT_CONFLICT, None])
+        callback = mock.Mock()
+
+        _save(provider, callback)
+
+        # Two attempts, each on metadata fetched anew so the retry carries
+        # the fingerprint the conflict invalidated.
+        self.assertEqual(provider.gcp_compute.fetches, 2)
+        self.assertEqual(callback.call_count, 2)
+        self.assertEqual(
+            [body['fingerprint'] for body in provider.gcp_compute.saved_bodies],
+            ['fp-1', 'fp-2'])
+
+    def test_other_operation_failures_are_raised_as_typed_errors(self):
+        provider = _FakeProvider([OTHER_FAILURE])
+        callback = mock.Mock()
+
+        with self.assertRaises(GCPOperationError) as raised:
+            _save(provider, callback)
+
+        self.assertEqual(callback.call_count, 1)
+        self.assertEqual(raised.exception.codes, ['RESOURCE_NOT_FOUND'])
+        self.assertIn("was not found", str(raised.exception))