From 8405db900ff25e21015351f74fa5c61a6a247a1e Mon Sep 17 00:00:00 2001
From: Rajasi Kulkarni <rajasikulkarni18@gmail.com>
Date: Sun, 3 Jul 2016 15:05:19 +0530
Subject: [PATCH] Pass security group id to novaclient

In RemoveServerSecurityGroup we currently pass the entire security
group object, which results in TypeError in novaclient.
Added unit test case to test command 'openstack server remove
security group -h <server> <group>'

Change-Id: I6d486403a83804c3a30d6f89d2cf7f64f09797c6
Closes-Bug: 1590883
---
 openstackclient/compute/v2/server.py          |  2 +-
 .../tests/compute/v2/test_server.py           | 53 +++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 7d04ffc3a0..a317c11d11 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -1222,7 +1222,7 @@ class RemoveServerSecurityGroup(command.Command):
             parsed_args.group,
         )
 
-        server.remove_security_group(security_group)
+        server.remove_security_group(security_group.id)
 
 
 class RemoveServerVolume(command.Command):
diff --git a/openstackclient/tests/compute/v2/test_server.py b/openstackclient/tests/compute/v2/test_server.py
index 24f92e4676..1c5a5fe462 100644
--- a/openstackclient/tests/compute/v2/test_server.py
+++ b/openstackclient/tests/compute/v2/test_server.py
@@ -43,6 +43,11 @@ class TestServer(compute_fakes.TestComputev2):
         self.flavors_mock = self.app.client_manager.compute.flavors
         self.flavors_mock.reset_mock()
 
+        # Get a shortcut to the compute client SecurityGroupManager Mock
+        self.security_groups_mock = \
+            self.app.client_manager.compute.security_groups
+        self.security_groups_mock.reset_mock()
+
         # Get a shortcut to the image client ImageManager Mock
         self.images_mock = self.app.client_manager.image.images
         self.images_mock.reset_mock()
@@ -981,6 +986,54 @@ class TestServerRemoveFloatingIP(TestServer):
         self.assertIsNone(result)
 
 
+class TestServerRemoveSecurityGroup(TestServer):
+
+    def setUp(self):
+        super(TestServerRemoveSecurityGroup, self).setUp()
+
+        self.security_group = \
+            compute_fakes.FakeSecurityGroup.create_one_security_group()
+        # This is the return value for utils.find_resource() for security group
+        self.security_groups_mock.get.return_value = self.security_group
+
+        attrs = {
+            'security_groups': [{'name': self.security_group.id}]
+        }
+        methods = {
+            'remove_security_group': None,
+        }
+
+        self.server = compute_fakes.FakeServer.create_one_server(
+            attrs=attrs,
+            methods=methods
+        )
+        # This is the return value for utils.find_resource() for server
+        self.servers_mock.get.return_value = self.server
+
+        # Get the command object to test
+        self.cmd = server.RemoveServerSecurityGroup(self.app, None)
+
+    def test_server_remove_security_group(self):
+        arglist = [
+            self.server.id,
+            self.security_group.id
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('group', self.security_group.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+        self.security_groups_mock.get.assert_called_with(
+            self.security_group.id,
+        )
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.server.remove_security_group.assert_called_with(
+            self.security_group.id,
+        )
+        self.assertIsNone(result)
+
+
 class TestServerResize(TestServer):
 
     def setUp(self):