Switch command server remove volume to sdk

Change-Id: If6f6cf93b55a67e767c54de8ce21f25252cf99ca

2 files changed, 59 insertions, 13 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 18c1197c..121a7b82 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -3793,22 +3793,29 @@ class RemoveServerVolume(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
-        volume_client = self.app.client_manager.volume
+        compute_client = self.app.client_manager.sdk_connection.compute
+        volume_client = self.app.client_manager.sdk_connection.volume
 
-        server = utils.find_resource(
-            compute_client.servers,
+        server = compute_client.find_server(
             parsed_args.server,
+            ignore_missing=False,
         )
-        volume = utils.find_resource(
-            volume_client.volumes,
+        volume = volume_client.find_volume(
             parsed_args.volume,
+            ignore_missing=False,
         )
 
-        compute_client.volumes.delete_server_volume(
-            server.id,
-            volume.id,
-        )
+        volume_attachments = compute_client.volume_attachments(server)
+        for volume_attachment in volume_attachments:
+            if volume_attachment.volume_id == volume.id:
+                compute_client.delete_volume_attachment(
+                    volume_attachment,
+                    server,
+                )
+                break
+        else:
+            msg = _('Target volume attachment not found.')
+            raise exceptions.CommandError(msg)
 
 
 class RescueServer(command.Command):
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 203e47eb..10ea07ad 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -692,9 +692,6 @@ class TestServerVolume(TestServer):
             'create_volume_attachment': None,
         }
 
-        # Get the command object to test
-        self.cmd = server.AddServerVolume(self.app, None)
-
         self.servers = self.setup_sdk_servers_mock(count=1)
         self.volumes = self.setup_sdk_volumes_mock(count=1)
 
@@ -709,6 +706,15 @@ class TestServerVolume(TestServer):
         self.sdk_client.create_volume_attachment.return_value = \
             self.volume_attachment
 
+
+class TestServerAddVolume(TestServerVolume):
+
+    def setUp(self):
+        super(TestServerAddVolume, self).setUp()
+
+        # Get the command object to test
+        self.cmd = server.AddServerVolume(self.app, None)
+
     @mock.patch.object(sdk_utils, 'supports_microversion', return_value=False)
     def test_server_add_volume(self, sm_mock):
 
@@ -985,6 +991,39 @@ class TestServerVolume(TestServer):
                       'with argument --enable-delete-on-termination', str(ex))
 
 
+class TestServerRemoveVolume(TestServerVolume):
+
+    def setUp(self):
+        super(TestServerRemoveVolume, self).setUp()
+
+        # Get the command object to test
+        self.cmd = server.RemoveServerVolume(self.app, None)
+
+    def test_server_remove_volume(self):
+        self.sdk_client.volume_attachments.return_value = [
+            self.volume_attachment
+        ]
+
+        arglist = [
+            self.servers[0].id,
+            self.volumes[0].id,
+        ]
+
+        verifylist = [
+            ('server', self.servers[0].id),
+            ('volume', self.volumes[0].id),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.assertIsNone(result)
+        self.sdk_client.delete_volume_attachment.assert_called_once_with(
+            self.volume_attachment,
+            self.servers[0])
+
+
 class TestServerAddNetwork(TestServer):
 
     def setUp(self):