Fix Instance secgroup update error with Nova secgroup

Nova add/remove_security_group takes secgroup name instead of id.
Add api test for update_instance_security_group in api.nova.

Change the parameter name "new_sgs" of server_update_security_groups
to "new_security_group_ids" to clarify it takes ID as a parameter.

Based on the initial patch in https://review.openstack.org/#/c/39940

Change-Id: I8d9b6f5c22eee5adbaea51ce352483ab74f488f6
Closes-Bug: #1207184

openstack_dashboard/api/network.py

@@ -124,9 +124,10 @@ def server_security_groups(request, instance_id):
     return NetworkClient(request).secgroups.list_by_instance(instance_id)
 
 
-def server_update_security_groups(request, instance_id, new_sgs):
+def server_update_security_groups(request, instance_id,
+                                  new_security_group_ids):
     return NetworkClient(request).secgroups.update_instance_security_group(
-        instance_id, new_sgs)
+        instance_id, new_security_group_ids)
 
 
 def security_group_backend(request):

openstack_dashboard/api/network_base.py

@@ -211,6 +211,7 @@ def list_by_instance(self, instance_id):
         pass
 
     @abc.abstractmethod
-    def update_instance_security_group(self, instance_id, new_sgs):
+    def update_instance_security_group(self, instance_id,
+                                       new_security_group_ids):
         """Update security groups of a specified instance."""
         pass

openstack_dashboard/api/neutron.py

@@ -283,10 +283,11 @@ def list_by_instance(self, instance_id):
             sg_ids += p.security_groups
         return self._list(id=set(sg_ids))
 
-    def update_instance_security_group(self, instance_id, new_sgs):
+    def update_instance_security_group(self, instance_id,
+                                       new_security_group_ids):
         ports = port_list(self.request, device_id=instance_id)
         for p in ports:
-            params = {'security_groups': new_sgs}
+            params = {'security_groups': new_security_group_ids}
             port_modify(self.request, p.id, **params)
 
 

openstack_dashboard/api/nova.py

@@ -241,17 +241,23 @@ def list_by_instance(self, instance_id):
             security_groups = [SecurityGroup(sg) for sg in sg_objs]
         return security_groups
 
-    def update_instance_security_group(self, instance_id, new_sgs):
+    def update_instance_security_group(self, instance_id,
+                                       new_security_group_ids):
+        try:
+            all_groups = self.list()
+        except Exception:
+            raise Exception(_("Couldn't get security group list."))
+        wanted_groups = set([sg.name for sg in all_groups
+                             if sg.id in new_security_group_ids])
 
-        wanted_groups = set(new_sgs)
         try:
             current_groups = self.list_by_instance(instance_id)
         except Exception:
             raise Exception(_("Couldn't get current security group "
                               "list for instance %s.")
                             % instance_id)
+        current_group_names = set([sg.name for sg in current_groups])
 
-        current_group_names = set(map(lambda g: g.id, current_groups))
         groups_to_add = wanted_groups - current_group_names
         groups_to_remove = current_group_names - wanted_groups
 

openstack_dashboard/test/api_tests/network_tests.py

@@ -35,6 +35,33 @@ def setUp(self):
             .AndReturn(False)
 
 
+class NetworkApiNovaSecurityGroupTests(NetworkApiNovaTestBase):
+    def test_server_update_security_groups(self):
+        all_secgroups = self.security_groups.list()
+        added_secgroup = all_secgroups[2]
+        rm_secgroup = all_secgroups[0]
+        cur_secgroups_raw = [{'id': sg.id, 'name': sg.name,
+                              'rules': []}
+                             for sg in all_secgroups[0:2]]
+        cur_secgroups_ret = {'security_groups': cur_secgroups_raw}
+        new_sg_ids = [sg.id for sg in all_secgroups[1:3]]
+        instance_id = self.servers.first().id
+
+        novaclient = self.stub_novaclient()
+        novaclient.security_groups = self.mox.CreateMockAnything()
+        novaclient.servers = self.mox.CreateMockAnything()
+        novaclient.client = self.mox.CreateMockAnything()
+        novaclient.security_groups.list().AndReturn(all_secgroups)
+        url = '/servers/%s/os-security-groups' % instance_id
+        novaclient.client.get(url).AndReturn((200, cur_secgroups_ret))
+        novaclient.servers.add_security_group(instance_id, added_secgroup.name)
+        novaclient.servers.remove_security_group(instance_id, rm_secgroup.name)
+        self.mox.ReplayAll()
+
+        api.network.server_update_security_groups(
+            self.request, instance_id, new_sg_ids)
+
+
 class NetworkApiNovaFloatingIpTests(NetworkApiNovaTestBase):
     def test_floating_ip_pools_list(self):
         pool_names = ['pool1', 'pool2']