From 068cc997d570b1bcc28075517639c4c76bf5787a Mon Sep 17 00:00:00 2001 From: Jan Hartkopf Date: Mon, 27 Mar 2023 15:46:36 +0200 Subject: [PATCH] Allow resources to be skipped on project cleanup Story: 2010370 Task: 46596 Change-Id: Id6d68e40656c92910f491fe3b66e40c69b44e352 Signed-off-by: Jan Hartkopf --- openstack/block_storage/v3/_proxy.py | 150 +++++----- openstack/cloud/openstackcloud.py | 4 + openstack/compute/v2/_proxy.py | 4 + openstack/dns/v2/_proxy.py | 52 ++-- openstack/network/v2/_proxy.py | 258 ++++++++++-------- openstack/object_store/v1/_proxy.py | 6 + openstack/orchestration/v1/_proxy.py | 4 + openstack/proxy.py | 27 +- openstack/tests/unit/test_proxy.py | 10 + ...eanup-exclude-option-65cba962eaa5b61a.yaml | 6 + 10 files changed, 303 insertions(+), 218 deletions(-) create mode 100644 releasenotes/notes/project-cleanup-exclude-option-65cba962eaa5b61a.yaml diff --git a/openstack/block_storage/v3/_proxy.py b/openstack/block_storage/v3/_proxy.py index 34f03aea9..bb24bd6bc 100644 --- a/openstack/block_storage/v3/_proxy.py +++ b/openstack/block_storage/v3/_proxy.py @@ -1675,6 +1675,7 @@ class Proxy(_base_proxy.BaseBlockStorageProxy): identified_resources=None, filters=None, resource_evaluation_fn=None, + skip_resources=None, ): # It is not possible to delete backup if there are dependent backups. # In order to be able to do cleanup those is required to have multiple @@ -1684,11 +1685,63 @@ class Proxy(_base_proxy.BaseBlockStorageProxy): # required to limit amount of iterations we do (currently pick 10). In # dry_run all those iterations are doing not what we want, therefore # only iterate in a real cleanup mode. - if dry_run: - # Just iterate and evaluate backups in dry_run mode - for obj in self.backups(details=False): + if not self.should_skip_resource_cleanup("backup", skip_resources): + if dry_run: + # Just iterate and evaluate backups in dry_run mode + for obj in self.backups(details=False): + need_delete = self._service_cleanup_del_res( + self.delete_backup, + obj, + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=filters, + resource_evaluation_fn=resource_evaluation_fn, + ) + else: + # Set initial iterations conditions + need_backup_iteration = True + max_iterations = 10 + while need_backup_iteration and max_iterations > 0: + # Reset iteration controls + need_backup_iteration = False + max_iterations -= 1 + backups = [] + # To increase success chance sort backups by age, dependent + # backups are logically younger. + for obj in self.backups( + details=True, sort_key='created_at', sort_dir='desc' + ): + if not obj.has_dependent_backups: + # If no dependent backups - go with it + need_delete = self._service_cleanup_del_res( + self.delete_backup, + obj, + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=filters, + resource_evaluation_fn=resource_evaluation_fn, + ) + if not dry_run and need_delete: + backups.append(obj) + else: + # Otherwise we need another iteration + need_backup_iteration = True + + # Before proceeding need to wait for backups to be deleted + for obj in backups: + try: + self.wait_for_delete(obj) + except exceptions.SDKException: + # Well, did our best, still try further + pass + + if not self.should_skip_resource_cleanup("snapshot", skip_resources): + snapshots = [] + for obj in self.snapshots(details=False): need_delete = self._service_cleanup_del_res( - self.delete_backup, + self.delete_snapshot, obj, dry_run=dry_run, client_status_queue=client_status_queue, @@ -1696,74 +1749,25 @@ class Proxy(_base_proxy.BaseBlockStorageProxy): filters=filters, resource_evaluation_fn=resource_evaluation_fn, ) - else: - # Set initial iterations conditions - need_backup_iteration = True - max_iterations = 10 - while need_backup_iteration and max_iterations > 0: - # Reset iteration controls - need_backup_iteration = False - max_iterations -= 1 - backups = [] - # To increase success chance sort backups by age, dependent - # backups are logically younger. - for obj in self.backups( - details=True, sort_key='created_at', sort_dir='desc' - ): - if not obj.has_dependent_backups: - # If no dependent backups - go with it - need_delete = self._service_cleanup_del_res( - self.delete_backup, - obj, - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=filters, - resource_evaluation_fn=resource_evaluation_fn, - ) - if not dry_run and need_delete: - backups.append(obj) - else: - # Otherwise we need another iteration - need_backup_iteration = True + if not dry_run and need_delete: + snapshots.append(obj) - # Before proceeding need to wait for backups to be deleted - for obj in backups: - try: - self.wait_for_delete(obj) - except exceptions.SDKException: - # Well, did our best, still try further - pass + # Before deleting volumes need to wait for snapshots to be deleted + for obj in snapshots: + try: + self.wait_for_delete(obj) + except exceptions.SDKException: + # Well, did our best, still try further + pass - snapshots = [] - for obj in self.snapshots(details=False): - need_delete = self._service_cleanup_del_res( - self.delete_snapshot, - obj, - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=filters, - resource_evaluation_fn=resource_evaluation_fn, - ) - if not dry_run and need_delete: - snapshots.append(obj) - - # Before deleting volumes need to wait for snapshots to be deleted - for obj in snapshots: - try: - self.wait_for_delete(obj) - except exceptions.SDKException: - # Well, did our best, still try further - pass - - for obj in self.volumes(details=True): - self._service_cleanup_del_res( - self.delete_volume, - obj, - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=filters, - resource_evaluation_fn=resource_evaluation_fn, - ) + if not self.should_skip_resource_cleanup("volume", skip_resources): + for obj in self.volumes(details=True): + self._service_cleanup_del_res( + self.delete_volume, + obj, + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=filters, + resource_evaluation_fn=resource_evaluation_fn, + ) diff --git a/openstack/cloud/openstackcloud.py b/openstack/cloud/openstackcloud.py index b0fc5eca3..28a2f5777 100644 --- a/openstack/cloud/openstackcloud.py +++ b/openstack/cloud/openstackcloud.py @@ -813,6 +813,7 @@ class _OpenStackCloudMixin: status_queue=None, filters=None, resource_evaluation_fn=None, + skip_resources=None, ): """Cleanup the project resources. @@ -829,6 +830,8 @@ class _OpenStackCloudMixin: :param resource_evaluation_fn: A callback function, which will be invoked for each resurce and must return True/False depending on whether resource need to be deleted or not. + :param skip_resources: List of specific resources whose cleanup should + be skipped. """ dependencies = {} get_dep_fn_name = '_get_cleanup_dependencies' @@ -879,6 +882,7 @@ class _OpenStackCloudMixin: identified_resources=cleanup_resources, filters=filters, resource_evaluation_fn=resource_evaluation_fn, + skip_resources=skip_resources, ) except exceptions.ServiceDisabledException: # same reason as above diff --git a/openstack/compute/v2/_proxy.py b/openstack/compute/v2/_proxy.py index 574a38b98..e9917784b 100644 --- a/openstack/compute/v2/_proxy.py +++ b/openstack/compute/v2/_proxy.py @@ -2548,7 +2548,11 @@ class Proxy(proxy.Proxy): identified_resources=None, filters=None, resource_evaluation_fn=None, + skip_resources=None, ): + if self.should_skip_resource_cleanup("server", skip_resources): + return + servers = [] for obj in self.servers(): need_delete = self._service_cleanup_del_res( diff --git a/openstack/dns/v2/_proxy.py b/openstack/dns/v2/_proxy.py index 174093e99..de2ee5742 100644 --- a/openstack/dns/v2/_proxy.py +++ b/openstack/dns/v2/_proxy.py @@ -667,27 +667,33 @@ class Proxy(proxy.Proxy): identified_resources=None, filters=None, resource_evaluation_fn=None, + skip_resources=None, ): - # Delete all zones - for obj in self.zones(): - self._service_cleanup_del_res( - self.delete_zone, - obj, - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=filters, - resource_evaluation_fn=resource_evaluation_fn, - ) - # Unset all floatingIPs - # NOTE: FloatingIPs are not cleaned when filters are set - for obj in self.floating_ips(): - self._service_cleanup_del_res( - self.unset_floating_ip, - obj, - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=filters, - resource_evaluation_fn=resource_evaluation_fn, - ) + if not self.should_skip_resource_cleanup("zone", skip_resources): + # Delete all zones + for obj in self.zones(): + self._service_cleanup_del_res( + self.delete_zone, + obj, + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=filters, + resource_evaluation_fn=resource_evaluation_fn, + ) + + if not self.should_skip_resource_cleanup( + "floating_ip", skip_resources + ): + # Unset all floatingIPs + # NOTE: FloatingIPs are not cleaned when filters are set + for obj in self.floating_ips(): + self._service_cleanup_del_res( + self.unset_floating_ip, + obj, + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=filters, + resource_evaluation_fn=resource_evaluation_fn, + ) diff --git a/openstack/network/v2/_proxy.py b/openstack/network/v2/_proxy.py index d891cf7d8..42e98ad4f 100644 --- a/openstack/network/v2/_proxy.py +++ b/openstack/network/v2/_proxy.py @@ -6212,106 +6212,137 @@ class Proxy(proxy.Proxy, Generic[T]): identified_resources=None, filters=None, resource_evaluation_fn=None, + skip_resources=None, ): project_id = self.get_project_id() - # Delete floating_ips in the project if no filters defined OR all - # filters are matching and port_id is empty - for obj in self.ips(project_id=project_id): - self._service_cleanup_del_res( - self.delete_ip, - obj, - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=filters, - resource_evaluation_fn=fip_cleanup_evaluation, - ) - # Delete (try to delete) all security groups in the project - # Let's hope we can't drop SG in use - for obj in self.security_groups(project_id=project_id): - if obj.name != 'default': + if not self.should_skip_resource_cleanup( + "floating_ip", skip_resources + ): + # Delete floating_ips in the project if no filters defined OR all + # filters are matching and port_id is empty + for obj in self.ips(project_id=project_id): self._service_cleanup_del_res( - self.delete_security_group, + self.delete_ip, obj, dry_run=dry_run, client_status_queue=client_status_queue, identified_resources=identified_resources, filters=filters, - resource_evaluation_fn=resource_evaluation_fn, + resource_evaluation_fn=fip_cleanup_evaluation, ) - # Networks are crazy, try to delete router+net+subnet - # if there are no "other" ports allocated on the net - for net in self.networks(project_id=project_id): - network_has_ports_allocated = False - router_if = list() - for port in self.ports(project_id=project_id, network_id=net.id): - self.log.debug('Looking at port %s' % port) - if port.device_owner in [ - 'network:router_interface', - 'network:router_interface_distributed', - 'network:ha_router_replicated_interface', - ]: - router_if.append(port) - elif port.device_owner == 'network:dhcp': - # we don't treat DHCP as a real port - continue - elif port.device_owner is None or port.device_owner == '': - # Nobody owns the port - go with it - continue - elif ( - identified_resources - and port.device_id not in identified_resources - ): - # It seems some no other service identified this resource - # to be deleted. We can assume it doesn't count - network_has_ports_allocated = True - if network_has_ports_allocated: - # If some ports are on net - we cannot delete it - continue - self.log.debug('Network %s should be deleted' % net) - # __Check__ if we need to drop network according to filters - network_must_be_deleted = self._service_cleanup_del_res( - self.delete_network, - net, - dry_run=True, - client_status_queue=None, - identified_resources=None, - filters=filters, - resource_evaluation_fn=resource_evaluation_fn, - ) - if not network_must_be_deleted: - # If not - check another net - continue - # otherwise disconnect router, drop net, subnet, router - # Disconnect - for port in router_if: - if client_status_queue: - client_status_queue.put(port) - if not dry_run: - try: - self.remove_interface_from_router( - router=port.device_id, port_id=port.id - ) - except exceptions.SDKException: - self.log.error('Cannot delete object %s' % obj) - # router disconnected, drop it - self._service_cleanup_del_res( - self.delete_router, - self.get_router(port.device_id), - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=None, - resource_evaluation_fn=None, - ) - # Drop ports not belonging to anybody - for port in self.ports(project_id=project_id, network_id=net.id): - if port.device_owner is None or port.device_owner == '': + if not self.should_skip_resource_cleanup( + "security_group", skip_resources + ): + # Delete (try to delete) all security groups in the project + # Let's hope we can't drop SG in use + for obj in self.security_groups(project_id=project_id): + if obj.name != 'default': self._service_cleanup_del_res( - self.delete_port, - port, + self.delete_security_group, + obj, + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=filters, + resource_evaluation_fn=resource_evaluation_fn, + ) + + if ( + not self.should_skip_resource_cleanup("network", skip_resources) + and not self.should_skip_resource_cleanup("port", skip_resources) + and not self.should_skip_resource_cleanup("subnet", skip_resources) + ): + # Networks are crazy, try to delete router+net+subnet + # if there are no "other" ports allocated on the net + for net in self.networks(project_id=project_id): + network_has_ports_allocated = False + router_if = list() + for port in self.ports( + project_id=project_id, network_id=net.id + ): + self.log.debug('Looking at port %s' % port) + if port.device_owner in [ + 'network:router_interface', + 'network:router_interface_distributed', + 'network:ha_router_replicated_interface', + ]: + router_if.append(port) + elif port.device_owner == 'network:dhcp': + # we don't treat DHCP as a real port + continue + elif port.device_owner is None or port.device_owner == '': + # Nobody owns the port - go with it + continue + elif ( + identified_resources + and port.device_id not in identified_resources + ): + # It seems some no other service identified this resource + # to be deleted. We can assume it doesn't count + network_has_ports_allocated = True + if network_has_ports_allocated: + # If some ports are on net - we cannot delete it + continue + self.log.debug('Network %s should be deleted' % net) + # __Check__ if we need to drop network according to filters + network_must_be_deleted = self._service_cleanup_del_res( + self.delete_network, + net, + dry_run=True, + client_status_queue=None, + identified_resources=None, + filters=filters, + resource_evaluation_fn=resource_evaluation_fn, + ) + if not network_must_be_deleted: + # If not - check another net + continue + # otherwise disconnect router, drop net, subnet, router + # Disconnect + for port in router_if: + if client_status_queue: + client_status_queue.put(port) + if not dry_run: + try: + self.remove_interface_from_router( + router=port.device_id, port_id=port.id + ) + except exceptions.SDKException: + self.log.error('Cannot delete object %s' % obj) + # router disconnected, drop it + self._service_cleanup_del_res( + self.delete_router, + self.get_router(port.device_id), + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=None, + resource_evaluation_fn=None, + ) + # Drop ports not belonging to anybody + for port in self.ports( + project_id=project_id, network_id=net.id + ): + if port.device_owner is None or port.device_owner == '': + self._service_cleanup_del_res( + self.delete_port, + port, + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=None, + resource_evaluation_fn=None, + ) + + # Drop all subnets in the net (no further conditions) + for obj in self.subnets( + project_id=project_id, network_id=net.id + ): + self._service_cleanup_del_res( + self.delete_subnet, + obj, dry_run=dry_run, client_status_queue=client_status_queue, identified_resources=identified_resources, @@ -6319,43 +6350,38 @@ class Proxy(proxy.Proxy, Generic[T]): resource_evaluation_fn=None, ) - # Drop all subnets in the net (no further conditions) - for obj in self.subnets(project_id=project_id, network_id=net.id): + # And now the network itself (we are here definitely only if we + # need that) self._service_cleanup_del_res( - self.delete_subnet, - obj, + self.delete_network, + net, dry_run=dry_run, client_status_queue=client_status_queue, identified_resources=identified_resources, filters=None, resource_evaluation_fn=None, ) - - # And now the network itself (we are here definitely only if we - # need that) - self._service_cleanup_del_res( - self.delete_network, - net, - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=None, - resource_evaluation_fn=None, + else: + self.log.debug( + "Skipping cleanup of networks, ports and subnets " + "as those resources require none of them to be " + "excluded, but at least one should be kept" ) - # It might happen, that we have routers not attached to anything - for obj in self.routers(): - ports = list(self.ports(device_id=obj.id)) - if len(ports) == 0: - self._service_cleanup_del_res( - self.delete_router, - obj, - dry_run=dry_run, - client_status_queue=client_status_queue, - identified_resources=identified_resources, - filters=None, - resource_evaluation_fn=None, - ) + if not self.should_skip_resource_cleanup("router", skip_resources): + # It might happen, that we have routers not attached to anything + for obj in self.routers(): + ports = list(self.ports(device_id=obj.id)) + if len(ports) == 0: + self._service_cleanup_del_res( + self.delete_router, + obj, + dry_run=dry_run, + client_status_queue=client_status_queue, + identified_resources=identified_resources, + filters=None, + resource_evaluation_fn=None, + ) def fip_cleanup_evaluation(obj, identified_resources=None, filters=None): diff --git a/openstack/object_store/v1/_proxy.py b/openstack/object_store/v1/_proxy.py index ea873728f..fae81678d 100644 --- a/openstack/object_store/v1/_proxy.py +++ b/openstack/object_store/v1/_proxy.py @@ -1130,7 +1130,13 @@ class Proxy(proxy.Proxy): identified_resources=None, filters=None, resource_evaluation_fn=None, + skip_resources=None, ): + if self.should_skip_resource_cleanup( + "container", skip_resources + ) or self.should_skip_resource_cleanup("object", skip_resources): + return + is_bulk_delete_supported = False bulk_delete_max_per_request = None try: diff --git a/openstack/orchestration/v1/_proxy.py b/openstack/orchestration/v1/_proxy.py index 2ac55c97f..27200f2e5 100644 --- a/openstack/orchestration/v1/_proxy.py +++ b/openstack/orchestration/v1/_proxy.py @@ -549,7 +549,11 @@ class Proxy(proxy.Proxy): identified_resources=None, filters=None, resource_evaluation_fn=None, + skip_resources=None, ): + if self.should_skip_resource_cleanup("stack", skip_resources): + return + stacks = [] for obj in self.stacks(): need_delete = self._service_cleanup_del_res( diff --git a/openstack/proxy.py b/openstack/proxy.py index 3a87fbcb1..ce4010721 100644 --- a/openstack/proxy.py +++ b/openstack/proxy.py @@ -100,7 +100,7 @@ class Proxy(adapter.Adapter, Generic[T]): influxdb_config=None, influxdb_client=None, *args, - **kwargs + **kwargs, ): # NOTE(dtantsur): keystoneauth defaults retriable_status_codes to None, # override it with a class-level value. @@ -144,7 +144,7 @@ class Proxy(adapter.Adapter, Generic[T]): connect_retries=1, global_request_id=None, *args, - **kwargs + **kwargs, ): conn = self._get_connection() if not global_request_id: @@ -180,7 +180,7 @@ class Proxy(adapter.Adapter, Generic[T]): connect_retries=connect_retries, raise_exc=raise_exc, global_request_id=global_request_id, - **kwargs + **kwargs, ), ), expiration_time=expiration_time, @@ -196,7 +196,7 @@ class Proxy(adapter.Adapter, Generic[T]): connect_retries=connect_retries, raise_exc=raise_exc, global_request_id=global_request_id, - **kwargs + **kwargs, ) for h in response.history: @@ -623,7 +623,7 @@ class Proxy(adapter.Adapter, Generic[T]): requires_id=True, base_path=None, skip_cache=False, - **attrs + **attrs, ): """Fetch a resource @@ -665,7 +665,7 @@ class Proxy(adapter.Adapter, Generic[T]): paginated=True, base_path=None, jmespath_filters=None, - **attrs + **attrs, ) -> Generator[T, None, None]: """List a resource @@ -736,6 +736,7 @@ class Proxy(adapter.Adapter, Generic[T]): identified_resources=None, filters=None, resource_evaluation_fn=None, + skip_resources=None, ): return None @@ -814,6 +815,20 @@ class Proxy(adapter.Adapter, Generic[T]): else: return False + def should_skip_resource_cleanup(self, resource=None, skip_resources=None): + if resource is None or skip_resources is None: + return False + + resource_name = f"{self.service_type.replace('-', '_')}.{resource}" + + if resource_name in skip_resources: + self.log.debug( + f"Skipping resource {resource_name} " "in project cleanup" + ) + return True + + return False + # TODO(stephenfin): Remove this and all users. Use of this generally indicates # a missing Resource type. diff --git a/openstack/tests/unit/test_proxy.py b/openstack/tests/unit/test_proxy.py index c6526a6c6..6c338f5f3 100644 --- a/openstack/tests/unit/test_proxy.py +++ b/openstack/tests/unit/test_proxy.py @@ -728,6 +728,7 @@ class TestProxyCleanup(base.TestCase): self.res_no_updated.created_at = '2020-01-02T03:04:05' self.sot = proxy.Proxy(self.session) + self.sot.service_type = "block-storage" self.delete_mock = mock.Mock() @@ -867,3 +868,12 @@ class TestProxyCleanup(base.TestCase): ) ) self.assertEqual(self.res, q.get_nowait()) + + def test_should_skip_resource_cleanup(self): + excluded = ["block_storage.backup"] + self.assertTrue( + self.sot.should_skip_resource_cleanup("backup", excluded) + ) + self.assertFalse( + self.sot.should_skip_resource_cleanup("volume", excluded) + ) diff --git a/releasenotes/notes/project-cleanup-exclude-option-65cba962eaa5b61a.yaml b/releasenotes/notes/project-cleanup-exclude-option-65cba962eaa5b61a.yaml new file mode 100644 index 000000000..17516d552 --- /dev/null +++ b/releasenotes/notes/project-cleanup-exclude-option-65cba962eaa5b61a.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Project cleanup now supports skipping specific resources, + which will be kept as-is. Resource names are based on the + resource registry names, e. g. "block_storage.volume".