From: Fiete Ostkamp Date: Tue, 24 Feb 2026 11:04:42 +0000 (+0100) Subject: Fix vnf_basic_vm failing due to cleanup error X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;h=340b0cbe38c8dcbd9f0d7fcf11885f4d5f7abb3b;p=testsuite%2Fpythonsdk-tests.git Fix vnf_basic_vm failing due to cleanup error - the test currently fails in the cleanup step because the cloud region cannot be deleted - it cannot be deleted because there are still flavor and image objects in AAI that reference it - add test case to test the CloudRegionCreateStep Issue-ID: INT-2359 Change-Id: Ic368be04befe55fd3e9e5fba284a6a17458187de Signed-off-by: Fiete Ostkamp --- diff --git a/src/onaptests/steps/cloud/cloud_region_create.py b/src/onaptests/steps/cloud/cloud_region_create.py index 1c92a05..22de2f5 100644 --- a/src/onaptests/steps/cloud/cloud_region_create.py +++ b/src/onaptests/steps/cloud/cloud_region_create.py @@ -58,6 +58,50 @@ class CloudRegionCreateStep(BaseStep): complex_name=settings.COMPLEX_PHYSICAL_LOCATION_ID, ) + @staticmethod + def _delete_cloud_region_sub_resources(cloud_region: CloudRegion) -> None: + """Delete child objects that block cloud region deletion. + + AAI enforces referential integrity and will reject a DELETE on a + cloud region that still has image or flavor children. This helper + removes them first. + """ + logger = CloudRegionCreateStep._logger # class-level logger + + # --- flavors --- + try: + flavors_resp = cloud_region.send_message_json( + "GET", "get flavors", f"{cloud_region.url}/flavors" + ) + for flavor in flavors_resp.get("flavor", []): + flavor_id = flavor["flavor-id"] + cloud_region.send_message( + "DELETE", + f"Delete flavor {flavor_id} from cloud region " + f"{cloud_region.cloud_region_id}", + f"{cloud_region.url}/flavors/flavor/{flavor_id}", + params={"resource-version": flavor["resource-version"]}, + ) + except ResourceNotFound: + logger.info("No flavors to clean up in cloud region") + + # --- images --- + try: + images_resp = cloud_region.send_message_json( + "GET", "get images", f"{cloud_region.url}/images" + ) + for image in images_resp.get("image", []): + image_id = image["image-id"] + cloud_region.send_message( + "DELETE", + f"Delete image {image_id} from cloud region " + f"{cloud_region.cloud_region_id}", + f"{cloud_region.url}/images/image/{image_id}", + params={"resource-version": image["resource-version"]}, + ) + except ResourceNotFound: + logger.info("No images to clean up in cloud region") + @BaseStep.store_state(cleanup=True) def cleanup(self) -> None: """Cleanup created cloud region.""" @@ -67,6 +111,7 @@ class CloudRegionCreateStep(BaseStep): cloud_owner=settings.CLOUD_REGION_CLOUD_OWNER, cloud_region_id=settings.CLOUD_REGION_ID, ) + self._delete_cloud_region_sub_resources(cloud_region) cloud_region.delete() except ResourceNotFound: self._logger.info("Resource trying to delete is not available..") diff --git a/tests/test_cloud_region_create.py b/tests/test_cloud_region_create.py new file mode 100644 index 0000000..73bdaf2 --- /dev/null +++ b/tests/test_cloud_region_create.py @@ -0,0 +1,243 @@ +"""Test module for CloudRegionCreateStep cleanup logic.""" + +import sys +from unittest import mock + +from onapsdk.configuration import settings + +# Mock kubernetes helper before any step imports to avoid settings dependency +sys.modules['onaptests.utils.kubernetes'] = mock.MagicMock() + +# Configure required settings before importing step modules +settings.LOG_CONFIG = { + "version": 1, + "disable_existing_loggers": False, + "formatters": { + "default": { + "class": "logging.Formatter", + "format": "%(message)s" + } + }, + "handlers": { + "console": { + "level": "DEBUG", + "class": "logging.StreamHandler", + "formatter": "default" + } + }, + "root": { + "level": "DEBUG", + "handlers": ["console"] + } +} +settings.K8S_TESTS_NAMESPACE = 'test-namespace' +settings.CLEANUP_FLAG = True +settings.IF_VALIDATION = False + +from onapsdk.aai.cloud_infrastructure import CloudRegion # noqa: E402 +from onapsdk.exceptions import ResourceNotFound # noqa: E402 + +from onaptests.steps.cloud.cloud_region_create import CloudRegionCreateStep # noqa: E402 + + +CLOUD_OWNER = "test-cloud-owner" +CLOUD_REGION_ID = "test-region-id" +CLOUD_REGION_URL = ( + f"https://aai.api.sparky.simpledemo.onap.org:30233/aai/v27" + f"/cloud-infrastructure/cloud-regions/cloud-region" + f"/{CLOUD_OWNER}/{CLOUD_REGION_ID}" +) + + +def _make_cloud_region(**kwargs): + """Create a CloudRegion instance with sensible defaults.""" + defaults = dict( + cloud_owner=CLOUD_OWNER, + cloud_region_id=CLOUD_REGION_ID, + orchestration_disabled=False, + in_maint=False, + ) + defaults.update(kwargs) + return CloudRegion(**defaults) + + +# -- _delete_cloud_region_sub_resources ---------------------------------------- + +class TestDeleteCloudRegionSubResources: + """Tests for CloudRegionCreateStep._delete_cloud_region_sub_resources.""" + + @mock.patch.object(CloudRegion, "send_message") + @mock.patch.object(CloudRegion, "send_message_json") + def test_deletes_flavors_and_images(self, mock_json, mock_send): + """Flavors and images present ⇒ each is deleted individually.""" + cloud_region = _make_cloud_region() + + flavors_response = { + "flavor": [ + {"flavor-id": "f1", "resource-version": "rv-f1"}, + {"flavor-id": "f2", "resource-version": "rv-f2"}, + ] + } + images_response = { + "image": [ + {"image-id": "img1", "resource-version": "rv-img1"}, + ] + } + + mock_json.side_effect = [flavors_response, images_response] + + CloudRegionCreateStep._delete_cloud_region_sub_resources(cloud_region) + + # Two GETs: one for flavors, one for images + assert mock_json.call_count == 2 + mock_json.assert_any_call( + "GET", "get flavors", f"{cloud_region.url}/flavors" + ) + mock_json.assert_any_call( + "GET", "get images", f"{cloud_region.url}/images" + ) + + # Three DELETEs total (2 flavors + 1 image) + assert mock_send.call_count == 3 + mock_send.assert_any_call( + "DELETE", + f"Delete flavor f1 from cloud region {CLOUD_REGION_ID}", + f"{cloud_region.url}/flavors/flavor/f1", + params={"resource-version": "rv-f1"}, + ) + mock_send.assert_any_call( + "DELETE", + f"Delete flavor f2 from cloud region {CLOUD_REGION_ID}", + f"{cloud_region.url}/flavors/flavor/f2", + params={"resource-version": "rv-f2"}, + ) + mock_send.assert_any_call( + "DELETE", + f"Delete image img1 from cloud region {CLOUD_REGION_ID}", + f"{cloud_region.url}/images/image/img1", + params={"resource-version": "rv-img1"}, + ) + + @mock.patch.object(CloudRegion, "send_message") + @mock.patch.object(CloudRegion, "send_message_json") + def test_no_flavors_or_images(self, mock_json, mock_send): + """Empty flavors/images lists ⇒ no DELETEs issued.""" + cloud_region = _make_cloud_region() + mock_json.side_effect = [{"flavor": []}, {"image": []}] + + CloudRegionCreateStep._delete_cloud_region_sub_resources(cloud_region) + + assert mock_json.call_count == 2 + mock_send.assert_not_called() + + @mock.patch.object(CloudRegion, "send_message") + @mock.patch.object(CloudRegion, "send_message_json") + def test_resource_not_found_is_skipped(self, mock_json, mock_send): + """Skip gracefully when ResourceNotFound is raised during GET.""" + cloud_region = _make_cloud_region() + # flavors → 404, images → one image present + mock_json.side_effect = [ + ResourceNotFound("no flavors"), + {"image": [{"image-id": "img1", "resource-version": "rv1"}]}, + ] + + CloudRegionCreateStep._delete_cloud_region_sub_resources(cloud_region) + + # Only one DELETE (the image); flavors were skipped + assert mock_send.call_count == 1 + mock_send.assert_called_once_with( + "DELETE", + f"Delete image img1 from cloud region {CLOUD_REGION_ID}", + f"{cloud_region.url}/images/image/img1", + params={"resource-version": "rv1"}, + ) + + @mock.patch.object(CloudRegion, "send_message") + @mock.patch.object(CloudRegion, "send_message_json") + def test_both_resource_not_found(self, mock_json, mock_send): + """Both flavors and images return 404 ⇒ nothing is deleted.""" + cloud_region = _make_cloud_region() + mock_json.side_effect = [ + ResourceNotFound("no flavors"), + ResourceNotFound("no images"), + ] + + CloudRegionCreateStep._delete_cloud_region_sub_resources(cloud_region) + + mock_send.assert_not_called() + + @mock.patch.object(CloudRegion, "send_message") + @mock.patch.object(CloudRegion, "send_message_json") + def test_only_images_present(self, mock_json, mock_send): + """Only images (no flavors key) ⇒ only images are deleted.""" + cloud_region = _make_cloud_region() + mock_json.side_effect = [ + {}, # flavors response with no 'flavor' key + {"image": [{"image-id": "i1", "resource-version": "rv-i1"}]}, + ] + + CloudRegionCreateStep._delete_cloud_region_sub_resources(cloud_region) + + assert mock_send.call_count == 1 + mock_send.assert_called_once_with( + "DELETE", + f"Delete image i1 from cloud region {CLOUD_REGION_ID}", + f"{cloud_region.url}/images/image/i1", + params={"resource-version": "rv-i1"}, + ) + + +# -- cleanup integration ------------------------------------------------------- + +class TestCloudRegionCreateStepCleanup: + """Tests that cleanup() wires _delete_cloud_region_sub_resources correctly.""" + + @mock.patch.object(CloudRegionCreateStep, "_delete_cloud_region_sub_resources") + @mock.patch.object(CloudRegion, "delete") + @mock.patch.object(CloudRegion, "get_by_id") + @mock.patch("onaptests.steps.cloud.cloud_region_create.settings") + def test_cleanup_calls_sub_resource_deletion_before_delete( + self, mock_settings, mock_get_by_id, mock_delete, mock_sub + ): + """cleanup() must delete sub-resources before deleting the region.""" + mock_settings.CLEANUP_FLAG = True + mock_settings.CLOUD_REGION_CLOUD_OWNER = CLOUD_OWNER + mock_settings.CLOUD_REGION_ID = CLOUD_REGION_ID + + cloud_region = _make_cloud_region() + mock_get_by_id.return_value = cloud_region + + call_order = [] + mock_sub.side_effect = lambda cr: call_order.append("sub_resources") + mock_delete.side_effect = lambda: call_order.append("delete") + + step = CloudRegionCreateStep() + # Simulate that execute() has already run so store_state allows cleanup + step._state_execute = True + step._executed = True + step.cleanup() + + mock_sub.assert_called_once_with(cloud_region) + mock_delete.assert_called_once() + assert call_order == ["sub_resources", "delete"], \ + "Sub-resources must be deleted before the cloud region itself" + + @mock.patch.object(CloudRegionCreateStep, "_delete_cloud_region_sub_resources") + @mock.patch.object(CloudRegion, "get_by_id") + @mock.patch("onaptests.steps.cloud.cloud_region_create.settings") + def test_cleanup_handles_resource_not_found( + self, mock_settings, mock_get_by_id, mock_sub + ): + """cleanup() gracefully handles ResourceNotFound from get_by_id.""" + mock_settings.CLEANUP_FLAG = True + mock_settings.CLOUD_REGION_CLOUD_OWNER = CLOUD_OWNER + mock_settings.CLOUD_REGION_ID = CLOUD_REGION_ID + mock_get_by_id.side_effect = ResourceNotFound("gone") + + step = CloudRegionCreateStep() + # Simulate that execute() has already run so store_state allows cleanup + step._state_execute = True + step._executed = True + step.cleanup() + + mock_sub.assert_not_called()