fix(ctf): keep spare teardown retryable after dispatch failure

shifter/shifter_platform/ctf/services/range/spares.py

@@ -333,7 +333,7 @@ def cleanup_event_spares(event_id: UUID) -> dict[str, Any]:
     Raises:
         CTFNotFoundError: If the event does not exist.
     """
-    from ctf.bridges import cms_destroy_range
+    from ctf.bridges import cms_destroy_range, cms_find_range_instance_id
 
     event = _get_event(event_id)
     unconsumed = CTFSpareRange.objects.filter(event=event, consumed_by__isnull=True).exclude(
@@ -347,8 +347,18 @@ def cleanup_event_spares(event_id: UUID) -> dict[str, Any]:
     for spare in unconsumed:
         owner = spare.owner_user
         try:
-            if spare.range_instance_id is not None and owner is not None:
-                cms_destroy_range(owner, spare.range_instance_id)
+            if spare.range_instance_id is None and spare.request_id is not None:
+                spare.range_instance_id = cms_find_range_instance_id(spare.request_id)
+                if spare.range_instance_id is not None:
+                    spare.save(update_fields=["range_instance_id", "updated_at"])
+            if spare.range_instance_id is None or owner is None:
+                failed += 1
+                logger.warning(
+                    "cleanup_event_spares: cannot destroy spare with missing range or owner (event=%s)",
+                    safe_log_value(event_id),
+                )
+                continue
+            cms_destroy_range(owner, spare.range_instance_id)
             destroyed += 1
         except Exception:
             failed += 1
@@ -357,6 +367,9 @@ def cleanup_event_spares(event_id: UUID) -> dict[str, Any]:
                 spare.range_instance_id,
                 safe_log_value(event_id),
             )
+            # Preserve the owner and nonterminal status so a later cleanup
+            # attempt can retry. A failed dispatch is not a destroyed range.
+            continue
         if delete_managed_spare_user(owner):
             users_deleted += 1
         spare.status = SpareRangeStatus.FAILED.value

shifter/shifter_platform/tests/ctf/test_services/test_range_spares.py

@@ -409,32 +409,90 @@ def test_tears_down_unconsumed_spares_and_deletes_users(self, event_with_scenari
 
     @pytest.mark.django_db
     def test_a_single_spares_destroy_failure_is_counted_without_aborting_the_others(
-        self, event_with_scenario, organizer_user
+        self, event_with_scenario, organizer_user, monkeypatch
     ):
-        """One spare's underlying ``RangeInstance`` is hard-deleted out from under it
-        (a real, unmocked way to make ``cms_destroy_range`` genuinely raise
-        ``CMSError`` -- "range not found" -- for that spare only), so the loop's
-        ``except Exception`` branch is exercised for real. The other spare must
-        still be destroyed and both managed users still cleaned up.
-        """
+        """A transient CMS dispatch failure retains its owner for retry."""
+        from ctf import bridges
+
         provision_event_spares(event_with_scenario.pk, 2, operator=organizer_user)
         spares = list(CTFSpareRange.objects.filter(event=event_with_scenario))
         owner_ids = [s.owner_user_id for s in spares]
-        doomed_range_id = spares[0].range_instance_id
-        RangeInstance.all_objects.filter(pk=doomed_range_id).delete()
+        retry_range_id = spares[0].range_instance_id
+        original_destroy = bridges.cms_destroy_range
+        failed_once = False
+
+        def transient_destroy(user, range_instance_id):
+            nonlocal failed_once
+            if range_instance_id == retry_range_id and not failed_once:
+                failed_once = True
+                raise RuntimeError("transient dispatch failure")
+            original_destroy(user, range_instance_id)
+
+        monkeypatch.setattr(bridges, "cms_destroy_range", transient_destroy)
 
         result = cleanup_event_spares(event_with_scenario.pk)
 
         assert result["destroyed"] == 1
         assert result["failed"] == 1
-        # Both managed users are freed regardless of whether their range's
-        # destroy call succeeded.
-        assert result["users_deleted"] == 2
-        for owner_id in owner_ids:
-            assert not User.objects.filter(pk=owner_id).exists()
-        for spare in CTFSpareRange.objects.filter(event=event_with_scenario):
-            assert spare.status == SpareRangeStatus.FAILED.value
-            assert spare.owner_user_id is None
+        assert result["users_deleted"] == 1
+        retry_spare = CTFSpareRange.objects.get(range_instance_id=retry_range_id)
+        assert retry_spare.status == SpareRangeStatus.PROVISIONING.value
+        assert retry_spare.owner_user_id == owner_ids[0]
+        assert User.objects.filter(pk=owner_ids[0]).exists()
+        assert not User.objects.filter(pk=owner_ids[1]).exists()
+
+        retried = cleanup_event_spares(event_with_scenario.pk)
+        assert retried["destroyed"] == 1
+        assert retried["failed"] == 0
+        assert retried["users_deleted"] == 1
+        retry_spare.refresh_from_db()
+        assert retry_spare.status == SpareRangeStatus.FAILED.value
+        assert retry_spare.owner_user_id is None
+        assert not User.objects.filter(pk=owner_ids[0]).exists()
+
+    @pytest.mark.django_db
+    def test_resolves_late_range_instance_before_destroy(self, event_with_scenario, organizer_user):
+        provision_event_spares(event_with_scenario.pk, 1, operator=organizer_user)
+        spare = CTFSpareRange.objects.get(event=event_with_scenario)
+        range_id = spare.range_instance_id
+        spare.range_instance_id = None
+        spare.save(update_fields=["range_instance_id", "updated_at"])
+
+        result = cleanup_event_spares(event_with_scenario.pk)
+
+        assert result["destroyed"] == 1
+        spare.refresh_from_db()
+        assert spare.range_instance_id == range_id
+        assert spare.owner_user_id is None
+
+    @pytest.mark.django_db
+    def test_unresolved_range_retains_owner_until_retry(self, event_with_scenario, organizer_user, monkeypatch):
+        from ctf import bridges
+
+        provision_event_spares(event_with_scenario.pk, 1, operator=organizer_user)
+        spare = CTFSpareRange.objects.get(event=event_with_scenario)
+        owner_id = spare.owner_user_id
+        spare.range_instance_id = None
+        spare.save(update_fields=["range_instance_id", "updated_at"])
+        original_find = bridges.cms_find_range_instance_id
+        monkeypatch.setattr(bridges, "cms_find_range_instance_id", lambda _request_id: None)
+
+        deferred = cleanup_event_spares(event_with_scenario.pk)
+
+        assert deferred["destroyed"] == 0
+        assert deferred["failed"] == 1
+        spare.refresh_from_db()
+        assert spare.status == SpareRangeStatus.PROVISIONING.value
+        assert spare.owner_user_id == owner_id
+        assert User.objects.filter(pk=owner_id).exists()
+
+        monkeypatch.setattr(bridges, "cms_find_range_instance_id", original_find)
+        retried = cleanup_event_spares(event_with_scenario.pk)
+        assert retried["destroyed"] == 1
+        assert retried["failed"] == 0
+        spare.refresh_from_db()
+        assert spare.status == SpareRangeStatus.FAILED.value
+        assert spare.owner_user_id is None
 
     @pytest.mark.django_db
     def test_leaves_consumed_spares_alone(self, event_with_scenario, organizer_user, participant_user):