Merge "Detach interface for server"

api-ref/source/v1/parameters.yaml

@@ -27,6 +27,12 @@ flavor_uuid_path:
   in: path
   required: true
   type: string
+port_ident:
+  description: |
+    The UUID of a network port.
+  in: path
+  required: true
+  type: string
 server_ident:
   description: |
     The UUID of the server.

api-ref/source/v1/server_networks.inc

@@ -126,3 +126,29 @@ Response
 --------
 
 If successful, this method does not return content in the response body.
+
+
+Detach a network interface.
+=================================
+
+.. rest_method:: DELETE /v1/servers/{server_uuid}/networks/interfaces/{port_id}
+
+Detach a network interface from a server.
+
+Normal response codes: 204
+
+Error response codes: badRequest(400), unauthorized(401), forbidden(403),
+itemNotFound(404), conflict(409)
+
+Request
+-------
+
+.. rest_parameters:: parameters.yaml
+
+    - server_uuid: server_ident
+    - port_id: port_ident
+
+Response
+--------
+
+If successful, this method does not return content in the response body.

mogan/api/controllers/v1/servers.py

@@ -201,7 +201,7 @@ class FloatingIPController(ServerControllerBase):
         super(FloatingIPController, self).__init__(*args, **kwargs)
         self.network_api = network.API()
 
-    @policy.authorize_wsgi("mogan:server", "associate_floatingip", False)
+    @policy.authorize_wsgi("mogan:server", "associate_floatingip")
     @expose.expose(None, types.uuid, body=types.jsontype,
                    status_code=http_client.NO_CONTENT)
     def post(self, server_uuid, floatingip):
@@ -354,7 +354,7 @@ class InterfaceController(ServerControllerBase):
     def __init__(self, *args, **kwargs):
         super(InterfaceController, self).__init__(*args, **kwargs)
 
-    @policy.authorize_wsgi("mogan:server", "attach_interface", False)
+    @policy.authorize_wsgi("mogan:server", "attach_interface")
     @expose.expose(None, types.uuid, body=types.jsontype,
                    status_code=http_client.NO_CONTENT)
     def post(self, server_uuid, interface):
@@ -385,6 +385,23 @@ class InterfaceController(ServerControllerBase):
             raise wsme.exc.ClientSideError(
                 six.text_type(e), status_code=http_client.CONFLICT)
 
+    @policy.authorize_wsgi("mogan:server", "detach_interface")
+    @expose.expose(None, types.uuid, types.uuid,
+                   status_code=http_client.NO_CONTENT)
+    def delete(self, server_uuid, port_id):
+        """Detach Interface
+
+        :param server_uuid: UUID of a server.
+        :param port_id: The Port ID within the request body.
+        """
+        server = self._resource or self._get_resource(server_uuid)
+        server_nics = server.nics
+        if port_id not in [nic.port_id for nic in server_nics]:
+            raise exception.InterfaceNotFoundForServer(server=server_uuid)
+
+        pecan.request.engine_api.detach_interface(pecan.request.context,
+                                                  server, port_id)
+
 
 class ServerNetworks(base.APIBase):
     """API representation of the networks of a server."""

mogan/common/exception.py

@@ -310,6 +310,18 @@ class InterfaceAttachFailed(Invalid):
                 "%(server_uuid)s")
 
 
+class InterfaceNotFoundForServer(NotFound):
+    _msg_fmt = _("Interface not found for server %(server)s.")
+
+
+class InterfaceNotAttached(Invalid):
+    _msg_fmt = _("Interface is not attached.")
+
+
+class InterfaceDetachFailed(Invalid):
+    _msg_fmt = _("Failed to detach network for %(server_uuid)s")
+
+
 class FloatingIpNotFoundForAddress(NotFound):
     _msg_fmt = _("Floating IP not found for address %(address)s.")
 

mogan/common/policy.py

@@ -144,6 +144,9 @@ server_policies = [
     policy.RuleDefault('mogan:node:get_all',
                        'rule:admin_api',
                        description='Retrieve all compute nodes'),
+    policy.RuleDefault('mogan:server:detach_interface',
+                       'rule:default',
+                       description='Detach a network interface'),
 ]
 
 

mogan/engine/api.py

@@ -516,3 +516,8 @@ class API(object):
     @check_server_lock
     def attach_interface(self, context, server, net_id):
         self.engine_rpcapi.attach_interface(context, server, net_id)
+
+    @check_server_lock
+    def detach_interface(self, context, server, port_id):
+        self.engine_rpcapi.detach_interface(context, server=server,
+                                            port_id=port_id)

mogan/engine/baremetal/driver.py

