Merge "volume: Migrate 'block storage cleanup' to SDK"

Author: Zuul

openstackclient/tests/unit/volume/v3/test_block_storage_cleanup.py

@@ -17,25 +17,36 @@
 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.volume_client.workers
-        self.worker_mock.reset_mock()
-
-
-class TestBlockStorageCleanup(TestBlockStorage):
-    cleaning, unavailable = volume_fakes.create_cleanup_records()
-
+FAKE_CLEANING = [
+    {
+        'id': 1,
+        'host': 'devstack@fakedriver-1',
+        'binary': 'cinder-volume',
+        'cluster_name': 'fake_cluster',
+    }
+]
+FAKE_UNAVAILABLE = [
+    {
+        'id': 2,
+        'host': 'devstack@fakedriver-2',
+        'binary': 'cinder-scheduler',
+        'cluster_name': 'new_cluster',
+    }
+]
+FAKE_CLEANUP_RESPONSE = {
+    'cleaning': FAKE_CLEANING,
+    'unavailable': FAKE_UNAVAILABLE,
+}
+
+
+class TestBlockStorageCleanup(volume_fakes.TestVolume):
     def setUp(self):
         super().setUp()
 
-        self.worker_mock.clean.return_value = (self.cleaning, self.unavailable)
+        self.volume_sdk_client.cleanup_service_workers.return_value = (
+            FAKE_CLEANUP_RESPONSE
+        )
 
-        # Get the command object to test
         self.cmd = block_storage_cleanup.BlockStorageCleanup(self.app, None)
 
     def test_cleanup(self):
@@ -55,24 +66,28 @@ def test_cleanup(self):
         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
+        expected_data = (
+            (
+                1,
+                'fake_cluster',
+                'devstack@fakedriver-1',
+                'cinder-volume',
+                'Cleaning',
+            ),
+            (
+                2,
+                'new_cluster',
+                'devstack@fakedriver-2',
+                'cinder-scheduler',
+                'Unavailable',
+            ),
         )
-        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()
+        self.volume_sdk_client.cleanup_service_workers.assert_called_once_with()
 
     def test_block_storage_cleanup_pre_324(self):
         arglist = []
@@ -132,22 +147,28 @@ def test_cleanup_with_args(self):
         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 = (
+            (
+                1,
+                'fake_cluster',
+                'devstack@fakedriver-1',
+                'cinder-volume',
+                'Cleaning',
+            ),
+            (
+                2,
+                'new_cluster',
+                'devstack@fakedriver-2',
+                'cinder-scheduler',
+                '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(
+        self.volume_sdk_client.cleanup_service_workers.assert_called_once_with(
             cluster_name=fake_cluster,
             host=fake_host,
             binary=fake_binary,

openstackclient/volume/v3/block_storage_cleanup.py

@@ -14,7 +14,7 @@
 from collections.abc import Iterable
 from typing import Any
 
-from cinderclient import api_versions
+from openstack import utils as sdk_utils
 from osc_lib import exceptions
 
 from openstackclient import command
@@ -33,15 +33,21 @@ def _format_cleanup_response(
     )
     combined_data = []
     for obj in cleaning:
-        details = (obj.id, obj.cluster_name, obj.host, obj.binary, '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,
+            obj['id'],
+            obj['cluster_name'],
+            obj['host'],
+            obj['binary'],
             'Unavailable',
         )
         combined_data.append(details)
@@ -136,9 +142,11 @@ def get_parser(self, prog_name: str) -> argparse.ArgumentParser:
     def take_action(
         self, parsed_args: argparse.Namespace
     ) -> tuple[tuple[str, ...], Iterable[tuple[Any, ...]]]:
-        volume_client = self.app.client_manager.volume
+        volume_client = sdk_utils.ensure_service_version(
+            self.app.client_manager.sdk_connection.volume, '3'
+        )
 
-        if volume_client.api_version < api_versions.APIVersion('3.24'):
+        if not sdk_utils.supports_microversion(volume_client, '3.24'):
             msg = _(
                 "--os-volume-api-version 3.24 or greater is required to "
                 "support the 'block storage cleanup' command"
@@ -157,5 +165,7 @@ def take_action(
         }
 
         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)
+        result = volume_client.cleanup_service_workers(**filters)
+        return _format_cleanup_response(
+            result['cleaning'], result['unavailable']
+        )