add/remove security groups to/from the servers as server actions

4 files changed, 296 insertions, 328 deletions

diff --git a/nova/api/openstack/contrib/security_groups.py b/nova/api/openstack/contrib/security_groups.py
index a104a42e4..1fd64f3b8 100644
--- a/nova/api/openstack/contrib/security_groups.py
+++ b/nova/api/openstack/contrib/security_groups.py
@@ -168,135 +168,6 @@ class SecurityGroupController(object):
                             "than 255 characters.") % typ
             raise exc.HTTPBadRequest(explanation=msg)
 
-    def associate(self, req, id, body):
-        context = req.environ['nova.context']
-
-        if not body:
-            raise exc.HTTPUnprocessableEntity()
-
-        if not 'security_group_associate' in body:
-            raise exc.HTTPUnprocessableEntity()
-
-        security_group = self._get_security_group(context, id)
-
-        servers = body['security_group_associate'].get('servers')
-
-        if not servers:
-            msg = _("No servers found")
-            return exc.HTTPBadRequest(explanation=msg)
-
-        hosts = set()
-        for server in servers:
-            if server['id']:
-                try:
-                    # check if the server exists
-                    inst = db.instance_get(context, server['id'])
-                    #check if the security group is assigned to the server
-                    if self._is_security_group_associated_to_server(
-                                                  security_group, inst['id']):
-                        msg = _("Security group %s is already associated with"
-                                " the instance %s") % (security_group['id'],
-                                                  server['id'])
-                        raise exc.HTTPBadRequest(explanation=msg)
-
-                    #check if the instance is in running state
-                    if inst['state'] != power_state.RUNNING:
-                        msg = _("Server %s is not in the running state")\
-                                            % server['id']
-                        raise exc.HTTPBadRequest(explanation=msg)
-
-                    hosts.add(inst['host'])
-                except exception.InstanceNotFound as exp:
-                    return exc.HTTPNotFound(explanation=unicode(exp))
-
-         # Associate security group with the server in the db
-        for server in servers:
-            if server['id']:
-                db.instance_add_security_group(context.elevated(),
-                                                    server['id'],
-                                                    security_group['id'])
-
-        for host in hosts:
-            rpc.cast(context,
-                     db.queue_get_for(context, FLAGS.compute_topic, host),
-                     {"method": "refresh_security_group_rules",
-                      "args": {"security_group_id": security_group['id']}})
-
-        return exc.HTTPAccepted()
-
-    def _is_security_group_associated_to_server(self, security_group,
-                                                instance_id):
-        if not security_group:
-            return False
-
-        instances = security_group.get('instances')
-        if not instances:
-            return False
-
-        inst_id = None
-        for inst_id in (instance['id'] for instance in instances \
-                        if instance_id == instance['id']):
-            return True
-
-        return False
-
-    def disassociate(self, req, id, body):
-        context = req.environ['nova.context']
-
-        if not body:
-            raise exc.HTTPUnprocessableEntity()
-
-        if not 'security_group_disassociate' in body:
-            raise exc.HTTPUnprocessableEntity()
-
-        security_group = self._get_security_group(context, id)
-
-        servers = body['security_group_disassociate'].get('servers')
-
-        if not servers:
-            msg = _("No servers found")
-            return exc.HTTPBadRequest(explanation=msg)
-
-        hosts = set()
-        for server in servers:
-            if server['id']:
-                try:
-                    # check if the instance exists
-                    inst = db.instance_get(context, server['id'])
-                    # Check if the security group is not associated
-                    # with the instance
-                    if not self._is_security_group_associated_to_server(
-                                                security_group, inst['id']):
-                        msg = _("Security group %s is not associated with the"
-                                "instance %s") % (security_group['id'],
-                                                  server['id'])
-                        raise exc.HTTPBadRequest(explanation=msg)
-
-                    #check if the instance is in running state
-                    if inst['state'] != power_state.RUNNING:
-                        msg = _("Server %s is not in the running state")\
-                                            % server['id']
-                        raise exp.HTTPBadRequest(explanation=msg)
-
-                    hosts.add(inst['host'])
-                except exception.InstanceNotFound as exp:
-                    return exc.HTTPNotFound(explanation=unicode(exp))
-
-        # Disassociate security group from the server
-        for server in servers:
-            if server['id']:
-                db.instance_remove_security_group(context.elevated(),
-                                                    server['id'],
-                                                    security_group['id'])
-
-        for host in hosts:
-            rpc.cast(context,
-                     db.queue_get_for(context, FLAGS.compute_topic, host),
-                     {"method": "refresh_security_group_rules",
-                      "args": {"security_group_id": security_group['id']}})
-
-        return exc.HTTPAccepted()
-
 
 class SecurityGroupRulesController(SecurityGroupController):
 