@@ -84,8 +84,8 @@ class BaseEngineDriver(object):
         """
         raise NotImplementedError()
 
-    def unplug_vifs(self, context, server):
+    def unplug_vif(self, context, server, port_id):
-        """Unplug network interfaces.
+        """Unplug network interface.
 
         :param server: the server object.
         """

mogan/engine/baremetal/ironic/driver.py

@@ -261,27 +261,31 @@ class IronicDriver(base_driver.BaseEngineDriver):
     def plug_vif(self, node_uuid, port_id):
         self.ironicclient.call("node.vif_attach", node_uuid, port_id)
 
-    def unplug_vifs(self, context, server):
+    def unplug_vif(self, context, server, port_id):
-        LOG.debug("unplug: server_uuid=%(uuid)s vif=%(server_nics)s",
+        LOG.debug("unplug: server_uuid=%(uuid)s vif=%(server_nics)s "
+                  "port=%(port_id)s",
                   {'uuid': server.uuid,
-                   'server_nics': str(server.nics)})
+                   'server_nics': str(server.nics),
-        patch = [{'op': 'remove',
+                   'port_id': port_id})
-                  'path': '/extra/vif_port_id'}]
+        node = self._get_node(server.node_uuid)
+        self._unplug_vif(node, server, port_id)
 
-        ports = self.get_ports_from_node(server.node_uuid)
+    def _unplug_vif(self, node, server, port_id):
-
+        for vif in server.nics:
-        for port in ports:
+            if port_id == vif['port_id']:
-            try:
+                try:
-                if 'vif_port_id' in port.extra:
+                    self.ironicclient.call("node.vif_detach", node.uuid,
-                    self.ironicclient.call("port.update",
+                                           port_id)
-                                           port.uuid, patch)
+                except ironic.exc.BadRequest:
-            except client_e.BadRequest:
+                    LOG.debug(
-                pass
+                        "VIF %(vif)s isn't attached to Ironic node %(node)s",
+                        {'vif': port_id, 'node': node.uuid})
 
     def _cleanup_deploy(self, context, node, server):
         # NOTE(liusheng): here we may need to stop firewall if we have
         # implemented in ironic like what Nova dose.
