diff --git a/requirements.txt b/requirements.txt index cac1937..92e5e5d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ cloudimg>=1.15.0 pubtools>=1.3.0 more_executors>=2.11.4 pushcollector>=1.3.0 -pushsource>=2.48.1 +pushsource>=2.50.0 strenum>=0.4.15 starmap-client>=2.1.1 cloudpub>=1.2.1 diff --git a/src/pubtools/_marketplacesvm/cloud_providers/ms_azure.py b/src/pubtools/_marketplacesvm/cloud_providers/ms_azure.py index a883e7b..dc257d3 100644 --- a/src/pubtools/_marketplacesvm/cloud_providers/ms_azure.py +++ b/src/pubtools/_marketplacesvm/cloud_providers/ms_azure.py @@ -353,9 +353,10 @@ def _delete_push_images(self, push_item, **kwargs) -> Tuple[VHDPushItem, Any]: Returns: 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, None + name = self._name_from_push_item(push_item) + delete_meta_kwargs = {"image_name": name, "container": UPLOAD_CONTAINER_NAME} + self.upload_svc.delete(**delete_meta_kwargs) + return push_item def ensure_offer_is_writable(self, destination: str, nochannel: bool) -> None: """ diff --git a/src/pubtools/_marketplacesvm/tasks/delete/command.py b/src/pubtools/_marketplacesvm/tasks/delete/command.py index 95e4625..c791b8b 100644 --- a/src/pubtools/_marketplacesvm/tasks/delete/command.py +++ b/src/pubtools/_marketplacesvm/tasks/delete/command.py @@ -6,7 +6,7 @@ from typing import Any, Dict, Iterator, List, Tuple from attrs import asdict, evolve -from pushsource import AmiPushItem, Source, VMIPushItem +from pushsource import AmiPushItem, Source, VHDPushItem, VMIPushItem from ...arguments import SplitAndExtend from ...services import CloudService, CollectorService @@ -39,9 +39,9 @@ def raw_items(self) -> Iterator[AmiPushItem]: with Source.get(source_url) as source: log.info("Loading items from %s", source_url) for item in source: - if not isinstance(item, AmiPushItem): + if not isinstance(item, VMIPushItem): log.warning( - "Push Item %s at %s is not an AmiPushItem, dropping it from the queue.", + "Push Item %s at %s is not a VMIPushItem, dropping it from the queue.", item.name, item.src, ) @@ -156,7 +156,7 @@ def _set_ami_invisible(self, push_item: AmiPushItem, provider: str) -> None: exc_info=True, ) - def _delete( + def _delete_ami( self, push_item: VMIPushItem, **kwargs, @@ -193,6 +193,53 @@ def _delete( pi = evolve(push_item, state=State.SKIPPED) return pi + def _delete_vhd( + self, + push_item: VMIPushItem, + **kwargs, + ) -> VMIPushItem: + if push_item.cloud_info: + marketplaces = [push_item.cloud_info.account] + else: + # If we don't have cloud info we will need to try either + marketplaces = ["azure-na", "azure-emea"] + if push_item.build in self.args.builds: + if self.args.dry_run: + log.info("Would have deleted: %s in build %s", push_item.name, push_item.build) + self._SKIPPED = True + pi = evolve(push_item, state=State.SKIPPED) + 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: + log.info( + "Deleting %s in account %s", + push_item.name, + marketplace, + ) + pi, _ = self.cloud_instance(marketplace).delete_push_images(push_item, **kwargs) + log.info( + "Delete finished for %s in account %s", + push_item.name, + marketplace, + ) + pi = evolve(pi, state=State.DELETED) + return pi + log.info("Skipped: %s in build %s", push_item.name, push_item.build) + self._SKIPPED = True + pi = evolve(push_item, state=State.SKIPPED) + return pi + + def _delete( + self, + push_item: VMIPushItem, + **kwargs, + ) -> VMIPushItem: + if isinstance(push_item, AmiPushItem): + return self._delete_ami(push_item, **kwargs) + elif isinstance(push_item, VHDPushItem): + return self._delete_vhd(push_item, **kwargs) + def add_args(self): """Include the required CLI arguments for VMDelete.""" super(VMDelete, self).add_args() diff --git a/tests/delete/conftest.py b/tests/delete/conftest.py index bc5e3b1..167e22c 100644 --- a/tests/delete/conftest.py +++ b/tests/delete/conftest.py @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-3.0-or-later -from typing import List +from datetime import datetime +from typing import Any, Dict, List import pytest from pushsource import ( @@ -9,6 +10,7 @@ AmiSecurityGroup, KojiBuildInfo, VHDPushItem, + VMIRelease, ) from pubtools._marketplacesvm.tasks.delete.command import VMDelete @@ -44,6 +46,26 @@ def ami_release() -> AmiRelease: return AmiRelease(**params) +@pytest.fixture +def release_params() -> Dict[str, Any]: + return { + "product": "sample-product", + "version": "7.0", + "arch": "x86_64", + "respin": 1, + "date": datetime.now(), + } + + +@pytest.fixture +def push_item_params() -> Dict[str, str]: + return { + "name": "name", + "description": "", + "build_info": KojiBuildInfo(name="test-build", version="7.0", release="20230101"), + } + + @pytest.fixture def security_group() -> AmiSecurityGroup: params = { @@ -149,10 +171,33 @@ def aws_rhcos_push_item(ami_release: AmiRelease, security_group: AmiSecurityGrou @pytest.fixture -def pub_response(aws_rhcos_push_item: AmiPushItem, aws_push_item: AmiPushItem) -> List[AmiPushItem]: +def vhd_push_item(release_params: Dict[str, Any], push_item_params: Dict[str, Any]) -> VHDPushItem: + """Return a minimal VHDPushItem.""" + release = VMIRelease(**release_params) + push_item_params.update( + { + "name": "azure-testing.vhd.xz", + "release": release, + "src": "mnt/azure/azure-testing.vhd.xz", + "dest": ["azure-testing"], + "build": "azure-testing", + } + ) + return VHDPushItem(**push_item_params) + + +@pytest.fixture +def pub_response_ami( + aws_rhcos_push_item: AmiPushItem, aws_push_item: AmiPushItem +) -> List[AmiPushItem]: return [aws_rhcos_push_item, aws_push_item] +@pytest.fixture +def pub_response_azure(vhd_push_item: VHDPushItem) -> List[VHDPushItem]: + return [vhd_push_item] + + @pytest.fixture def pub_response_diff_amis( aws_push_item_2: AmiPushItem, aws_push_item: AmiPushItem @@ -160,11 +205,22 @@ def pub_response_diff_amis( return [aws_push_item, aws_push_item_2] +class BadPubResponse: + name: str + description: str + src: str + + def __init__(self, name, description, src): + self.name = name + self.description = description + self.src = src + + @pytest.fixture -def bad_pub_response() -> List[VHDPushItem]: +def bad_pub_response_vmi() -> List[BadPubResponse]: params = { "name": "vhd_pushitem", "description": "fakevhd", + "src": "fake-src", } - vhd_push_item = VHDPushItem(**params) - return [vhd_push_item, vhd_push_item] + return [BadPubResponse(**params), BadPubResponse(**params)] diff --git a/tests/delete/test_delete.py b/tests/delete/test_delete.py index 02b2f64..27a2458 100644 --- a/tests/delete/test_delete.py +++ b/tests/delete/test_delete.py @@ -5,7 +5,7 @@ import pytest from attrs import evolve -from pushsource import AmiPushItem, VMICloudInfo +from pushsource import AmiPushItem, VHDPushItem, VMICloudInfo from pubtools._marketplacesvm.cloud_providers.base import CloudProvider from pubtools._marketplacesvm.tasks.delete import VMDelete, entry_point @@ -34,14 +34,23 @@ def _delete_push_images(self, push_item, **kwargs): @pytest.fixture() -def fake_source(pub_response: List[AmiPushItem]) -> Generator[mock.MagicMock, None, None]: +def fake_ami_source(pub_response_ami: 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 + m.get.return_value.__enter__.return_value = pub_response_ami yield m @pytest.fixture() -def fake_source_dif_amis( +def fake_azure_source( + pub_response_azure: List[VHDPushItem], +) -> 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_azure + yield m + + +@pytest.fixture() +def fake_ami_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: @@ -50,11 +59,11 @@ def fake_source_dif_amis( @pytest.fixture() -def bad_fake_source( - bad_pub_response: List[Dict[str, str]] +def bad_fake_vmi_source( + bad_pub_response_vmi: List[Dict[str, str]] ) -> Generator[mock.MagicMock, None, None]: with mock.patch("pubtools._marketplacesvm.tasks.delete.command.Source") as m: - m.get.return_value.__enter__.return_value = bad_pub_response + m.get.return_value.__enter__.return_value = bad_pub_response_vmi yield m @@ -102,8 +111,8 @@ def fake_rhsm_api(requests_mocker): requests_mocker.register_uri("POST", re.compile("amazon/amis")) -def test_delete( - fake_source: mock.MagicMock, +def test_delete_ami( + fake_ami_source: mock.MagicMock, fake_cloud_instance: mock.MagicMock, command_tester: CommandTester, ) -> None: @@ -123,8 +132,38 @@ def test_delete( ], ) - fake_source.get.assert_called_once() + fake_ami_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 + assert fake_cloud_instance.call_args_list[0].args == ('aws-china-storage',) + assert fake_cloud_instance.call_args_list[1].args == ('aws-na',) + + +def test_delete_vhd( + fake_azure_source: mock.MagicMock, + fake_cloud_instance: mock.MagicMock, + command_tester: CommandTester, +) -> None: + """Test a successfull delete.""" + command_tester.test( + lambda: entry_point(VMDelete), + [ + "test-delete", + "--credentials", + "eyJtYXJrZXRwbGFjZV9hY2NvdW50IjogInRlc3QtbmEiLCAiYXV0aCI6eyJmb28iOiJiYXIifQo=", + "--rhsm-url", + "https://rhsm.com/test/api/", + "--debug", + "--builds", + "azure-testing", + "pub:https://fakepub.com?task-id=12345", + ], + ) + + fake_azure_source.get.assert_called_once() + assert fake_cloud_instance.call_count == 1 + for call in fake_cloud_instance.call_args_list: + assert call.args == ('azure-na',) @mock.patch("pubtools._marketplacesvm.tasks.delete.command.Source") @@ -157,8 +196,39 @@ def test_delete_using_cloud_info( fake_cloud_instance.assert_called_once_with("aws-us-storage") +def test_delete_vhd_cloud_info( + fake_azure_source: mock.MagicMock, + fake_cloud_instance: mock.MagicMock, + vhd_push_item: VHDPushItem, + command_tester: CommandTester, +) -> None: + """Test a successfull delete.""" + cloud_info = VMICloudInfo(provider="", account="azure-emea") + pi = evolve(vhd_push_item, cloud_info=cloud_info) + fake_azure_source.get.return_value.__enter__.return_value = [pi] + command_tester.test( + lambda: entry_point(VMDelete), + [ + "test-delete", + "--credentials", + "eyJtYXJrZXRwbGFjZV9hY2NvdW50IjogInRlc3QtbmEiLCAiYXV0aCI6eyJmb28iOiJiYXIifQo=", + "--rhsm-url", + "https://rhsm.com/test/api/", + "--debug", + "--builds", + "azure-testing", + "pub:https://fakepub.com?task-id=12345", + ], + ) + + fake_azure_source.get.assert_called_once() + assert fake_cloud_instance.call_count == 1 + for call in fake_cloud_instance.call_args_list: + assert call.args == ('azure-emea',) + + def test_delete_skip_build( - fake_source: mock.MagicMock, + fake_ami_source: mock.MagicMock, fake_cloud_instance: mock.MagicMock, command_tester: CommandTester, ) -> None: @@ -178,13 +248,39 @@ def test_delete_skip_build( ], ) - fake_source.get.assert_called_once() + fake_ami_source.get.assert_called_once() # 1 call for RHCOS delete assert fake_cloud_instance.call_count == 1 +def test_delete_vhd_skipped( + fake_azure_source: mock.MagicMock, + fake_cloud_instance: mock.MagicMock, + command_tester: CommandTester, +) -> None: + """Test a successfull delete.""" + command_tester.test( + lambda: entry_point(VMDelete), + [ + "test-delete", + "--credentials", + "eyJtYXJrZXRwbGFjZV9hY2NvdW50IjogInRlc3QtbmEiLCAiYXV0aCI6eyJmb28iOiJiYXIifQo=", + "--rhsm-url", + "https://rhsm.com/test/api/", + "--debug", + "--builds", + "skipping", + "pub:https://fakepub.com?task-id=12345", + ], + ) + + fake_azure_source.get.assert_called_once() + # 1 call for RHCOS delete + assert fake_cloud_instance.call_count == 0 + + def test_delete_ami_id_not_found_rhsm( - fake_source: mock.MagicMock, + fake_ami_source: mock.MagicMock, fake_cloud_instance: mock.MagicMock, command_tester: CommandTester, requests_mocker, @@ -222,12 +318,13 @@ def test_delete_ami_id_not_found_rhsm( ], ) - fake_source.get.assert_called_once() + fake_ami_source.get.assert_called_once() + # 2 call for RHCOS delete assert fake_cloud_instance.call_count == 2 def test_delete_dry_run( - fake_source: mock.MagicMock, + fake_ami_source: mock.MagicMock, fake_cloud_instance: mock.MagicMock, command_tester: CommandTester, ) -> None: @@ -248,13 +345,40 @@ def test_delete_dry_run( ], ) - fake_source.get.assert_called_once() + fake_ami_source.get.assert_called_once() + # 0 calls for dry-run, should just report to log + assert fake_cloud_instance.call_count == 0 + + +def test_delete_vhd_dry_run( + fake_azure_source: mock.MagicMock, + fake_cloud_instance: mock.MagicMock, + command_tester: CommandTester, +) -> None: + """Test a successfull delete.""" + command_tester.test( + lambda: entry_point(VMDelete), + [ + "test-delete", + "--credentials", + "eyJtYXJrZXRwbGFjZV9hY2NvdW50IjogInRlc3QtbmEiLCAiYXV0aCI6eyJmb28iOiJiYXIifQo=", + "--rhsm-url", + "https://rhsm.com/test/api/", + "--debug", + "--dry-run", + "--builds", + "azure-testing", + "pub:https://fakepub.com?task-id=12345", + ], + ) + + fake_azure_source.get.assert_called_once() # 0 calls for dry-run, should just report to log assert fake_cloud_instance.call_count == 0 def test_delete_failed( - fake_source: mock.MagicMock, + fake_ami_source: mock.MagicMock, fake_cloud_instance: mock.MagicMock, command_tester: CommandTester, ) -> None: @@ -280,13 +404,13 @@ def _delete_push_images(self, push_item, **kwargs): ], ) - fake_source.get.assert_called_once() + fake_ami_source.get.assert_called_once() # 3 calls since we errored on aws-na, aws-emea, aws-us-storage assert fake_cloud_instance.call_count == 1 def test_delete_failed_one( - fake_source_dif_amis: mock.MagicMock, + fake_ami_source_dif_amis: mock.MagicMock, fake_cloud_instance: mock.MagicMock, command_tester: CommandTester, ) -> None: @@ -316,13 +440,13 @@ def _delete_push_images(self, push_item, **kwargs): ], ) - fake_source_dif_amis.get.assert_called_once() + fake_ami_source_dif_amis.get.assert_called_once() # 4 Calls since we errored on the first call assert fake_cloud_instance.call_count == 1 -def test_delete_not_AmiPushItem( - bad_fake_source: mock.MagicMock, +def test_delete_not_VmiPushItem( + bad_fake_vmi_source: mock.MagicMock, fake_cloud_instance: mock.MagicMock, command_tester: CommandTester, ) -> None: @@ -342,13 +466,13 @@ def test_delete_not_AmiPushItem( ], ) - bad_fake_source.get.assert_called_once() + bad_fake_vmi_source.get.assert_called_once() # No calls as there was nothing to work assert fake_cloud_instance.call_count == 0 def test_delete_bad_rhsm( - fake_source: mock.MagicMock, + fake_ami_source: mock.MagicMock, fake_cloud_instance: mock.MagicMock, command_tester: CommandTester, requests_mocker, @@ -371,4 +495,4 @@ def test_delete_bad_rhsm( ], ) - fake_source.get.assert_called_once() + fake_ami_source.get.assert_called_once() diff --git a/tests/logs/delete/test_delete/test_delete.jsonl b/tests/logs/delete/test_delete/test_delete_ami.jsonl similarity index 100% rename from tests/logs/delete/test_delete/test_delete.jsonl rename to tests/logs/delete/test_delete/test_delete_ami.jsonl diff --git a/tests/logs/delete/test_delete/test_delete.txt b/tests/logs/delete/test_delete/test_delete_ami.txt similarity index 100% rename from tests/logs/delete/test_delete/test_delete.txt rename to tests/logs/delete/test_delete/test_delete_ami.txt diff --git a/tests/logs/delete/test_delete/test_delete_not_AmiPushItem.txt b/tests/logs/delete/test_delete/test_delete_not_AmiPushItem.txt deleted file mode 100644 index 81ccd04..0000000 --- a/tests/logs/delete/test_delete/test_delete_not_AmiPushItem.txt +++ /dev/null @@ -1,7 +0,0 @@ -[ INFO] Loading items from pub:https://fakepub.com?task-id=12345 -[ WARNING] Push Item vhd_pushitem at None is not an AmiPushItem, dropping it from the queue. -[ WARNING] Push Item vhd_pushitem at None is not an AmiPushItem, dropping it from the queue. -[ ERROR] No AmiPushItems to process -[ INFO] Collecting results -[ ERROR] Delete failed -# Raised: 30 diff --git a/tests/logs/delete/test_delete/test_delete_not_AmiPushItem.jsonl b/tests/logs/delete/test_delete/test_delete_not_VmiPushItem.jsonl similarity index 100% rename from tests/logs/delete/test_delete/test_delete_not_AmiPushItem.jsonl rename to tests/logs/delete/test_delete/test_delete_not_VmiPushItem.jsonl diff --git a/tests/logs/delete/test_delete/test_delete_not_VmiPushItem.txt b/tests/logs/delete/test_delete/test_delete_not_VmiPushItem.txt new file mode 100644 index 0000000..b56f921 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_not_VmiPushItem.txt @@ -0,0 +1,7 @@ +[ INFO] Loading items from pub:https://fakepub.com?task-id=12345 +[ WARNING] Push Item vhd_pushitem at fake-src is not a VMIPushItem, dropping it from the queue. +[ WARNING] Push Item vhd_pushitem at fake-src is not a VMIPushItem, dropping it from the queue. +[ ERROR] No AmiPushItems to process +[ INFO] Collecting results +[ ERROR] Delete failed +# Raised: 30 diff --git a/tests/logs/delete/test_delete/test_delete_vhd.jsonl b/tests/logs/delete/test_delete/test_delete_vhd.jsonl new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_vhd.jsonl @@ -0,0 +1 @@ + diff --git a/tests/logs/delete/test_delete/test_delete_vhd.txt b/tests/logs/delete/test_delete/test_delete_vhd.txt new file mode 100644 index 0000000..e5f1968 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_vhd.txt @@ -0,0 +1,5 @@ +[ INFO] Loading items from pub:https://fakepub.com?task-id=12345 +[ INFO] Deleting azure-testing.vhd.xz in account azure-na +[ INFO] Delete finished for azure-testing.vhd.xz in account azure-na +[ INFO] Collecting results +[ INFO] Delete completed diff --git a/tests/logs/delete/test_delete/test_delete_vhd_cloud_info.jsonl b/tests/logs/delete/test_delete/test_delete_vhd_cloud_info.jsonl new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_vhd_cloud_info.jsonl @@ -0,0 +1 @@ + diff --git a/tests/logs/delete/test_delete/test_delete_vhd_cloud_info.txt b/tests/logs/delete/test_delete/test_delete_vhd_cloud_info.txt new file mode 100644 index 0000000..850e301 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_vhd_cloud_info.txt @@ -0,0 +1,5 @@ +[ INFO] Loading items from pub:https://fakepub.com?task-id=12345 +[ INFO] Deleting azure-testing.vhd.xz in account azure-emea +[ INFO] Delete finished for azure-testing.vhd.xz in account azure-emea +[ INFO] Collecting results +[ INFO] Delete completed diff --git a/tests/logs/delete/test_delete/test_delete_vhd_dry_run.jsonl b/tests/logs/delete/test_delete/test_delete_vhd_dry_run.jsonl new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_vhd_dry_run.jsonl @@ -0,0 +1 @@ + diff --git a/tests/logs/delete/test_delete/test_delete_vhd_dry_run.txt b/tests/logs/delete/test_delete/test_delete_vhd_dry_run.txt new file mode 100644 index 0000000..3f07a87 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_vhd_dry_run.txt @@ -0,0 +1,4 @@ +[ INFO] Loading items from pub:https://fakepub.com?task-id=12345 +[ INFO] Would have deleted: azure-testing.vhd.xz in build azure-testing +[ INFO] Collecting results +[ INFO] Delete completed diff --git a/tests/logs/delete/test_delete/test_delete_vhd_skipped.jsonl b/tests/logs/delete/test_delete/test_delete_vhd_skipped.jsonl new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_vhd_skipped.jsonl @@ -0,0 +1 @@ + diff --git a/tests/logs/delete/test_delete/test_delete_vhd_skipped.txt b/tests/logs/delete/test_delete/test_delete_vhd_skipped.txt new file mode 100644 index 0000000..1dd2f94 --- /dev/null +++ b/tests/logs/delete/test_delete/test_delete_vhd_skipped.txt @@ -0,0 +1,4 @@ +[ INFO] Loading items from pub:https://fakepub.com?task-id=12345 +[ INFO] Skipped: azure-testing.vhd.xz in build azure-testing +[ INFO] Collecting results +[ INFO] Delete completed