volume: Migrate 'volume attachment *' to SDK

rajathere · stephenfin · commit 9f30ee9af2f4 · 2024-07-08T09:59:21.000+01:00
This patch migrates the volume attachment create, get, list,
delete, update and complete commands to SDK.

Change-Id: Ib237d25cc1c3fc72946b9d088ff3447433162130
diff --git a/openstackclient/tests/unit/volume/v3/fakes.py b/openstackclient/tests/unit/volume/v3/fakes.py
@@ -24,6 +24,7 @@
 from openstack.block_storage.v3 import extension as _extension
 from openstack.block_storage.v3 import resource_filter as _filters
 from openstack.block_storage.v3 import volume as _volume
+from openstack.compute.v2 import _proxy as _compute_proxy
 from openstack.image.v2 import _proxy as _image_proxy
 
 from openstackclient.tests.unit import fakes
@@ -129,16 +130,16 @@ class TestVolume(
     def setUp(self):
         super().setUp()
 
-        # avoid circular imports
-        from openstackclient.tests.unit.compute.v2 import (
-            fakes as compute_fakes,
+        # avoid circular imports by defining this manually rather than using
+        # openstackclient.tests.unit.compute.v2.fakes.FakeClientMixin
+        # TODO(stephenfin): Rename to 'compute_client' once all commands are
+        # migrated to SDK
+        self.app.client_manager.sdk_connection.compute = mock.Mock(
+            _compute_proxy.Proxy
         )
-
-        self.app.client_manager.compute = compute_fakes.FakeComputev2Client(
-            endpoint=fakes.AUTH_URL,
-            token=fakes.AUTH_TOKEN,
+        self.compute_sdk_client = (
+            self.app.client_manager.sdk_connection.compute
         )
-        self.compute_client = self.app.client_manager.compute
 
         # avoid circular imports by defining this manually rather than using
         # openstackclient.tests.unit.image.v2.fakes.FakeClientMixin
diff --git a/openstackclient/tests/unit/volume/v3/test_volume_attachment.py b/openstackclient/tests/unit/volume/v3/test_volume_attachment.py
@@ -23,22 +23,12 @@ class TestVolumeAttachment(volume_fakes.TestVolume):
     def setUp(self):
         super().setUp()
 
-        self.volumes_mock = self.volume_client.volumes
-        self.volumes_mock.reset_mock()
-
-        self.volume_attachments_mock = self.volume_client.attachments
-        self.volume_attachments_mock.reset_mock()
-
-        self.projects_mock = self.identity_client.projects
-        self.projects_mock.reset_mock()
-
-        self.servers_mock = self.compute_client.servers
-        self.servers_mock.reset_mock()
+        self.projects_mock = self.app.client_manager.identity.projects
 
 
 class TestVolumeAttachmentCreate(TestVolumeAttachment):
     volume = volume_fakes.create_one_volume()
-    server = compute_fakes.create_one_server()
+    server = compute_fakes.create_one_sdk_server()
     volume_attachment = volume_fakes.create_one_volume_attachment(
         attrs={'instance': server.id, 'volume_id': volume.id},
     )
@@ -67,12 +57,11 @@ class TestVolumeAttachmentCreate(TestVolumeAttachment):
     def setUp(self):
         super().setUp()
 
-        self.volumes_mock.get.return_value = self.volume
-        self.servers_mock.get.return_value = self.server
-        # VolumeAttachmentManager.create returns a dict
-        self.volume_attachments_mock.create.return_value = (
+        self.volume_sdk_client.find_volume.return_value = self.volume
+        self.volume_sdk_client.create_attachment.return_value = (
             self.volume_attachment.to_dict()
         )
+        self.compute_sdk_client.find_server.return_value = self.server
 
         self.cmd = volume_attachment.CreateVolumeAttachment(self.app, None)
 
@@ -100,13 +89,17 @@ def test_volume_attachment_create(self):
 
         columns, data = self.cmd.take_action(parsed_args)
 
-        self.volumes_mock.get.assert_called_once_with(self.volume.id)
-        self.servers_mock.get.assert_called_once_with(self.server.id)
-        self.volume_attachments_mock.create.assert_called_once_with(
+        self.volume_sdk_client.find_volume.assert_called_once_with(
+            self.volume.id, ignore_missing=False
+        )
+        self.compute_sdk_client.find_server.assert_called_once_with(
+            self.server.id, ignore_missing=False
+        )
+        self.volume_sdk_client.create_attachment.assert_called_once_with(
             self.volume.id,
-            {},
-            self.server.id,
-            None,
+            connector={},
+            instance=self.server.id,
+            mode=None,
         )
         self.assertEqual(self.columns, columns)
         self.assertCountEqual(self.data, data)
@@ -163,13 +156,17 @@ def test_volume_attachment_create_with_connect(self):
             ]
         )
 
-        self.volumes_mock.get.assert_called_once_with(self.volume.id)
-        self.servers_mock.get.assert_called_once_with(self.server.id)
-        self.volume_attachments_mock.create.assert_called_once_with(
+        self.volume_sdk_client.find_volume.assert_called_once_with(
+            self.volume.id, ignore_missing=False
+        )
+        self.compute_sdk_client.find_server.assert_called_once_with(
+            self.server.id, ignore_missing=False
+        )
+        self.volume_sdk_client.create_attachment.assert_called_once_with(
             self.volume.id,
-            connect_info,
-            self.server.id,
-            'null',
+            connector=connect_info,
+            instance=self.server.id,
+            mode='null',
         )
         self.assertEqual(self.columns, columns)
         self.assertCountEqual(self.data, data)
@@ -248,7 +245,7 @@ class TestVolumeAttachmentDelete(TestVolumeAttachment):
     def setUp(self):
         super().setUp()
 
-        self.volume_attachments_mock.delete.return_value = None
+        self.volume_sdk_client.delete_attachment.return_value = None
 
         self.cmd = volume_attachment.DeleteVolumeAttachment(self.app, None)
 
@@ -265,7 +262,7 @@ def test_volume_attachment_delete(self):
 
         result = self.cmd.take_action(parsed_args)
 
-        self.volume_attachments_mock.delete.assert_called_once_with(
+        self.volume_sdk_client.delete_attachment.assert_called_once_with(
             self.volume_attachment.id,
         )
         self.assertIsNone(result)
@@ -316,7 +313,7 @@ class TestVolumeAttachmentSet(TestVolumeAttachment):
     def setUp(self):
         super().setUp()
 
-        self.volume_attachments_mock.update.return_value = (
+        self.volume_sdk_client.update_attachment.return_value = (
             self.volume_attachment
         )
 
@@ -367,9 +364,9 @@ def test_volume_attachment_set(self):
             ]
         )
 
-        self.volume_attachments_mock.update.assert_called_once_with(
+        self.volume_sdk_client.update_attachment.assert_called_once_with(
             self.volume_attachment.id,
-            connect_info,
+            connector=connect_info,
         )
         self.assertEqual(self.columns, columns)
         self.assertCountEqual(self.data, data)
@@ -402,7 +399,7 @@ class TestVolumeAttachmentComplete(TestVolumeAttachment):
     def setUp(self):
         super().setUp()
 
-        self.volume_attachments_mock.complete.return_value = None
+        self.volume_sdk_client.complete_attachment.return_value = None
 
         self.cmd = volume_attachment.CompleteVolumeAttachment(self.app, None)
 
@@ -419,7 +416,7 @@ def test_volume_attachment_complete(self):
 
         result = self.cmd.take_action(parsed_args)
 
-        self.volume_attachments_mock.complete.assert_called_once_with(
+        self.volume_sdk_client.complete_attachment.assert_called_once_with(
             self.volume_attachment.id,
         )
         self.assertIsNone(result)
@@ -467,7 +464,7 @@ def setUp(self):
         super().setUp()
 
         self.projects_mock.get.return_value = self.project
-        self.volume_attachments_mock.list.return_value = (
+        self.volume_sdk_client.attachments.return_value = (
             self.volume_attachments
         )
 
@@ -489,7 +486,7 @@ def test_volume_attachment_list(self):
 
         columns, data = self.cmd.take_action(parsed_args)
 
-        self.volume_attachments_mock.list.assert_called_once_with(
+        self.volume_sdk_client.attachments.assert_called_once_with(
             search_opts={
                 'all_tenants': False,
                 'project_id': None,
@@ -529,7 +526,7 @@ def test_volume_attachment_list_with_options(self):
 
         columns, data = self.cmd.take_action(parsed_args)
 
-        self.volume_attachments_mock.list.assert_called_once_with(
+        self.volume_sdk_client.attachments.assert_called_once_with(
             search_opts={
                 'all_tenants': True,
                 'project_id': self.project.id,
diff --git a/openstackclient/volume/v3/volume_attachment.py b/openstackclient/volume/v3/volume_attachment.py
@@ -12,7 +12,7 @@
 
 import logging
 
-from cinderclient import api_versions
+from openstack import utils as sdk_utils
 from osc_lib.cli import format_columns
 from osc_lib.command import command
 from osc_lib import exceptions
@@ -170,18 +170,18 @@ def get_parser(self, prog_name):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
-        compute_client = self.app.client_manager.compute
+        volume_client = self.app.client_manager.sdk_connection.volume
+        compute_client = self.app.client_manager.sdk_connection.compute
 
-        if volume_client.api_version < api_versions.APIVersion('3.27'):
+        if not sdk_utils.supports_microversion(volume_client, '3.27'):
             msg = _(
                 "--os-volume-api-version 3.27 or greater is required to "
                 "support the 'volume attachment create' command"
             )
             raise exceptions.CommandError(msg)
 
         if parsed_args.mode:
-            if volume_client.api_version < api_versions.APIVersion('3.54'):
+            if not sdk_utils.supports_microversion(volume_client, '3.54'):
                 msg = _(
                     "--os-volume-api-version 3.54 or greater is required to "
                     "support the '--mode' option"
@@ -218,17 +218,18 @@ def take_action(self, parsed_args):
                 )
                 raise exceptions.CommandError(msg)
 
-        volume = utils.find_resource(
-            volume_client.volumes,
-            parsed_args.volume,
+        volume = volume_client.find_volume(
+            parsed_args.volume, ignore_missing=False
         )
-        server = utils.find_resource(
-            compute_client.servers,
-            parsed_args.server,
+        server = compute_client.find_server(
+            parsed_args.server, ignore_missing=False
         )
 
-        attachment = volume_client.attachments.create(
-            volume.id, connector, server.id, parsed_args.mode
+        attachment = volume_client.create_attachment(
+            volume.id,
+            connector=connector,
+            instance=server.id,
+            mode=parsed_args.mode,
         )
 
         return _format_attachment(attachment)
@@ -256,16 +257,16 @@ def get_parser(self, prog_name):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
 
-        if volume_client.api_version < api_versions.APIVersion('3.27'):
+        if not sdk_utils.supports_microversion(volume_client, '3.27'):
             msg = _(
                 "--os-volume-api-version 3.27 or greater is required to "
                 "support the 'volume attachment delete' command"
             )
             raise exceptions.CommandError(msg)
 
-        volume_client.attachments.delete(parsed_args.attachment)
+        volume_client.delete_attachment(parsed_args.attachment)
 
 
 class SetVolumeAttachment(command.ShowOne):
@@ -330,9 +331,9 @@ def get_parser(self, prog_name):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
 
-        if volume_client.api_version < api_versions.APIVersion('3.27'):
+        if not sdk_utils.supports_microversion(volume_client, '3.27'):
             msg = _(
                 "--os-volume-api-version 3.27 or greater is required to "
                 "support the 'volume attachment set' command"
@@ -349,8 +350,9 @@ def take_action(self, parsed_args):
             'mountpoint': parsed_args.mountpoint,
         }
 
-        attachment = volume_client.attachments.update(
-            parsed_args.attachment, connector
+        attachment = volume_client.update_attachment(
+            parsed_args.attachment,
+            connector=connector,
         )
 
         return _format_attachment(attachment)
@@ -369,16 +371,16 @@ def get_parser(self, prog_name):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
 
-        if volume_client.api_version < api_versions.APIVersion('3.44'):
+        if not sdk_utils.supports_microversion(volume_client, '3.44'):
             msg = _(
                 "--os-volume-api-version 3.44 or greater is required to "
                 "support the 'volume attachment complete' command"
             )
             raise exceptions.CommandError(msg)
 
-        volume_client.attachments.complete(parsed_args.attachment)
+        volume_client.complete_attachment(parsed_args.attachment)
 
 
 class ListVolumeAttachment(command.Lister):
@@ -429,10 +431,10 @@ def get_parser(self, prog_name):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
         identity_client = self.app.client_manager.identity
 
-        if volume_client.api_version < api_versions.APIVersion('3.27'):
+        if not sdk_utils.supports_microversion(volume_client, '3.27'):
             msg = _(
                 "--os-volume-api-version 3.27 or greater is required to "
                 "support the 'volume attachment list' command"
@@ -458,7 +460,7 @@ def take_action(self, parsed_args):
         #     search_opts.update(shell_utils.extract_filters(AppendFilters.filters))
 
         # TODO(stephenfin): Implement sorting
-        attachments = volume_client.attachments.list(
+        attachments = volume_client.attachments(
             search_opts=search_opts,
             marker=parsed_args.marker,
             limit=parsed_args.limit,
@@ -496,15 +498,15 @@ def get_parser(self, prog_name):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
 
-        if volume_client.api_version < api_versions.APIVersion('3.27'):
+        if not sdk_utils.supports_microversion(volume_client, '3.27'):
             msg = _(
                 "--os-volume-api-version 3.27 or greater is required to "
                 "support the 'volume attachment show' command"
             )
             raise exceptions.CommandError(msg)
 
-        attachment = volume_client.attachments.show(parsed_args.attachment)
+        attachment = volume_client.get_attachment(parsed_args.attachment)
 
         return _format_attachment(attachment)
diff --git a/releasenotes/notes/migrate-volume-attachment-commands-4309409bca1ca5d4.yaml b/releasenotes/notes/migrate-volume-attachment-commands-4309409bca1ca5d4.yaml
@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Migrated volume attachment commands to SDK.

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +features:
 +  - |
 +    Migrated volume attachment commands to SDK.