-        self.unplug_vifs(context, server)
+        for vif in server.nics:
+            self.unplug_vif(context, server, vif['port_id'])
 
     def spawn(self, context, server, configdrive_value):
         """Deploy a server.

mogan/engine/manager.py

@@ -508,8 +508,8 @@ class EngineManager(base_manager.BaseEngineManager):
                 LOG.error("Destroy networks for server %(uuid)s failed. "
                           "Exception: %(exception)s",
                           {"uuid": server.uuid, "exception": e})
-
+        for vif in server.nics:
-        self.driver.unplug_vifs(context, server)
+            self.driver.unplug_vif(context, server, vif['port_id'])
         self.driver.destroy(context, server)
 
     @wrap_server_fault
@@ -640,3 +640,24 @@ class EngineManager(base_manager.BaseEngineManager):
             server.save()
         except Exception as e:
             raise exception.InterfaceAttachFailed(message=six.text_type(e))
+
+    def detach_interface(self, context, server, port_id):
+        LOG.info('Detaching interface...', server=server)
+        try:
+            self.driver.unplug_vif(context, server, port_id)
+        except exception.MoganException as e:
+            LOG.warning("Detach interface failed, port_id=%(port_id)s,"
+                        " reason: %(msg)s",
+                        {'port_id': port_id, 'msg': six.text_type(e)})
+            raise exception.InterfaceDetachFailed(server_uuid=server.uuid)
+        else:
+            try:
+                self.network_api.delete_port(context, port_id, server.uuid)
+            except Exception as e:
+                raise exception.InterfaceDetachFailed(server_uuid=server.uuid)
+
+        for nic in server.nics:
+            if nic.port_id == port_id:
+                nic.delete(context)
+
+        LOG.info('Interface was successfully detached')

mogan/engine/rpcapi.py

@@ -87,3 +87,8 @@ class EngineAPI(object):
         cctxt = self.client.prepare(topic=self.topic, server=CONF.host)
         cctxt.call(context, 'attach_interface',
                    server=server, net_id=net_id)
+
+    def detach_interface(self, context, server, port_id):
+        cctxt = self.client.prepare(topic=self.topic, server=CONF.host)
+        cctxt.call(context, 'detach_interface', server=server,
+                   port_id=port_id)

mogan/tests/tempest/api/test_servers.py

@@ -167,7 +167,7 @@ class BaremetalComputeAPIServersTest(base.BaseBaremetalComputeTest):
         self.assertIn('subnet_id', fixed_ip)
         self.assertIn('ip_address', fixed_ip)
 
-    def test_server_attach_interface(self):
+    def test_server_attach_detach_interface(self):
         self._ensure_states_before_test()
         nics_before = self.baremetal_compute_client.server_get_networks(
             self.server_ids[0])
@@ -185,6 +185,14 @@ class BaremetalComputeAPIServersTest(base.BaseBaremetalComputeTest):
             fixed_ip = nic['fixed_ips'][0]
             self.assertIn('subnet_id', fixed_ip)
             self.assertIn('ip_address', fixed_ip)
+        nics_before = self.baremetal_compute_client.server_get_networks(
+            self.server_ids[0])
+        port_id = nics_before[0]['port_id']
+        self.baremetal_compute_client.server_detach_interface(
+            self.server_ids[0], port_id=port_id)
+        nics_after = self.baremetal_compute_client.server_get_networks(
+            self.server_ids[0])
+        self.assertEqual(len(nics_before) - 1, len(nics_after))
 
     def test_floatingip_association_disassociation(self):
         self._ensure_states_before_test()

mogan/tests/tempest/service/client.py

@@ -225,6 +225,15 @@ class BaremetalComputeClient(rest_client.RestClient):
             body = self.deserialize(body)
         return rest_client.ResponseBody(resp, body)
 
+    def server_detach_interface(self, server_id, port_id):
+        uri = '%s/servers/%s/networks/interfaces/%s' % (self.uri_prefix,
+                                                        server_id, port_id)
+        resp, body = self.delete(uri)
+        self.expected_success(204, resp.status)
+        if body:
+            body = self.deserialize(body)
+        return rest_client.ResponseBody(resp, body)
+
 
 class BaremetalNodeClient(rest_client.RestClient):
     version = '1'

mogan/tests/unit/engine/test_engine_api.py

@@ -372,3 +372,12 @@ class ComputeAPIUnitTest(base.DbTestCase):
         azs = self.engine_api.list_availability_zones(self.context)
 
         self.assertItemsEqual(['az1', 'az2'], azs['availability_zones'])
+
+    @mock.patch.object(engine_rpcapi.EngineAPI, 'detach_interface')
+    def test_detach_interface(self, mock_detach_interface):
+        fake_server = db_utils.get_test_server(
+            user_id=self.user_id, project_id=self.project_id)
+        fake_server_obj = self._create_fake_server_obj(fake_server)
+        self.engine_api.detach_interface(self.context, fake_server_obj,
+                                         fake_server_obj['nics'][0]['port_id'])
+        self.assertTrue(mock_detach_interface.called)

mogan/tests/unit/engine/test_manager.py

@@ -53,7 +53,7 @@ class ManageServerTestCase(mgr_utils.ServiceSetUpMixin,
             self.context, server_port_id, server.uuid)
 
     @mock.patch.object(IronicDriver, 'destroy')
-    @mock.patch.object(IronicDriver, 'unplug_vifs')
+    @mock.patch.object(IronicDriver, 'unplug_vif')
     @mock.patch.object(manager.EngineManager, 'destroy_networks')
     def _test__delete_server(self, destroy_networks_mock, unplug_mock,
                              destroy_node_mock, state=None):
@@ -69,7 +69,7 @@ class ManageServerTestCase(mgr_utils.ServiceSetUpMixin,
         self._stop_service()
 
         destroy_networks_mock.assert_called_once_with(self.context, server)
-        unplug_mock.assert_called_once_with(self.context, server)
+        self.assertEqual(unplug_mock.call_count, len(server.nics))
         destroy_node_mock.assert_called_once_with(self.context, server)
 
     def test__delete_server_cleaning(self):
@@ -148,6 +148,21 @@ class ManageServerTestCase(mgr_utils.ServiceSetUpMixin,
         self.assertEqual('localhost', console['host'])
         self.assertIn('token', console)
 
+    @mock.patch.object(network_api.API, 'delete_port')
+    @mock.patch.object(IronicDriver, 'unplug_vif')
+    def test_detach_interface(self, unplug_vif_mock, delete_port_mock):
+        fake_node = mock.MagicMock()
+        fake_node.provision_state = ironic_states.ACTIVE
+        server = obj_utils.create_test_server(
+            self.context, status=states.ACTIVE, node_uuid=None)
+        port_id = server['nics'][0]['port_id']
+        self._start_service()
+        self.service.detach_interface(self.context, server, port_id)
+        self._stop_service()
+        unplug_vif_mock.assert_called_once_with(self.context, server, port_id)
+        delete_port_mock.assert_called_once_with(self.context, port_id,
+                                                 server.uuid)
+
     def test_wrap_server_fault(self):
         server = {"uuid": uuidutils.generate_uuid()}