From 2be359677908b81d9985917fd78f9c6897759fb9 Mon Sep 17 00:00:00 2001 From: whoami-rajat Date: Wed, 1 Feb 2023 15:04:43 +0530 Subject: [PATCH] Add block storage cleanup command This patch adds the ``block storage cleanup`` command that allow operators to cleanup resources (volumes and snapshots) with failed operations by requesting services in other hosts in the same cluster to cleanup resources of a failed service. Change-Id: I1375223f525021db5201fa0a9f9a647d17dd01f7 --- .../command-objects/block-storage-cleanup.rst | 8 + doc/source/cli/data/cinder.csv | 2 +- openstackclient/tests/unit/volume/v3/fakes.py | 32 ++++ .../volume/v3/test_block_storage_cleanup.py | 178 ++++++++++++++++++ .../volume/v3/block_storage_cleanup.py | 146 ++++++++++++++ ...kers-cleanup-command-720573c0f642efe9.yaml | 6 + setup.cfg | 1 + 7 files changed, 372 insertions(+), 1 deletion(-) create mode 100644 doc/source/cli/command-objects/block-storage-cleanup.rst create mode 100644 openstackclient/tests/unit/volume/v3/test_block_storage_cleanup.py create mode 100644 openstackclient/volume/v3/block_storage_cleanup.py create mode 100644 releasenotes/notes/add-workers-cleanup-command-720573c0f642efe9.yaml diff --git a/doc/source/cli/command-objects/block-storage-cleanup.rst b/doc/source/cli/command-objects/block-storage-cleanup.rst new file mode 100644 index 000000000..6a593c118 --- /dev/null +++ b/doc/source/cli/command-objects/block-storage-cleanup.rst @@ -0,0 +1,8 @@ +============= +block storage +============= + +Block Storage v3 + +.. autoprogram-cliff:: openstack.volume.v3 + :command: block storage cleanup diff --git a/doc/source/cli/data/cinder.csv b/doc/source/cli/data/cinder.csv index 55055bfab..ebf57221f 100644 --- a/doc/source/cli/data/cinder.csv +++ b/doc/source/cli/data/cinder.csv @@ -140,7 +140,7 @@ type-update,volume type set,"Updates volume type name description and/or is_publ unmanage,volume delete --remote,Stop managing a volume. upload-to-image,image create --volume,Uploads volume to Image Service as an image. version-list,versions show --service block-storage,List all API versions. (Supported by API versions 3.0 - 3.latest) -work-cleanup,,Request cleanup of services with optional filtering. (Supported by API versions 3.24 - 3.latest) +work-cleanup,block storage cleanup,Request cleanup of services with optional filtering. (Supported by API versions 3.24 - 3.latest) bash-completion,complete,Prints arguments for bash_completion. help,help,Shows help about this program or one of its subcommands. list-extensions,extension list --volume,Lists all available os-api extensions. diff --git a/openstackclient/tests/unit/volume/v3/fakes.py b/openstackclient/tests/unit/volume/v3/fakes.py index 4e8e6597b..7bde154e8 100644 --- a/openstackclient/tests/unit/volume/v3/fakes.py +++ b/openstackclient/tests/unit/volume/v3/fakes.py @@ -49,6 +49,8 @@ class FakeVolumeClient: self.volume_types.resource_class = fakes.FakeResource(None, {}) self.services = mock.Mock() self.services.resource_class = fakes.FakeResource(None, {}) + self.workers = mock.Mock() + self.workers.resource_class = fakes.FakeResource(None, {}) class TestVolume(utils.TestCommand): @@ -455,3 +457,33 @@ def create_service_log_level_entry(attrs=None): service_log_level = fakes.FakeResource( None, service_log_level_info, loaded=True) return service_log_level + + +def create_cleanup_records(): + """Create fake service cleanup records. + + :return: A list of FakeResource objects + """ + cleaning_records = [] + unavailable_records = [] + cleaning_work_info = { + 'id': 1, + 'host': 'devstack@fakedriver-1', + 'binary': 'cinder-volume', + 'cluster_name': 'fake_cluster', + } + unavailable_work_info = { + 'id': 2, + 'host': 'devstack@fakedriver-2', + 'binary': 'cinder-scheduler', + 'cluster_name': 'new_cluster', + } + cleaning_records.append(cleaning_work_info) + unavailable_records.append(unavailable_work_info) + + cleaning = [fakes.FakeResource( + None, obj, loaded=True) for obj in cleaning_records] + unavailable = [fakes.FakeResource( + None, obj, loaded=True) for obj in unavailable_records] + + return cleaning, unavailable diff --git a/openstackclient/tests/unit/volume/v3/test_block_storage_cleanup.py b/openstackclient/tests/unit/volume/v3/test_block_storage_cleanup.py new file mode 100644 index 000000000..b48ce2f91 --- /dev/null +++ b/openstackclient/tests/unit/volume/v3/test_block_storage_cleanup.py @@ -0,0 +1,178 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import uuid + +from cinderclient import api_versions +from osc_lib import exceptions + +from openstackclient.tests.unit.volume.v3 import fakes as volume_fakes +from openstackclient.volume.v3 import block_storage_cleanup + + +class TestBlockStorage(volume_fakes.TestVolume): + + def setUp(self): + super().setUp() + + # Get a shortcut to the BlockStorageWorkerManager Mock + self.worker_mock = self.app.client_manager.volume.workers + self.worker_mock.reset_mock() + + +class TestBlockStorageCleanup(TestBlockStorage): + + cleaning, unavailable = volume_fakes.create_cleanup_records() + + def setUp(self): + super().setUp() + + self.worker_mock.clean.return_value = (self.cleaning, self.unavailable) + + # Get the command object to test + self.cmd = \ + block_storage_cleanup.BlockStorageCleanup(self.app, None) + + def test_cleanup(self): + self.app.client_manager.volume.api_version = \ + api_versions.APIVersion('3.24') + + arglist = [ + ] + verifylist = [ + ('cluster', None), + ('host', None), + ('binary', None), + ('is_up', None), + ('disabled', None), + ('resource_id', None), + ('resource_type', None), + ('service_id', None), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + expected_columns = ('ID', 'Cluster Name', 'Host', 'Binary', 'Status') + cleaning_data = tuple( + ( + obj.id, + obj.cluster_name, + obj.host, + obj.binary, + 'Cleaning' + ) for obj in self.cleaning + ) + unavailable_data = tuple( + ( + obj.id, + obj.cluster_name, + obj.host, + obj.binary, + 'Unavailable' + ) for obj in self.unavailable + ) + expected_data = cleaning_data + unavailable_data + columns, data = self.cmd.take_action(parsed_args) + + self.assertEqual(expected_columns, columns) + self.assertEqual(expected_data, tuple(data)) + + # checking if proper call was made to cleanup resources + # Since we ignore all parameters with None value, we don't + # have any arguments passed to the API + self.worker_mock.clean.assert_called_once_with() + + def test_block_storage_cleanup_pre_324(self): + arglist = [ + ] + verifylist = [ + ('cluster', None), + ('host', None), + ('binary', None), + ('is_up', None), + ('disabled', None), + ('resource_id', None), + ('resource_type', None), + ('service_id', None), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + exc = self.assertRaises(exceptions.CommandError, self.cmd.take_action, + parsed_args) + self.assertIn( + '--os-volume-api-version 3.24 or greater is required', str(exc)) + + def test_cleanup_with_args(self): + self.app.client_manager.volume.api_version = \ + api_versions.APIVersion('3.24') + + fake_cluster = 'fake-cluster' + fake_host = 'fake-host' + fake_binary = 'fake-service' + fake_resource_id = str(uuid.uuid4()) + fake_resource_type = 'Volume' + fake_service_id = 1 + arglist = [ + '--cluster', fake_cluster, + '--host', fake_host, + '--binary', fake_binary, + '--down', + '--enabled', + '--resource-id', fake_resource_id, + '--resource-type', fake_resource_type, + '--service-id', str(fake_service_id), + ] + verifylist = [ + ('cluster', fake_cluster), + ('host', fake_host), + ('binary', fake_binary), + ('is_up', False), + ('disabled', False), + ('resource_id', fake_resource_id), + ('resource_type', fake_resource_type), + ('service_id', fake_service_id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + expected_columns = ('ID', 'Cluster Name', 'Host', 'Binary', 'Status') + cleaning_data = tuple( + ( + obj.id, + obj.cluster_name, + obj.host, + obj.binary, + 'Cleaning' + ) for obj in self.cleaning + ) + unavailable_data = tuple( + ( + obj.id, + obj.cluster_name, + obj.host, + obj.binary, + 'Unavailable' + ) for obj in self.unavailable + ) + expected_data = cleaning_data + unavailable_data + columns, data = self.cmd.take_action(parsed_args) + + self.assertEqual(expected_columns, columns) + self.assertEqual(expected_data, tuple(data)) + + # checking if proper call was made to cleanup resources + self.worker_mock.clean.assert_called_once_with( + cluster_name=fake_cluster, + host=fake_host, + binary=fake_binary, + is_up=False, + disabled=False, + resource_id=fake_resource_id, + resource_type=fake_resource_type, + service_id=fake_service_id) diff --git a/openstackclient/volume/v3/block_storage_cleanup.py b/openstackclient/volume/v3/block_storage_cleanup.py new file mode 100644 index 000000000..f99b82177 --- /dev/null +++ b/openstackclient/volume/v3/block_storage_cleanup.py @@ -0,0 +1,146 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from cinderclient import api_versions +from osc_lib.command import command +from osc_lib import exceptions + +from openstackclient.i18n import _ + + +def _format_cleanup_response(cleaning, unavailable): + column_headers = ( + 'ID', + 'Cluster Name', + 'Host', + 'Binary', + 'Status', + ) + combined_data = [] + for obj in cleaning: + details = (obj.id, obj.cluster_name, obj.host, obj.binary, 'Cleaning') + combined_data.append(details) + + for obj in unavailable: + details = (obj.id, obj.cluster_name, obj.host, obj.binary, + 'Unavailable') + combined_data.append(details) + + return (column_headers, combined_data) + + +class BlockStorageCleanup(command.Lister): + """Do block storage cleanup. + + This command requires ``--os-volume-api-version`` 3.24 or greater. + """ + + def get_parser(self, prog_name): + parser = super().get_parser(prog_name) + parser.add_argument( + '--cluster', + metavar='', + help=_('Name of block storage cluster in which cleanup needs ' + 'to be performed (name only)') + ) + parser.add_argument( + "--host", + metavar="", + default=None, + help=_("Host where the service resides. (name only)") + ) + parser.add_argument( + '--binary', + metavar='', + default=None, + help=_("Name of the service binary.") + ) + service_up_parser = parser.add_mutually_exclusive_group() + service_up_parser.add_argument( + '--up', + dest='is_up', + action='store_true', + default=None, + help=_( + 'Filter by up status. If this is set, services need to be up.' + ) + ) + service_up_parser.add_argument( + '--down', + dest='is_up', + action='store_false', + help=_( + 'Filter by down status. If this is set, services need to be ' + 'down.' + ) + ) + service_disabled_parser = parser.add_mutually_exclusive_group() + service_disabled_parser.add_argument( + '--disabled', + dest='disabled', + action='store_true', + default=None, + help=_('Filter by disabled status.') + ) + service_disabled_parser.add_argument( + '--enabled', + dest='disabled', + action='store_false', + help=_('Filter by enabled status.') + ) + parser.add_argument( + '--resource-id', + metavar='', + default=None, + help=_('UUID of a resource to cleanup.') + ) + parser.add_argument( + '--resource-type', + metavar='', + choices=('Volume', 'Snapshot'), + help=_('Type of resource to cleanup.') + ) + parser.add_argument( + '--service-id', + type=int, + default=None, + help=_( + 'The service ID field from the DB, not the UUID of the ' + 'service.' + ) + ) + return parser + + def take_action(self, parsed_args): + volume_client = self.app.client_manager.volume + + if volume_client.api_version < api_versions.APIVersion('3.24'): + msg = _( + "--os-volume-api-version 3.24 or greater is required to " + "support the 'block storage cleanup' command" + ) + raise exceptions.CommandError(msg) + + filters = { + 'cluster_name': parsed_args.cluster, + 'host': parsed_args.host, + 'binary': parsed_args.binary, + 'is_up': parsed_args.is_up, + 'disabled': parsed_args.disabled, + 'resource_id': parsed_args.resource_id, + 'resource_type': parsed_args.resource_type, + 'service_id': parsed_args.service_id + } + + filters = {k: v for k, v in filters.items() if v is not None} + cleaning, unavailable = volume_client.workers.clean(**filters) + return _format_cleanup_response(cleaning, unavailable) diff --git a/releasenotes/notes/add-workers-cleanup-command-720573c0f642efe9.yaml b/releasenotes/notes/add-workers-cleanup-command-720573c0f642efe9.yaml new file mode 100644 index 000000000..7406cd62f --- /dev/null +++ b/releasenotes/notes/add-workers-cleanup-command-720573c0f642efe9.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Added ``block storage cleanup`` command that allows cleanup + of resources (volumes and snapshots) by services in other nodes + in a cluster in an Active-Active deployments. diff --git a/setup.cfg b/setup.cfg index 6b8a114ee..7ee9f4890 100644 --- a/setup.cfg +++ b/setup.cfg @@ -824,3 +824,4 @@ openstack.volume.v3 = volume_revert = openstackclient.volume.v3.volume:VolumeRevertToSnapshot block_storage_log_level_list = openstackclient.volume.v3.block_storage_log_level:BlockStorageLogLevelList block_storage_log_level_set = openstackclient.volume.v3.block_storage_log_level:BlockStorageLogLevelSet + block_storage_cleanup = openstackclient.volume.v3.block_storage_cleanup:BlockStorageCleanup