@@ -461,6 +332,11 @@ class SecurityGroupRulesController(SecurityGroupController):
 
 
 class Security_groups(extensions.ExtensionDescriptor):
+
+    def __init__(self):
+        self.compute_api = compute.API()
+        super(Security_groups, self).__init__()
+
     def get_name(self):
         return "SecurityGroups"
 
@@ -476,6 +352,82 @@ class Security_groups(extensions.ExtensionDescriptor):
     def get_updated(self):
         return "2011-07-21T00:00:00+00:00"
 
+    def _addSecurityGroup(self, input_dict, req, instance_id):
+        context = req.environ['nova.context']
+
+        try:
+            body = input_dict['addSecurityGroup']
+            group_name = body['name']
+            instance_id = int(instance_id)
+        except ValueError:
+            msg = _("Server id should be integer")
+            raise exc.HTTPBadRequest(explanation=msg)
+        except TypeError:
+            msg = _("Missing parameter dict")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+        except KeyError:
+            msg = _("Security group not specified")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        if not group_name or group_name.strip() == '':
+            msg = _("Security group name cannot be empty")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        try:
+            self.compute_api.add_security_group(context, instance_id,
+                                                group_name)
+        except exception.SecurityGroupNotFound as exp:
+            return exc.HTTPNotFound(explanation=unicode(exp))
+        except exception.InstanceNotFound as exp:
+            return exc.HTTPNotFound(explanation=unicode(exp))
+        except exception.Invalid as exp:
+            return exc.HTTPBadRequest(explanation=unicode(exp))
+
+        return exc.HTTPAccepted()
+
+    def _removeSecurityGroup(self, input_dict, req, instance_id):
+        context = req.environ['nova.context']
+
+        try:
+            body = input_dict['removeSecurityGroup']
+            group_name = body['name']
+            instance_id = int(instance_id)
+        except ValueError:
+            msg = _("Server id should be integer")
+            raise exc.HTTPBadRequest(explanation=msg)
+        except TypeError:
+            msg = _("Missing parameter dict")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+        except KeyError:
+            msg = _("Security group not specified")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        if not group_name or group_name.strip() == '':
+            msg = _("Security group name cannot be empty")
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
+        try:
+            self.compute_api.remove_security_group(context, instance_id,
+                                                   group_name)
+        except exception.SecurityGroupNotFound as exp:
+            return exc.HTTPNotFound(explanation=unicode(exp))
+        except exception.InstanceNotFound as exp:
+            return exc.HTTPNotFound(explanation=unicode(exp))
+        except exception.Invalid as exp:
+            return exc.HTTPBadRequest(explanation=unicode(exp))
+
+        return exc.HTTPAccepted()
+
+    def get_actions(self):
+        """Return the actions the extensions adds"""
+        actions = [
+                extensions.ActionExtension("servers", "addSecurityGroup",
+                                           self._addSecurityGroup),
+                extensions.ActionExtension("servers", "removeSecurityGroup",
+                                           self._removeSecurityGroup)
+                   ]
+        return actions
+
     def get_resources(self):
         resources = []
 
@@ -493,10 +445,6 @@ class Security_groups(extensions.ExtensionDescriptor):
 
         res = extensions.ResourceExtension('os-security-groups',
                                 controller=SecurityGroupController(),
-                                member_actions={
-                                   'associate': 'POST',
-                                   'disassociate': 'POST'
-                                },
                                 deserializer=deserializer,
                                 serializer=serializer)
 
@@ -534,40 +482,6 @@ class SecurityGroupXMLDeserializer(wsgi.MetadataXMLDeserializer):
                 security_group['description'] = self.extract_text(desc_node)
         return {'body': {'security_group': security_group}}
 
