Azure: rework how conflict detectino works

JAVGan · JAVGan · commit 37c8615e98a8 · 2025-10-06T17:15:36.000-03:00
This commit changes the existing code of
`AzureService.ensure_can_publish` to behave as follows:

1. It no longer retries on error but simply raises whenever a conflict
   is detected

2. It no longer searches only for "live" or "preview" but for all
   existing submissions

3. It improves the logging mechanism to inform which state the
   submission was and its target.

Rationale:

1. If we simply wait for the publishing to finish as the previous version
   (using retry) we could end up in a scenario where the changes made by
   the library are no longer the latest, causing the Graph API to return
   the error: `The submission cannot be pushed to as its not the latest`

   With that, we would need to retry the whole `publish` operations so
   it makes more sense to just raise `ConflictError` and let `pubtools`
   retry when this error is caught.

2. The previous code were not able to catch a lot of conflicts,
   resulting in many errors with `An In Progress submission already exists`

   By looking into each submission target we may caught conflicts early.

3. Logs will now show exactly what was the reason it was already in
   progress and for which submission target.

4. A retry can/will be implemented on `pubtools-marketplacesvm` on Azure
   `push` in case a `ConflictError` is detected.

5. Replaces !119

Refers to SPSTRAT-611 and SPSTRAT-549

Signed-off-by: Jonathan Gangi &lt;jgangi@redhat.com&gt;
diff --git a/cloudpub/error.py b/cloudpub/error.py
@@ -21,5 +21,9 @@ class NotFoundError(ValueError):
     """Represent a missing resource."""
 
 
+class ConflictError(RuntimeError):
+    """Report a submission conflict error."""
+
+
 class Timeout(Exception):
     """Represent a missing resource."""
diff --git a/cloudpub/ms_azure/service.py b/cloudpub/ms_azure/service.py
@@ -12,7 +12,7 @@
 from tenacity.wait import wait_chain, wait_fixed
 
 from cloudpub.common import BaseService
