Fixes storing data in push_item when an error with rhsm happens.

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

@@ -398,14 +398,14 @@ def _upload(
                 str(exc),
                 stack_info=True,
             )
+            # At this point pi doesn't exist so we update push_item
             pi = evolve(push_item, state=State.UPLOADFAILED)
             return pi, marketplace
-
         # Then, if we're shipping the community image, we should update the RHSM
         # and change the the AWS group to "all" for the uploaded image
         if ship:
             try:
-                self.update_rhsm_metadata(image, push_item)
+                self.update_rhsm_metadata(image, pi)
                 if push_item.public_image:
                     log.info("Releasing image %s publicly", image.id)
                     groups = ["all"]
@@ -423,7 +423,8 @@ def _upload(
                     )
             except Exception as exc:
                 log.exception("Failed to publish %s: %s", push_item.name, str(exc), stack_info=True)
-                pi = evolve(push_item, state=State.NOTPUSHED)
+                # At this point pi exists so we need to make sure we don't lose anything
+                pi = evolve(pi, state=State.NOTPUSHED)
                 return pi, marketplace
 
         # Finally, if everything went well we return the updated push item

tests/community_push/test_community_push.py

@@ -845,17 +845,31 @@ def test_do_community_push_different_sharing_accounts(
     mock_starmap: mock.MagicMock,
     mock_source: mock.MagicMock,
     ami_push_item: AmiPushItem,
+    fake_cloud_instance: mock.MagicMock,
     starmap_ami_billing_config: Dict[str, Any],
     command_tester: CommandTester,
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
+
+    class FakePublish(FakeCloudProvider):
+        @classmethod
+        def from_credentials(cls, _):
+            return cls()
+
+        def _upload(self, push_item, custom_tags=None, **kwargs):
+            return push_item, UploadResponse({"id": "ami-00000000000000000", "name": "fake-name"})
+
+        def _pre_publish(self, push_item, **kwargs):
+            return push_item, kwargs
+
+        def _publish(self, push_item, nochannel, overwrite, **kwargs):
+            return push_item, nochannel
+
+        def _delete_push_images(self, push_item, **kwargs):
+            return push_item
+
     """Test a community-push with two push-items, each one having different sharing accounts."""
-    mock_ami = mock.MagicMock()
-    mock_ami.id = "ami-00000000000000000"
-    mock_ami.name = "fake-name"
-    mock_cloud_instance = mock.MagicMock()
-    mock_cloud_instance.return_value.upload.return_value = (ami_push_item, mock_ami)
-    monkeypatch.setattr(CommunityVMPush, "cloud_instance", mock_cloud_instance)
+    fake_cloud_instance.return_value = FakePublish()
     mock_source.get.return_value.__enter__.return_value = [ami_push_item for _ in range(2)]
     policy1 = [
         {
@@ -934,33 +948,6 @@ def test_do_community_push_different_sharing_accounts(
         ],
     )
 
-    # Since the push item get updated with destination and some stuff it's easier to just
-    # change it to "mock.ANY" as we just want to test here whether the sharing accounts are correct
-    mock_cloud_instance.return_value.upload.assert_has_calls(
-        [
-            mock.call(
-                mock.ANY,
-                custom_tags=None,
-                ami_tags={'billing_type': 'access'},
-                snapshot_tags={'billing_type': 'access'},
-                container='redhat-cloudimg-fake-destination',
-                accounts=['first_account', 'second_account'],
-                snapshot_accounts=None,
-                ami_version_template='{major}.{minor}.0',
-            ),
-            mock.call(
-                mock.ANY,
-                custom_tags=None,
-                ami_tags={'billing_type': 'access'},
-                snapshot_tags={'billing_type': 'access'},
-                container='redhat-cloudimg-fake-destination2',
-                accounts=['third_account', 'fourth_account'],
-                snapshot_accounts=None,
-                ami_version_template=None,
-            ),
-        ]
-    )
-
 
 @mock.patch("pubtools._marketplacesvm.tasks.community_push.CommunityVMPush.starmap")
 def test_sharing_accounts_community_format(

tests/logs/community_push/test_community_push/test_do_community_push_different_sharing_accounts.txt

@@ -85,7 +85,7 @@
 [    INFO] Attempting to update the existing image ami-00000000000000000 in RHSM
 [   DEBUG] {'image_id': 'ami-00000000000000000', 'image_name': 'fake-name', 'arch': 'x86_64', 'product_name': 'sample_product', 'version': '7.0', 'variant': 'variant'}
 [    INFO] Successfully registered image ami-00000000000000000 with RHSM
-[    INFO] Successfully uploaded ami_pushitem [None] [ami-00000000000000000]
+[    INFO] Successfully uploaded ami_pushitem [fake-destination] [ami-00000000000000000]
 [    INFO] Uploading /foo/bar/sample_product_test.raw to region fake-destination2 (type: access, ship: True, account: aws_storage) with sharing accounts: ['third_account', 'fourth_account'] and snapshot accounts: None
 [    INFO] Upload finished for ami_pushitem on fake-destination2
 [    INFO] Creating region fake-destination2 [anotherprovider]
@@ -94,6 +94,6 @@
 [    INFO] Attempting to update the existing image ami-00000000000000000 in RHSM
 [   DEBUG] {'image_id': 'ami-00000000000000000', 'image_name': 'fake-name', 'arch': 'x86_64', 'product_name': 'sample_product', 'version': '7.0', 'variant': 'variant'}
 [    INFO] Successfully registered image ami-00000000000000000 with RHSM
-[    INFO] Successfully uploaded ami_pushitem [None] [ami-00000000000000000]
+[    INFO] Successfully uploaded ami_pushitem [fake-destination2] [ami-00000000000000000]
 [    INFO] Collecting results
 [    INFO] Community VM push completed