Merge pull request #76 from release-engineering/fixdelete

Fix and Improve the DeleteCommand

src/pubtools/_marketplacesvm/cloud_providers/aws.py

@@ -19,6 +19,7 @@
 
 LOG = logging.getLogger("pubtools.marketplacesvm")
 UploadResult = namedtuple("UploadResult", "id")  # NOSONAR
+DeletedImage = namedtuple("DeletedImage", ["image_id", "snapshot_id"])
 
 
 def name_from_push_item(
@@ -640,9 +641,9 @@ def _post_publish(
 
         return push_item, publish_result
 
-    def _delete(self, region: str, **kwargs) -> None:
+    def _delete(self, region: str, **kwargs) -> DeletedImage:
         metadata = AWSDeleteMetadata(**kwargs)
-        self.upload_svc_partial(region).delete(metadata)
+        return self.upload_svc_partial(region).delete(metadata)
 
     def _delete_push_images(self, push_item: AmiPushItem, **kwargs) -> Tuple[AmiPushItem, Any]:
         """
@@ -662,9 +663,8 @@ def _delete_push_images(self, push_item: AmiPushItem, **kwargs) -> Tuple[AmiPush
             "image_id": push_item.image_id,
             "skip_snapshot": keep_snapshot,
         }
-        self._delete(region, **delete_meta_kwargs)
-
-        return AmiPushItem
+        res = self._delete(region, **delete_meta_kwargs)
+        return push_item, res
 
 
 register_provider(

src/pubtools/_marketplacesvm/cloud_providers/base.py

@@ -151,13 +151,15 @@ def _delete_push_images(
         self,
         push_item: T,
         **kwargs,
-    ) -> T:
+    ) -> Tuple[T, Any]:
         """
         Abstract method for deleting images from a public cloud provider.
 
         Args:
             push_item (VMIPushItem)
                 The push item to associate and publish a VM image into a product.
+        Returns:
+            Tuple: The processed push item and the delete result data.
         """
 
     #
@@ -196,7 +198,7 @@ def _post_publish(
         """
         return push_item, publish_result
 
-    def _post_delete(self, push_item: T, **kwargs) -> T:
+    def _post_delete(self, push_item: T, delete_result: Any, **kwargs) -> Tuple[T, Any]:
         """
         Define the default method for post publishing actions.
 
@@ -206,9 +208,9 @@ def _post_delete(self, push_item: T, **kwargs) -> T:
             delete_result (Any)
                 The resulting data from delete.
         Returns:
-            The delete result data.
+            Tuple: The processed push item and the resulting data from delete.
         """
-        return push_item
+        return push_item, delete_result
 
     #
     # Public interfaces - not intended to be changed by subclasses
@@ -286,7 +288,7 @@ def delete_push_images(
         self,
         push_item: T,
         **kwargs,
-    ) -> T:
+    ) -> Tuple[T, Any]:
         """
         Associate an existing VM image with a product and publish the changes.
 
@@ -296,10 +298,10 @@ def delete_push_images(
             builds (List[str])
                 List of builds to delete uploaded images from.
         Returns:
-            object: The publish result data.
+            Tuple: The processed push item and the delete result data.
         """
-        pi = self._delete_push_images(push_item, **kwargs)
-        return self._post_delete(pi, **kwargs)
+        pi, res = self._delete_push_images(push_item, **kwargs)
+        return self._post_delete(pi, res, **kwargs)
 
 
 P = TypeVar('P', bound=CloudProvider)

src/pubtools/_marketplacesvm/cloud_providers/ms_azure.py

@@ -341,7 +341,7 @@ def _post_publish(
 
         return push_item, update_res
 
-    def _delete_push_images(self, push_item, **kwargs):
+    def _delete_push_images(self, push_item, **kwargs) -> Tuple[VHDPushItem, Any]:
         """
         Not implemented for Azure. Passes back push_item without changing anything.
 
@@ -351,11 +351,11 @@ def _delete_push_images(self, push_item, **kwargs):
             builds (List[str])
                 The builds to delete.
         Returns:
-            A VHDPushItem
+            A tuple of VHDPushItem and None at the moment.
         """
         # TODO: Add delete functionality to Azure
         LOG.info("Deleting of Azure images from a push is not implemented yet")
-        return push_item
+        return push_item, None
 
     def ensure_offer_is_writable(self, destination: str, nochannel: bool) -> None:
         """

src/pubtools/_marketplacesvm/tasks/delete/command.py

@@ -49,7 +49,7 @@ def raw_items(self) -> Iterator[AmiPushItem]:
                     self.builds_borg.received_builds.add(item.build_info.id)
                     yield item
 
-    def update_rhsm_metadata(self, push_item: AmiPushItem) -> None:
+    def set_ami_invisible_rhsm(self, push_item: AmiPushItem, provider: str) -> None:
         """
         Update image in RHSM to 'invisible'.
 
@@ -64,9 +64,7 @@ def update_rhsm_metadata(self, push_item: AmiPushItem) -> None:
         else:
             try:
                 img_type = push_item.type or ""
-                product = self.get_rhsm_product(
-                    push_item.release.product, img_type, push_item.marketplace_entity_type
-                )
+                product = self.get_rhsm_product(push_item.release.product, img_type, provider)
             except RuntimeError:
                 log.info("%s not found in RHSM", push_item.release.product)
                 return
@@ -137,6 +135,21 @@ def _convert_provider_name(self, provider_name: str) -> List[str]:
         }
         return accounts[provider_name]
 
+    def _set_ami_invisible(self, push_item: AmiPushItem) -> None:
+        img_id = push_item.image_id
+        provider = push_item.marketplace_entity_type
+        log.debug("Marking AMI %s as invisible on RHSM for the provider %s.", img_id, provider)
+        try:
+            self.set_ami_invisible_rhsm(push_item, provider)
+        except Exception as err:
+            log.warning(
+                "Failed to mark %s invisible on RHSM: %s",
+                img_id,
+                err,
+                stack_info=True,
+                exc_info=True,
+            )
+
     def _delete(
         self,
         push_item: VMIPushItem,
@@ -145,20 +158,21 @@ def _delete(
         marketplaces = self._convert_provider_name(push_item.marketplace_entity_type)
         if push_item.build in self.args.builds:
             if self.args.dry_run:
+                self._set_ami_invisible(push_item)
                 log.info("Would have deleted: %s in build %s", push_item.image_id, push_item.build)
                 self._SKIPPED = True
                 pi = evolve(push_item, state=State.SKIPPED)
-                self.update_rhsm_metadata(push_item)
                 return pi
             # Cycle through potential marketplaces, this only matters in AmiProducts
             # as the build could exist in either aws-na or aws-emea.
             for marketplace in marketplaces:
+                self._set_ami_invisible(push_item)
                 log.info(
                     "Deleting %s in account %s",
                     push_item.image_id,
                     marketplace,
                 )
-                pi = self.cloud_instance(marketplace).delete_push_images(
+                pi, _ = self.cloud_instance(marketplace).delete_push_images(
                     push_item, keep_snapshot=self.args.keep_snapshot, **kwargs
                 )
                 log.info(
@@ -167,7 +181,6 @@ def _delete(
                     marketplace,
                 )
                 pi = evolve(pi, state=State.DELETED)
-                self.update_rhsm_metadata(push_item)
                 return pi
         log.info("Skipped: %s in build %s", push_item.image_id, push_item.build)
         self._SKIPPED = True

tests/delete/test_delete.py

@@ -29,7 +29,7 @@ def _publish(self, push_item, nochannel, overwrite, **kwargs):
         return push_item, nochannel
 
     def _delete_push_images(self, push_item, **kwargs):
-        return push_item
+        return push_item, kwargs
 
 
 @pytest.fixture()
@@ -123,7 +123,6 @@ def test_delete(
     )
 
     fake_source.get.assert_called_once()
-    # There's 2 as the AmiProduct deletes require trying aws-na and aws-emea
     assert fake_cloud_instance.call_count == 2
 
 
@@ -193,7 +192,6 @@ def test_delete_ami_id_not_found_rhsm(
     )
 
     fake_source.get.assert_called_once()
-    # 2 call for RHCOS delete
     assert fake_cloud_instance.call_count == 2
 
 
@@ -269,7 +267,7 @@ def _delete_push_images(self, push_item, **kwargs):
             if push_item.image_id not in image_seen:
                 image_seen.append(push_item.image_id)
                 raise Exception("Random exception")
-            return push_item
+            return push_item, kwargs
 
     fake_cloud_instance.return_value = FakePublish()
     command_tester.test(

tests/logs/delete/test_delete/test_delete.txt

@@ -1,15 +1,17 @@
 [    INFO] Loading items from pub:https://fakepub.com?task-id=12345
-[    INFO] Deleting ami-rhcos1 in account aws-china-storage
-[    INFO] Delete finished for ami-rhcos1 in account aws-china-storage
+[   DEBUG] Marking AMI ami-rhcos1 as invisible on RHSM for the provider ACN.
 [   DEBUG] Listing all images from rhsm, https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
 [   DEBUG] Searching for product sample_product_HOURLY for provider ACN in rhsm
 [   DEBUG] Fetching product from https://rhsm.com/v1/internal/cloud_access_providers/amazon/provider_image_groups
 [   DEBUG] 5 Products(AWS provider) in rhsm: RHEL_HA(awstest), SAP(awstest), rhcos(ACN), sample_product(fake), sample_product_HOURLY(ACN)
 [    INFO] Attempting to update the existing image ami-rhcos1 in rhsm
 [    INFO] Existing image ami-rhcos1 succesfully updated in rhsm
-[    INFO] Deleting ami-aws1 in account aws-na
-[    INFO] Delete finished for ami-aws1 in account aws-na
+[    INFO] Deleting ami-rhcos1 in account aws-china-storage
+[    INFO] Delete finished for ami-rhcos1 in account aws-china-storage
+[   DEBUG] Marking AMI ami-aws1 as invisible on RHSM for the provider AmiProduct.
 [   DEBUG] Searching for product sample_product for provider AmiProduct in rhsm
 [    INFO] sample_product not found in RHSM
+[    INFO] Deleting ami-aws1 in account aws-na
+[    INFO] Delete finished for ami-aws1 in account aws-na
 [    INFO] Collecting results
 [    INFO] Delete completed

tests/logs/delete/test_delete/test_delete_ami_id_not_found_rhsm.txt

@@ -1,14 +1,16 @@
 [    INFO] Loading items from pub:https://fakepub.com?task-id=12345
-[    INFO] Deleting ami-rhcos1 in account aws-china-storage
-[    INFO] Delete finished for ami-rhcos1 in account aws-china-storage
+[   DEBUG] Marking AMI ami-rhcos1 as invisible on RHSM for the provider ACN.
 [   DEBUG] Listing all images from rhsm, https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
 [   DEBUG] Searching for product sample_product_HOURLY for provider ACN in rhsm
 [   DEBUG] Fetching product from https://rhsm.com/v1/internal/cloud_access_providers/amazon/provider_image_groups
 [   DEBUG] 5 Products(AWS provider) in rhsm: RHEL_HA(awstest), SAP(awstest), rhcos(ACN), sample_product(fake), sample_product_HOURLY(ACN)
 [    INFO] Attempting to update the existing image ami-rhcos1 in rhsm
 [    INFO] Existing image ami-rhcos1 succesfully updated in rhsm
+[    INFO] Deleting ami-rhcos1 in account aws-china-storage
+[    INFO] Delete finished for ami-rhcos1 in account aws-china-storage
+[   DEBUG] Marking AMI ami-aws1 as invisible on RHSM for the provider AmiProduct.
+[ WARNING] AMI image: ami-aws1 not found, skipping update in rhsm.
 [    INFO] Deleting ami-aws1 in account aws-na
 [    INFO] Delete finished for ami-aws1 in account aws-na
-[ WARNING] AMI image: ami-aws1 not found, skipping update in rhsm.
 [    INFO] Collecting results
 [    INFO] Delete completed

tests/logs/delete/test_delete/test_delete_bad_rhsm.txt

@@ -1,10 +1,18 @@
 [    INFO] Loading items from pub:https://fakepub.com?task-id=12345
-[    INFO] Deleting ami-rhcos1 in account aws-china-storage
-[    INFO] Delete finished for ami-rhcos1 in account aws-china-storage
+[   DEBUG] Marking AMI ami-rhcos1 as invisible on RHSM for the provider ACN.
 [   DEBUG] Listing all images from rhsm, https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
 [   DEBUG] Searching for product sample_product_HOURLY for provider ACN in rhsm
 [   DEBUG] Fetching product from https://rhsm.com/v1/internal/cloud_access_providers/amazon/provider_image_groups
 [   DEBUG] 5 Products(AWS provider) in rhsm: RHEL_HA(awstest), SAP(awstest), rhcos(ACN), sample_product(fake), sample_product_HOURLY(ACN)
 [    INFO] Attempting to update the existing image ami-rhcos1 in rhsm
 [   ERROR] Failed updating image ami-rhcos1
-# Raised: 400 Client Error: None for url: https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
+[ WARNING] Failed to mark ami-rhcos1 invisible on RHSM: 400 Client Error: None for url: https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
+[    INFO] Deleting ami-rhcos1 in account aws-china-storage
+[    INFO] Delete finished for ami-rhcos1 in account aws-china-storage
+[   DEBUG] Marking AMI ami-aws1 as invisible on RHSM for the provider AmiProduct.
+[   DEBUG] Searching for product sample_product for provider AmiProduct in rhsm
+[    INFO] sample_product not found in RHSM
+[    INFO] Deleting ami-aws1 in account aws-na
+[    INFO] Delete finished for ami-aws1 in account aws-na
+[    INFO] Collecting results
+[    INFO] Delete completed

tests/logs/delete/test_delete/test_delete_dry_run.txt

@@ -1,12 +1,14 @@
 [    INFO] Loading items from pub:https://fakepub.com?task-id=12345
-[    INFO] Would have deleted: ami-rhcos1 in build rhcos-x86_64-414.92.202405201754-0
+[   DEBUG] Marking AMI ami-rhcos1 as invisible on RHSM for the provider ACN.
 [   DEBUG] Listing all images from rhsm, https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
 [   DEBUG] Searching for product sample_product_HOURLY for provider ACN in rhsm
 [   DEBUG] Fetching product from https://rhsm.com/v1/internal/cloud_access_providers/amazon/provider_image_groups
 [   DEBUG] 5 Products(AWS provider) in rhsm: RHEL_HA(awstest), SAP(awstest), rhcos(ACN), sample_product(fake), sample_product_HOURLY(ACN)
 [    INFO] Would have updated image ami-rhcos1 in rhsm
-[    INFO] Would have deleted: ami-aws1 in build sample_product-1.0.1-1-x86_64
+[    INFO] Would have deleted: ami-rhcos1 in build rhcos-x86_64-414.92.202405201754-0
+[   DEBUG] Marking AMI ami-aws1 as invisible on RHSM for the provider AmiProduct.
 [   DEBUG] Searching for product sample_product for provider AmiProduct in rhsm
 [    INFO] sample_product not found in RHSM
+[    INFO] Would have deleted: ami-aws1 in build sample_product-1.0.1-1-x86_64
 [    INFO] Collecting results
 [    INFO] Delete completed

tests/logs/delete/test_delete/test_delete_failed.txt

@@ -1,3 +1,10 @@
 [    INFO] Loading items from pub:https://fakepub.com?task-id=12345
+[   DEBUG] Marking AMI ami-rhcos1 as invisible on RHSM for the provider ACN.
+[   DEBUG] Listing all images from rhsm, https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
+[   DEBUG] Searching for product sample_product_HOURLY for provider ACN in rhsm
+[   DEBUG] Fetching product from https://rhsm.com/v1/internal/cloud_access_providers/amazon/provider_image_groups
+[   DEBUG] 5 Products(AWS provider) in rhsm: RHEL_HA(awstest), SAP(awstest), rhcos(ACN), sample_product(fake), sample_product_HOURLY(ACN)
+[    INFO] Attempting to update the existing image ami-rhcos1 in rhsm
+[    INFO] Existing image ami-rhcos1 succesfully updated in rhsm
 [    INFO] Deleting ami-rhcos1 in account aws-china-storage
 # Raised: Random exception

tests/logs/delete/test_delete/test_delete_failed_one.txt

@@ -1,3 +1,9 @@
 [    INFO] Loading items from pub:https://fakepub.com?task-id=12345
+[   DEBUG] Marking AMI ami-aws1 as invisible on RHSM for the provider AmiProduct.
+[   DEBUG] Listing all images from rhsm, https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
+[   DEBUG] Searching for product sample_product for provider AmiProduct in rhsm
+[   DEBUG] Fetching product from https://rhsm.com/v1/internal/cloud_access_providers/amazon/provider_image_groups
+[   DEBUG] 5 Products(AWS provider) in rhsm: RHEL_HA(awstest), SAP(awstest), rhcos(ACN), sample_product(fake), sample_product_HOURLY(ACN)
+[    INFO] sample_product not found in RHSM
 [    INFO] Deleting ami-aws1 in account aws-na
 # Raised: Random exception

tests/logs/delete/test_delete/test_delete_skip_build.txt

@@ -1,12 +1,13 @@
 [    INFO] Loading items from pub:https://fakepub.com?task-id=12345
-[    INFO] Deleting ami-rhcos1 in account aws-china-storage
-[    INFO] Delete finished for ami-rhcos1 in account aws-china-storage
+[   DEBUG] Marking AMI ami-rhcos1 as invisible on RHSM for the provider ACN.
 [   DEBUG] Listing all images from rhsm, https://rhsm.com/v1/internal/cloud_access_providers/amazon/amis
 [   DEBUG] Searching for product sample_product_HOURLY for provider ACN in rhsm
 [   DEBUG] Fetching product from https://rhsm.com/v1/internal/cloud_access_providers/amazon/provider_image_groups
 [   DEBUG] 5 Products(AWS provider) in rhsm: RHEL_HA(awstest), SAP(awstest), rhcos(ACN), sample_product(fake), sample_product_HOURLY(ACN)
 [    INFO] Attempting to update the existing image ami-rhcos1 in rhsm
 [    INFO] Existing image ami-rhcos1 succesfully updated in rhsm
+[    INFO] Deleting ami-rhcos1 in account aws-china-storage
+[    INFO] Delete finished for ami-rhcos1 in account aws-china-storage
 [    INFO] Skipped: ami-aws1 in build sample_product-1.0.1-1-x86_64
 [    INFO] Collecting results
 [    INFO] Delete completed