Skip to content

Commit

Permalink
Merge pull request #48 from release-engineering/fix-delete-marketplac…
Browse files Browse the repository at this point in the history
…e-errors

Fixes how delete handles if there's an error.
  • Loading branch information
jajreidy authored Sep 4, 2024
2 parents bc1250e + a15f3f7 commit 7ed31fd
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 9 deletions.
9 changes: 4 additions & 5 deletions src/pubtools/_marketplacesvm/tasks/delete/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def _delete(
return pi
# Cycle through potential marketplaces, this only matters in AmiProducts
# as the build could exist in either aws-na or aws-emea.
delete_failed = False
failed_marketplace = []
for marketplace in marketplaces:
try:
log.info(
Expand All @@ -170,18 +170,17 @@ def _delete(
marketplace,
)
pi = evolve(pi, state=State.DELETED)
delete_failed = False
self.update_rhsm_metadata(push_item)
return pi
except Exception as exc:
# If we failed the image might not exist, not necessarily an error
delete_error = exc
delete_failed = True
if delete_failed:
failed_marketplace.append(marketplace)
if len(failed_marketplace) == len(marketplaces):
log.info(
"Failed to delete %s in %s:%s",
push_item.image_id,
marketplace,
",".join(failed_marketplace),
delete_error,
stack_info=True,
)
Expand Down
36 changes: 36 additions & 0 deletions tests/delete/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,35 @@ def aws_push_item(
return AmiPushItem(**params)


@pytest.fixture
def aws_push_item_2(
ami_release: AmiRelease,
security_group: AmiSecurityGroup,
) -> AmiPushItem:
params = {
"name": "sample_product-1.0.1-1-x86_64.raw",
"description": "foo",
"src": "/foo/bar/image.raw",
"image_id": "ami-aws2",
"dest": ["product-uuid"],
"build": "sample_product-1.0.1-1-x86_64",
"build_info": KojiBuildInfo(
name="test-build", version="1.0.1", release="20230101", id=1234
),
"virtualization": "virt",
"volume": "gp2",
"release": ami_release,
"scanning_port": 22,
"user_name": "fake-user",
"release_notes": "https://access.redhat.com/{major_version}/{major_minor}",
"usage_instructions": "Example. {major_version} - {major_minor}",
"recommended_instance_type": "m5.large",
"marketplace_entity_type": "AmiProduct",
"security_groups": [security_group],
}
return AmiPushItem(**params)


@pytest.fixture
def aws_rhcos_push_item(ami_release: AmiRelease, security_group: AmiSecurityGroup) -> AmiPushItem:
params = {
Expand Down Expand Up @@ -124,6 +153,13 @@ def pub_response(aws_rhcos_push_item: AmiPushItem, aws_push_item: AmiPushItem) -
return [aws_rhcos_push_item, aws_push_item]


@pytest.fixture
def pub_response_diff_amis(
aws_push_item_2: AmiPushItem, aws_push_item: AmiPushItem
) -> List[AmiPushItem]:
return [aws_push_item, aws_push_item_2]


@pytest.fixture
def bad_pub_response() -> List[VHDPushItem]:
params = {
Expand Down
51 changes: 48 additions & 3 deletions tests/delete/test_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ def fake_source(pub_response: List[AmiPushItem]) -> Generator[mock.MagicMock, No
yield m


@pytest.fixture()
def fake_source_dif_amis(
pub_response_diff_amis: List[AmiPushItem],
) -> Generator[mock.MagicMock, None, None]:
with mock.patch("pubtools._marketplacesvm.tasks.delete.command.Source") as m:
m.get.return_value.__enter__.return_value = pub_response_diff_amis
yield m


@pytest.fixture()
def bad_fake_source(
bad_pub_response: List[Dict[str, str]]
Expand Down Expand Up @@ -114,7 +123,7 @@ def test_delete(
)

fake_source.get.assert_called_once()
# There's 3 as the AmiProduct deletes require trying aws-na and aws-emea
# There's 2 as the AmiProduct deletes require trying aws-na and aws-emea
assert fake_cloud_instance.call_count == 2


Expand Down Expand Up @@ -184,7 +193,7 @@ def test_delete_ami_id_not_found_rhsm(
)

fake_source.get.assert_called_once()
# 1 call for RHCOS delete
# 2 call for RHCOS delete
assert fake_cloud_instance.call_count == 2


Expand Down Expand Up @@ -243,10 +252,46 @@ def _delete_push_images(self, push_item, **kwargs):
)

fake_source.get.assert_called_once()
# 0 calls for dry-run, should just report to log
# 3 calls since we errored on aws-na, aws-emea, aws-us-storage
assert fake_cloud_instance.call_count == 3


def test_delete_failed_one(
fake_source_dif_amis: mock.MagicMock,
fake_cloud_instance: mock.MagicMock,
command_tester: CommandTester,
) -> None:
"""Test a failed delete."""
image_seen = []

class FakePublish(FakeCloudProvider):
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

fake_cloud_instance.return_value = FakePublish()
command_tester.test(
lambda: entry_point(VMDelete),
[
"test-delete",
"--credentials",
"eyJtYXJrZXRwbGFjZV9hY2NvdW50IjogInRlc3QtbmEiLCAiYXV0aCI6eyJmb28iOiJiYXIifQo=",
"--rhsm-url",
"https://rhsm.com/test/api/",
"--debug",
"--builds",
"rhcos-x86_64-414.92.202405201754-0,sample_product-1.0.1-1-x86_64",
"pub:https://fakepub.com?task-id=12345",
],
)

fake_source_dif_amis.get.assert_called_once()
# 4 Calls since we errored on the first call
assert fake_cloud_instance.call_count == 4


def test_delete_not_AmiPushItem(
bad_fake_source: mock.MagicMock,
fake_cloud_instance: mock.MagicMock,
Expand Down
2 changes: 1 addition & 1 deletion tests/logs/delete/test_delete/test_delete_failed.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
[ INFO] Failed to delete ami-rhcos1 in aws-china-storage:Random exception
[ INFO] Deleting ami-aws1 in account aws-na
[ INFO] Deleting ami-aws1 in account aws-emea
[ INFO] Failed to delete ami-aws1 in aws-emea:Random exception
[ INFO] Failed to delete ami-aws1 in aws-na,aws-emea:Random exception
[ INFO] Collecting results
[ ERROR] Delete failed
# Raised: 30
1 change: 1 addition & 0 deletions tests/logs/delete/test_delete/test_delete_failed_one.jsonl
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

15 changes: 15 additions & 0 deletions tests/logs/delete/test_delete/test_delete_failed_one.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[ INFO] Loading items from pub:https://fakepub.com?task-id=12345
[ INFO] Deleting ami-aws1 in account aws-na
[ INFO] Deleting ami-aws1 in account aws-emea
[ INFO] Delete finished for ami-aws1 in account aws-emea
[ 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-aws2 in account aws-na
[ INFO] Deleting ami-aws2 in account aws-emea
[ INFO] Delete finished for ami-aws2 in account aws-emea
[ WARNING] AMI image: ami-aws2 not found, skipping update in rhsm.
[ INFO] Collecting results
[ INFO] Delete completed

0 comments on commit 7ed31fd

Please sign in to comment.