-    def _get_servers(self, node):
-        servers_dict = {'servers': []}
-        if node is not None:
-            servers_node = self.find_first_child_named(node,
-                                           'servers')
-            if servers_node is not None:
-                for server_node in self.find_children_named(servers_node,
-                                                          "server"):
-                    servers_dict['servers'].append(
-                                {"id": self.extract_text(server_node)})
-        return servers_dict
-
-    def associate(self, string):
-        """Deserialize an xml-formatted security group associate request"""
-        dom = minidom.parseString(string)
-        node = self.find_first_child_named(dom,
-                                           'security_group_associate')
-        result = {'body': {}}
-        if node:
-            result['body']['security_group_associate'] = \
-                                                self._get_servers(node)
-        return result
-
-    def disassociate(self, string):
-        """Deserialize an xml-formatted security group disassociate request"""
-        dom = minidom.parseString(string)
-        node = self.find_first_child_named(dom,
-                                           'security_group_disassociate')
-        result = {'body': {}}
-        if node:
-            result['body']['security_group_disassociate'] = \
-                                                self._get_servers(node)
-        return result
-
 
 class SecurityGroupRulesXMLDeserializer(wsgi.MetadataXMLDeserializer):
     """
diff --git a/nova/compute/api.py b/nova/compute/api.py
index efc9da79b..0c6beacaa 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -613,6 +613,78 @@ class API(base.Base):
                      self.db.queue_get_for(context, FLAGS.compute_topic, host),
                      {'method': 'refresh_provider_fw_rules', 'args': {}})
 
+    def _is_security_group_associated_with_server(self, security_group,
+                                                instance_id):
+        """Check if the security group is already associated
+           with the instance. If Yes, return True.
+        """
+
+        if not security_group:
+            return False
+
+        instances = security_group.get('instances')
+        if not instances:
+            return False
+
+        inst_id = None
+        for inst_id in (instance['id'] for instance in instances \
+                        if instance_id == instance['id']):
+            return True
+
+        return False
+
+    def add_security_group(self, context, instance_id, security_group_name):
+        """Add security group to the instance"""
+        security_group = db.security_group_get_by_name(context,
+                                                       context.project_id,
+                                                       security_group_name)
+        # check if the server exists
+        inst = db.instance_get(context, instance_id)
+        #check if the security group is associated with the server
+        if self._is_security_group_associated_with_server(security_group,
+                                                        instance_id):
+            raise exception.SecurityGroupExistsForInstance(
+                                        security_group_id=security_group['id'],
+                                        instance_id=instance_id)
+
+        #check if the instance is in running state
+        if inst['state'] != power_state.RUNNING:
+            raise exception.InstanceNotRunning(instance_id=instance_id)
+
+        db.instance_add_security_group(context.elevated(),
+                                       instance_id,
+                                       security_group['id'])
+        rpc.cast(context,
+             db.queue_get_for(context, FLAGS.compute_topic, inst['host']),
+             {"method": "refresh_security_group_rules",
+              "args": {"security_group_id": security_group['id']}})
+
+    def remove_security_group(self, context, instance_id, security_group_name):
+        """Remove the security group associated with the instance"""
+        security_group = db.security_group_get_by_name(context,
+                                                       context.project_id,
+                                                       security_group_name)
+        # check if the server exists
+        inst = db.instance_get(context, instance_id)
+        #check if the security group is associated with the server
+        if not self._is_security_group_associated_with_server(security_group,
+                                                        instance_id):
+            raise exception.SecurityGroupNotExistsForInstance(
+                                    security_group_id=security_group['id'],
+                                    instance_id=instance_id)
+
+        #check if the instance is in running state
+        if inst['state'] != power_state.RUNNING:
+            raise exception.InstanceNotRunning(instance_id=instance_id)
+
+        db.instance_remove_security_group(context.elevated(),
+                                       instance_id,
+                                       security_group['id'])
+        rpc.cast(context,
+             db.queue_get_for(context, FLAGS.compute_topic, inst['host']),
+             {"method": "refresh_security_group_rules",
+              "args": {"security_group_id": security_group['id']}})
+
     @scheduler_api.reroute_compute("update")
     def update(self, context, instance_id, **kwargs):
         """Updates the instance in the datastore.
diff --git a/nova/exception.py b/nova/exception.py
index b09d50797..e8cb7bcb5 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -541,6 +541,16 @@ class SecurityGroupNotFoundForRule(SecurityGroupNotFound):
     message = _("Security group with rule %(rule_id)s not found.")
 
 