-from cloudpub.error import InvalidStateError, NotFoundError
+from cloudpub.error import ConflictError, InvalidStateError, NotFoundError
 from cloudpub.models.ms_azure import (
     RESOURCE_MAPING,
     AzureResource,
@@ -467,31 +467,28 @@ def submit_to_status(
         log.debug("Set the status \"%s\" to submission.", status)
         return self.configure(resources=cfg_res)
 
-    @retry(
-        wait=wait_fixed(300),
-        stop=stop_after_delay(max_delay=60 * 60 * 24 * 7),  # Give up after retrying for 7 days,
-        reraise=True,
-    )
     def ensure_can_publish(self, product_id: str) -> None:
         """
         Ensure the offer is not already being published.
 
-        It will wait for up to 7 days retrying to make sure it's possible to publish before
-        giving up and raising.
+        It will raise Conflict if a publish is already in progress in any submission target.
 
         Args:
             product_id (str)
                 The product ID to check the offer's publishing status
         Raises:
-            RuntimeError: whenever a publishing is already in progress.
+            ConflictError: whenever a publishing is already in progress for any submission target.
         """
         log.info("Ensuring no other publishing jobs are in progress for \"%s\"", product_id)
-        submission_targets = ["preview", "live"]
 
-        for target in submission_targets:
-            sub = self.get_submission_state(product_id, state=target)
-            if sub and sub.status and sub.status == "running":
-                raise RuntimeError(f"The offer {product_id} is already being published to {target}")
+        for sub in self.get_submissions(product_id):
+            if sub and sub.status and sub.status != "completed":
+                msg = (
+                    f"The offer {product_id} is already being published to "
+                    f"{sub.target.targetType}: {sub.status}/{sub.result}"
+                )
+                log.error(msg)
+                raise ConflictError(msg)
 
     def get_plan_tech_config(self, product: Product, plan: PlanSummary) -> VMIPlanTechConfig:
         """
diff --git a/tests/ms_azure/test_service.py b/tests/ms_azure/test_service.py
@@ -10,7 +10,6 @@
 from httmock import response
 from requests import Response
 from requests.exceptions import HTTPError
-from tenacity.stop import stop_after_attempt
 
 from cloudpub.common import BaseService
 from cloudpub.error import InvalidStateError, NotFoundError
@@ -750,85 +749,35 @@ def test_submit_to_status_not_found(
 
         mock_configure.assert_not_called()
 
-    @pytest.mark.parametrize("target", ["preview", "live"])
-    @mock.patch("cloudpub.ms_azure.AzureService.get_submission_state")
+    @mock.patch("cloudpub.ms_azure.AzureService.get_submissions")
     def test_ensure_can_publish_success(
         self,
-        mock_getsubst: mock.MagicMock,
-        target: str,
+        mock_getsubs: mock.MagicMock,
         azure_service: AzureService,
     ) -> None:
-        submission = {
-            "$schema": "https://product-ingestion.azureedge.net/schema/submission/2022-03-01-preview2",  # noqa: E501
-            "id": "submission/ffffffff-ffff-ffff-ffff-ffffffffffff/0",
-            "product": "product/ffffffff-ffff-ffff-ffff-ffffffffffff",
-            "target": {"targetType": target},
-            "lifecycleState": "generallyAvailable",
-            "status": "completed",
-            "result": "succeeded",
-            "created": "2024-07-04T22:06:16.2895521Z",
-        }
-        mock_getsubst.return_value = ProductSubmission.from_json(submission)
-        azure_service.ensure_can_publish.retry.sleep = mock.MagicMock()  # type: ignore
-        azure_service.ensure_can_publish.retry.stop = stop_after_attempt(1)  # type: ignore
-
-        azure_service.ensure_can_publish("ffffffff-ffff-ffff-ffff-ffffffffffff")
-
-        # All targets are called by the method, it should pass all
-        mock_getsubst.assert_has_calls(
-            [
-                mock.call("ffffffff-ffff-ffff-ffff-ffffffffffff", state="preview"),
-                mock.call("ffffffff-ffff-ffff-ffff-ffffffffffff", state="live"),
-            ]
-        )
-
-    @pytest.mark.parametrize("target", ["preview", "live"])
-    @mock.patch("cloudpub.ms_azure.AzureService.get_submission_state")
-    def test_ensure_can_publish_success_after_retry(
-        self,
-        mock_getsubst: mock.MagicMock,
-        target: str,
-        azure_service: AzureService,
-    ) -> None:
-        running = {
-            "$schema": "https://product-ingestion.azureedge.net/schema/submission/2022-03-01-preview2",  # noqa: E501
-            "id": "submission/ffffffff-ffff-ffff-ffff-ffffffffffff/0",
-            "product": "product/ffffffff-ffff-ffff-ffff-ffffffffffff",
-            "target": {"targetType": target},
-            "lifecycleState": "generallyAvailable",
-            "status": "running",
-            "result": "pending",
-            "created": "2024-07-04T22:06:16.2895521Z",
-        }
-        complete = {
-            "$schema": "https://product-ingestion.azureedge.net/schema/submission/2022-03-01-preview2",  # noqa: E501
-            "id": "submission/ffffffff-ffff-ffff-ffff-ffffffffffff/0",
-            "product": "product/ffffffff-ffff-ffff-ffff-ffffffffffff",
-            "target": {"targetType": target},
-            "lifecycleState": "generallyAvailable",
-            "status": "completed",
-            "result": "succeeded",
-            "created": "2024-07-04T22:06:16.2895521Z",
-        }
-        mock_getsubst.side_effect = [
-            ProductSubmission.from_json(running),
-            ProductSubmission.from_json(running),
-            ProductSubmission.from_json(complete),
-            ProductSubmission.from_json(complete),
+        submissions = [
+            {
+                "$schema": "https://product-ingestion.azureedge.net/schema/submission/2022-03-01-preview2",  # noqa: E501
+                "id": "submission/ffffffff-ffff-ffff-ffff-ffffffffffff/0",
+                "product": "product/ffffffff-ffff-ffff-ffff-ffffffffffff",
+                "target": {"targetType": tgt},
+                "lifecycleState": "generallyAvailable",
+                "status": "completed",
+                "result": "succeeded",
+                "created": "2024-07-04T22:06:16.2895521Z",
+            }
+            for tgt in ["draft", "preview", "live"]
         ]
-        azure_service.ensure_can_publish.retry.sleep = mock.MagicMock()  # type: ignore
-        azure_service.ensure_can_publish.retry.stop = stop_after_attempt(3)  # type: ignore
+        mock_getsubs.return_value = [ProductSubmission.from_json(s) for s in submissions]
 
         azure_service.ensure_can_publish("ffffffff-ffff-ffff-ffff-ffffffffffff")
-
-        # Calls for "live" and "preview" for 2 times before success == 4
-        assert mock_getsubst.call_count == 4
+        mock_getsubs.assert_called_once()
 
     @pytest.mark.parametrize("target", ["preview", "live"])
-    @mock.patch("cloudpub.ms_azure.AzureService.get_submission_state")
+    @mock.patch("cloudpub.ms_azure.AzureService.get_submissions")
     def test_ensure_can_publish_raises(
         self,
-        mock_getsubst: mock.MagicMock,
+        mock_getsubs: mock.MagicMock,
         target: str,
         azure_service: AzureService,
     ) -> None:
@@ -856,17 +805,13 @@ def test_ensure_can_publish_raises(
             "result": "pending",
             "created": "2024-07-04T22:06:16.2895521Z",
         }
-        if target == "preview":
-            subs = [ProductSubmission.from_json(sub2), ProductSubmission.from_json(sub1)]
-        else:
-            subs = [ProductSubmission.from_json(sub1), ProductSubmission.from_json(sub2)]
-        mock_getsubst.side_effect = subs
+        subs = [ProductSubmission.from_json(sub1), ProductSubmission.from_json(sub2)]
+        mock_getsubs.return_value = subs
 
         err = (
-            f"The offer ffffffff-ffff-ffff-ffff-ffffffffffff is already being published to {target}"
+            "The offer ffffffff-ffff-ffff-ffff-ffffffffffff is already being published to "
+            f"{target}: running/pending"
         )
-        azure_service.ensure_can_publish.retry.sleep = mock.MagicMock()  # type: ignore
-        azure_service.ensure_can_publish.retry.stop = stop_after_attempt(1)  # type: ignore
 
         with pytest.raises(RuntimeError, match=err):
             azure_service.ensure_can_publish("ffffffff-ffff-ffff-ffff-ffffffffffff")
@@ -1651,12 +1596,14 @@ def test_publish_live_arm64_only(
         mock_submit.assert_has_calls(submit_calls)
         mock_ensure_publish.assert_called_once_with(product_obj.id)
 
+    @mock.patch("cloudpub.ms_azure.AzureService.ensure_can_publish")
     @mock.patch("cloudpub.ms_azure.AzureService.compute_targets")
     @mock.patch("cloudpub.ms_azure.AzureService.get_productid")
     def test_publish_live_when_state_is_preview(
         self,
         mock_get_productid: mock.MagicMock,
         mock_compute_targets: mock.MagicMock,
+        mock_ensure_publish: mock.MagicMock,
         token: Dict[str, Any],
         auth_dict: Dict[str, Any],
         configure_running_response: Dict[str, Any],
@@ -1714,8 +1661,6 @@ def test_publish_live_when_state_is_preview(
                 m.get(
                     f"{base_url}/submission/{product_id}",
                     [
-                        {"json": submissions_inprog},  # ensure_can_publish call "preview"
-                        {"json": submissions_inprog},  # ensure_can_publish call "live"
                         {"json": submissions_inprog},  # _is_submission_in_preview call
                         {"json": submissions_inprog},  # submit_to_status check prev_state call
                         {"json": submissions_final},  # submit_to_status validation after configure
@@ -1749,10 +1694,6 @@ def test_publish_live_when_state_is_preview(
             'Requesting the product ID "ffffffff-ffff-ffff-ffff-ffffffffffff" with state "preview".'
             in caplog.text
         )
-        assert (
-            'Ensuring no other publishing jobs are in progress for "ffffffff-ffff-ffff-ffff-ffffffffffff"'  # noqa: E501
-            in caplog.text
-        )
         assert (
             'Looking up for submission in state "preview" for "ffffffff-ffff-ffff-ffff-ffffffffffff"'  # noqa: E501
             in caplog.text
@@ -1782,7 +1723,9 @@ def test_publish_live_when_state_is_preview(
             'Updating the technical configuration for "example-product/plan-1" on "preview".'
             not in caplog.text
         )
+        mock_ensure_publish.assert_called_once()
 
+    @mock.patch("cloudpub.ms_azure.AzureService.ensure_can_publish")
     @mock.patch("cloudpub.ms_azure.AzureService.compute_targets")
     @mock.patch("cloudpub.ms_azure.AzureService.get_productid")
     @mock.patch("cloudpub.ms_azure.AzureService.configure")
@@ -1791,6 +1734,7 @@ def test_publish_live_modular_push(
         mock_configure: mock.MagicMock,
         mock_get_productid: mock.MagicMock,
         mock_compute_targets: mock.MagicMock,
+        mock_ensure_publish: mock.MagicMock,
         token: Dict[str, Any],
         auth_dict: Dict[str, Any],
         configure_success_response: Dict[str, Any],
@@ -1861,8 +1805,6 @@ def test_publish_live_modular_push(
                 m.get(
                     f"{base_url}/submission/{product_id}",
                     [
-                        {"json": {"value": [submission]}},  # ensure_can_publish call "preview"
-                        {"json": {"value": [submission]}},  # ensure_can_publish call "live"
                         {"json": {"value": [submission]}},  # push_preview: call submit_status
                         {"json": {"value": [submission_preview]}},  # push_preview: check result
                         {"json": {"value": [submission_preview]}},  # push_live: call submit_status
@@ -1886,6 +1828,7 @@ def test_publish_live_modular_push(
             'Performing a modular push to "preview" for "ffffffff-ffff-ffff-ffff-ffffffffffff"'
             in caplog.text
         )
+        mock_ensure_publish.assert_called_once()
 
         # Configure request
         mock_configure.assert_has_calls(