+class SecurityGroupExistsForInstance(Invalid):
+    message = _("Security group %(security_group_id)s is already associated"
+                 " with the instance %(instance_id)s")
+
+
+class SecurityGroupNotExistsForInstance(Invalid):
+    message = _("Security group %(security_group_id)s is not associated with"
+                 " the instance %(instance_id)s")
+
+
 class MigrationNotFound(NotFound):
     message = _("Migration %(migration_id)s could not be found.")
 
diff --git a/nova/tests/api/openstack/contrib/test_security_groups.py b/nova/tests/api/openstack/contrib/test_security_groups.py
index 894b0c591..b44ebc9fb 100644
--- a/nova/tests/api/openstack/contrib/test_security_groups.py
+++ b/nova/tests/api/openstack/contrib/test_security_groups.py
@@ -63,17 +63,17 @@ def return_non_running_server(context, server_id):
                         'host': "localhost"}
 
 
-def return_security_group(context, group_id):
-    return {'id': group_id, "instances": [
+def return_security_group(context, project_id, group_name):
+    return {'id': 1, 'name': group_name, "instances": [
               {'id': 1}]}
 
 
-def return_security_group_without_instances(context, group_id):
-    return {'id': group_id}
+def return_security_group_without_instances(context, project_id, group_name):
+    return {'id': 1, 'name': group_name}
 
 
 def return_server_nonexistant(context, server_id):
-    raise exception.InstanceNotFound()
+    raise exception.InstanceNotFound(instance_id=server_id)
 
 
 class TestSecurityGroups(test.TestCase):
@@ -350,117 +350,89 @@ class TestSecurityGroups(test.TestCase):
         response = self._delete_security_group(11111111)
         self.assertEquals(response.status_int, 404)
 
-    def test_associate_by_non_existing_security_group_id(self):
-        req = webob.Request.blank('/v1.1/os-security-groups/111111/associate')
+    def test_associate_by_non_existing_security_group_name(self):
+        body = dict(addSecurityGroup=dict(name='non-existing'))
+        req = webob.Request.blank('/v1.1/servers/1/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_associate": {
-                         "servers": [
-                                     {"id": '2'}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 404)
 
-    def test_associate_by_invalid_security_group_id(self):
-        req = webob.Request.blank('/v1.1/os-security-groups/invalid/associate')
+    def test_associate_by_invalid_server_id(self):
+        body = dict(addSecurityGroup=dict(name='test'))
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group)
+        req = webob.Request.blank('/v1.1/servers/invalid/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_associate": {
-                         "servers": [
-                                     {"id": "2"}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 400)
 
     def test_associate_without_body(self):
-        req = webob.Request.blank('/v1.1/os-security-groups/1/associate')
+        req = webob.Request.blank('/v1.1/servers/1/action')
+        body = dict(addSecurityGroup=None)
+        self.stubs.Set(nova.db, 'instance_get', return_server)
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        req.body = json.dumps(None)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
-        self.assertEquals(response.status_int, 422)
+        self.assertEquals(response.status_int, 400)
 
-    def test_associate_no_security_group_element(self):
-        req = webob.Request.blank('/v1.1/os-security-groups/1/associate')
+    def test_associate_no_security_group_name(self):
+        req = webob.Request.blank('/v1.1/servers/1/action')
+        body = dict(addSecurityGroup=dict())
+        self.stubs.Set(nova.db, 'instance_get', return_server)
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_associate_invalid": {
-                         "servers": [
-                                     {"id": "2"}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
-        self.assertEquals(response.status_int, 422)
+        self.assertEquals(response.status_int, 400)
 
-    def test_associate_no_instances(self):
-        #self.stubs.Set(nova.db.api, 'instance_get', return_server)
-        self.stubs.Set(nova.db, 'security_group_get', return_security_group)
-        req = webob.Request.blank('/v1.1/os-security-groups/1/associate')
+    def test_associate_security_group_name_with_whitespaces(self):
+        req = webob.Request.blank('/v1.1/servers/1/action')
+        body = dict(addSecurityGroup=dict(name="   "))
+        self.stubs.Set(nova.db, 'instance_get', return_server)
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_associate": {
-                         "servers": [
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 400)
 
     def test_associate_non_existing_instance(self):
         self.stubs.Set(nova.db, 'instance_get', return_server_nonexistant)
-        self.stubs.Set(nova.db, 'security_group_get', return_security_group)
-        req = webob.Request.blank('/v1.1/os-security-groups/1/associate')
+        body = dict(addSecurityGroup=dict(name="test"))
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group)
+        req = webob.Request.blank('/v1.1/servers/10000/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_associate": {
-                         "servers": [
-                                 {'id': 2}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 404)
 
     def test_associate_non_running_instance(self):
         self.stubs.Set(nova.db, 'instance_get', return_non_running_server)
-        self.stubs.Set(nova.db, 'security_group_get',
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
                        return_security_group_without_instances)
-        req = webob.Request.blank('/v1.1/os-security-groups/1/associate')
+        body = dict(addSecurityGroup=dict(name="test"))
+        req = webob.Request.blank('/v1.1/servers/1/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_associate": {
-                         "servers": [
-                                 {'id': 1}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 400)
 
     def test_associate_already_associated_security_group_to_instance(self):
         self.stubs.Set(nova.db, 'instance_get', return_server)
-        self.stubs.Set(nova.db, 'security_group_get', return_security_group)
-        req = webob.Request.blank('/v1.1/os-security-groups/1/associate')
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group)
+        body = dict(addSecurityGroup=dict(name="test"))
+        req = webob.Request.blank('/v1.1/servers/1/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_associate": {
-                         "servers": [
-                                 {'id': 1}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 400)
 
@@ -470,134 +442,120 @@ class TestSecurityGroups(test.TestCase):
         nova.db.instance_add_security_group(mox.IgnoreArg(),
                                     mox.IgnoreArg(),
                                     mox.IgnoreArg())
-        self.stubs.Set(nova.db, 'security_group_get',
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
                        return_security_group_without_instances)
         self.mox.ReplayAll()
 
-        req = webob.Request.blank('/v1.1/os-security-groups/1/associate')
+        body = dict(addSecurityGroup=dict(name="test"))
+        req = webob.Request.blank('/v1.1/servers/1/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_associate": {
-                         "servers": [
-                                 {'id': 1}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
+        response = req.get_response(fakes.wsgi_app())
+        self.assertEquals(response.status_int, 202)
+
+    def test_associate_xml(self):
+        self.stubs.Set(nova.db, 'instance_get', return_server)
+        self.mox.StubOutWithMock(nova.db, 'instance_add_security_group')
+        nova.db.instance_add_security_group(mox.IgnoreArg(),
+                                    mox.IgnoreArg(),
+                                    mox.IgnoreArg())
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group_without_instances)
+        self.mox.ReplayAll()
+
+        req = webob.Request.blank('/v1.1/servers/1/action')
+        req.headers['Content-Type'] = 'application/xml'
+        req.method = 'POST'
+        req.body = """<addSecurityGroup>
+                           <name>test</name>
+                    </addSecurityGroup>"""
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 202)
 
-    def test_disassociate_by_non_existing_security_group_id(self):
-        req = webob.Request.blank('/v1.1/os-security-groups/1111/disassociate')
+    def test_disassociate_by_non_existing_security_group_name(self):
+        body = dict(removeSecurityGroup=dict(name='non-existing'))
+        req = webob.Request.blank('/v1.1/servers/1/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_disassociate": {
-                         "servers": [
-                                     {"id": "2"}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 404)
 
-    def test_disassociate_by_invalid_security_group_id(self):
-        req = webob.Request.blank('/v1.1/os-security-groups/id/disassociate')
+    def test_disassociate_by_invalid_server_id(self):
+        body = dict(removeSecurityGroup=dict(name='test'))
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group)
+        req = webob.Request.blank('/v1.1/servers/invalid/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_disassociate": {
-                         "servers": [
-                                     {"id": "2"}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 400)
 
     def test_disassociate_without_body(self):
-        req = webob.Request.blank('/v1.1/os-security-groups/1/disassociate')
+        req = webob.Request.blank('/v1.1/servers/1/action')
+        body = dict(removeSecurityGroup=None)
+        self.stubs.Set(nova.db, 'instance_get', return_server)
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        req.body = json.dumps(None)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
-        self.assertEquals(response.status_int, 422)
+        self.assertEquals(response.status_int, 400)
 
-    def test_disassociate_no_security_group_element(self):
-        req = webob.Request.blank('/v1.1/os-security-groups/1/disassociate')
+    def test_disassociate_no_security_group_name(self):
+        req = webob.Request.blank('/v1.1/servers/1/action')
+        body = dict(removeSecurityGroup=dict())
+        self.stubs.Set(nova.db, 'instance_get', return_server)
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_disassociate_invalid": {
-                         "servers": [
-                                     {"id": "2"}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
-        self.assertEquals(response.status_int, 422)
+        self.assertEquals(response.status_int, 400)
 
-    def test_disassociate_no_instances(self):
-        #self.stubs.Set(nova.db.api, 'instance_get', return_server)
-        self.stubs.Set(nova.db, 'security_group_get', return_security_group)
-        req = webob.Request.blank('/v1.1/os-security-groups/1/disassociate')
+    def test_disassociate_security_group_name_with_whitespaces(self):
+        req = webob.Request.blank('/v1.1/servers/1/action')
+        body = dict(removeSecurityGroup=dict(name="   "))
+        self.stubs.Set(nova.db, 'instance_get', return_server)
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_disassociate": {
-                         "servers": [
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 400)
 
     def test_disassociate_non_existing_instance(self):
         self.stubs.Set(nova.db, 'instance_get', return_server_nonexistant)
-        self.stubs.Set(nova.db, 'security_group_get', return_security_group)
-        req = webob.Request.blank('/v1.1/os-security-groups/1/disassociate')
+        body = dict(removeSecurityGroup=dict(name="test"))
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group)
+        req = webob.Request.blank('/v1.1/servers/10000/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_disassociate": {
-                         "servers": [
-                                 {'id': 2}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 404)
 
     def test_disassociate_non_running_instance(self):
         self.stubs.Set(nova.db, 'instance_get', return_non_running_server)
-        self.stubs.Set(nova.db, 'security_group_get',
-                       return_security_group_without_instances)
-        req = webob.Request.blank('/v1.1/os-security-groups/1/disassociate')
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group)
+        body = dict(removeSecurityGroup=dict(name="test"))
+        req = webob.Request.blank('/v1.1/servers/1/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_disassociate": {
-                         "servers": [
-                                 {'id': 1}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 400)
 
-    def test_disassociate_not_associated_security_group_to_instance(self):
+    def test_disassociate_already_associated_security_group_to_instance(self):
         self.stubs.Set(nova.db, 'instance_get', return_server)
-        self.stubs.Set(nova.db, 'security_group_get', return_security_group)
-        req = webob.Request.blank('/v1.1/os-security-groups/1/disassociate')
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group_without_instances)
+        body = dict(removeSecurityGroup=dict(name="test"))
+        req = webob.Request.blank('/v1.1/servers/1/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_disassociate": {
-                         "servers": [
-                                 {'id': 2}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 400)
 
@@ -607,20 +565,34 @@ class TestSecurityGroups(test.TestCase):
         nova.db.instance_remove_security_group(mox.IgnoreArg(),
                                     mox.IgnoreArg(),
                                     mox.IgnoreArg())
-        self.stubs.Set(nova.db, 'security_group_get',
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
                        return_security_group)
         self.mox.ReplayAll()
 
-        req = webob.Request.blank('/v1.1/os-security-groups/1/disassociate')
+        body = dict(removeSecurityGroup=dict(name="test"))
+        req = webob.Request.blank('/v1.1/servers/1/action')
         req.headers['Content-Type'] = 'application/json'
         req.method = 'POST'
-        body_dict = {"security_group_disassociate": {
-                         "servers": [
-                                 {'id': 1}
-                         ]
-                     }
-        }
-        req.body = json.dumps(body_dict)
+        req.body = json.dumps(body)
+        response = req.get_response(fakes.wsgi_app())
+        self.assertEquals(response.status_int, 202)
+
+    def test_disassociate_xml(self):
+        self.stubs.Set(nova.db, 'instance_get', return_server)
+        self.mox.StubOutWithMock(nova.db, 'instance_remove_security_group')
+        nova.db.instance_remove_security_group(mox.IgnoreArg(),
+                                    mox.IgnoreArg(),
+                                    mox.IgnoreArg())
+        self.stubs.Set(nova.db, 'security_group_get_by_name',
+                       return_security_group)
+        self.mox.ReplayAll()
+
+        req = webob.Request.blank('/v1.1/servers/1/action')
+        req.headers['Content-Type'] = 'application/xml'
+        req.method = 'POST'
+        req.body = """<removeSecurityGroup>
+                           <name>test</name>
+                    </removeSecurityGroup>"""
         response = req.get_response(fakes.wsgi_app())
         self.assertEquals(response.status_int, 202)