diff --git a/mogan/baremetal/ironic/driver.py b/mogan/baremetal/ironic/driver.py index e955d214..b3ac8159 100644 --- a/mogan/baremetal/ironic/driver.py +++ b/mogan/baremetal/ironic/driver.py @@ -231,24 +231,16 @@ class IronicDriver(base_driver.BaseEngineDriver): 'server_nics': str(server.nics), 'port_id': port_id}) node = self._get_node(server.node_uuid) - self._unplug_vif(node, server, port_id) + self._unplug_vif(node, port_id) - def _unplug_vif(self, node, server, port_id): - for vif in server.nics: - if port_id == vif['port_id']: - try: - self.ironicclient.call("node.vif_detach", node.uuid, - port_id) - except ironic.exc.BadRequest: - LOG.debug( - "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. - for vif in server.nics: - self.unplug_vif(context, server, vif['port_id']) + def _unplug_vif(self, node, port_id): + try: + self.ironicclient.call("node.vif_detach", node.uuid, + port_id) + except ironic.exc.BadRequest: + LOG.debug( + "VIF %(vif)s isn't attached to Ironic node %(node)s", + {'vif': port_id, 'node': node.uuid}) def spawn(self, context, server, configdrive_value): """Deploy a server. @@ -275,8 +267,6 @@ class IronicDriver(base_driver.BaseEngineDriver): validate_chk = self.ironicclient.call("node.validate", node_uuid) if (not validate_chk.deploy.get('result') or not validate_chk.power.get('result')): - # something is wrong. undo what we have done - self._cleanup_deploy(context, node, server) raise exception.ValidationError(_( "Ironic node: %(id)s failed to validate." " (deploy: %(deploy)s, power: %(power)s)") diff --git a/mogan/db/api.py b/mogan/db/api.py index 04d8c970..a8a44323 100644 --- a/mogan/db/api.py +++ b/mogan/db/api.py @@ -102,12 +102,14 @@ class Connection(object): This query the Nics info of the specified server. """ + @abc.abstractmethod def server_nic_update_or_create(self, context, port_id, values): """Update/Create a nic db entry. This creates or updates a nic db entry. """ + @abc.abstractmethod def server_nic_delete(self, context, port_id): """Delete a nic db entry. diff --git a/mogan/engine/manager.py b/mogan/engine/manager.py index af6fbd74..f2da42cc 100644 --- a/mogan/engine/manager.py +++ b/mogan/engine/manager.py @@ -283,9 +283,8 @@ class EngineManager(base_manager.BaseEngineManager): def destroy_networks(self, context, server): ports = server.nics.get_port_ids() - server.nics.delete(context) for port in ports: - self.network_api.delete_port(context, port, server.uuid) + self._detach_interface(context, server, port) def _rollback_servers_quota(self, context, number): reserve_opts = {'servers': number} @@ -431,8 +430,6 @@ 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_vif(context, server, vif['port_id']) self.driver.destroy(context, server) @wrap_server_fault @@ -554,9 +551,7 @@ class EngineManager(base_manager.BaseEngineManager): raise exception.InterfaceAttachFailed(message=six.text_type(e)) LOG.info('Attaching interface successfully') - def detach_interface(self, context, server, port_id): - LOG.debug("Detaching interface %(port_id) from server %(server)s", - {'port_id': port_id, 'server': server}) + def _detach_interface(self, context, server, port_id): try: self.driver.unplug_vif(context, server, port_id) except exception.MoganException as e: @@ -570,9 +565,15 @@ class EngineManager(base_manager.BaseEngineManager): 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) + try: + objects.ServerNic.delete_by_port_id(context, port_id) + except exception.PortNotFound: + pass + + def detach_interface(self, context, server, port_id): + LOG.debug("Detaching interface %(port_id) from server %(server)s", + {'port_id': port_id, 'server': server.uuid}) + self._detach_interface(context, server, port_id) LOG.info('Interface was successfully detached') diff --git a/mogan/objects/server_nic.py b/mogan/objects/server_nic.py index 0a47131c..2954de4a 100644 --- a/mogan/objects/server_nic.py +++ b/mogan/objects/server_nic.py @@ -58,6 +58,10 @@ class ServerNic(base.MoganObject, object_base.VersionedObjectDictCompat): obj.obj_reset_changes() return obj + @classmethod + def delete_by_port_id(cls, context, port_id): + cls.dbapi.server_nic_delete(context, port_id) + def save(self, context): updates = self.obj_get_changes() self.dbapi.server_nic_update_or_create( @@ -69,8 +73,7 @@ class ServerNic(base.MoganObject, object_base.VersionedObjectDictCompat): context, self.port_id, values) def delete(self, context): - self.dbapi.server_nic_delete( - context, self.port_id) + self.dbapi.server_nic_delete(context, self.port_id) @base.MoganObjectRegistry.register diff --git a/mogan/tests/unit/engine/test_manager.py b/mogan/tests/unit/engine/test_manager.py index 84f8e39e..37450d9e 100644 --- a/mogan/tests/unit/engine/test_manager.py +++ b/mogan/tests/unit/engine/test_manager.py @@ -37,31 +37,30 @@ class ManageServerTestCase(mgr_utils.ServiceSetUpMixin, tests_db_base.DbTestCase): @mock.patch.object(network_api.API, 'delete_port') - def test_destroy_networks(self, delete_port_mock): + @mock.patch.object(IronicDriver, 'unplug_vif') + def test_destroy_networks(self, unplug_vif_mock, delete_port_mock): server = obj_utils.create_test_server(self.context) server_port_id = server.nics[0].port_id delete_port_mock.side_effect = None - port = mock.MagicMock() - port.extra = {'vif_port_id': 'fake-vif'} - port.uuid = 'fake-uuid' + unplug_vif_mock.side_effect = None self._start_service() self.service.destroy_networks(self.context, server) self._stop_service() + unplug_vif_mock.assert_called_once_with( + self.context, server, server_port_id) delete_port_mock.assert_called_once_with( self.context, server_port_id, server.uuid) @mock.patch.object(IronicDriver, 'destroy') - @mock.patch.object(IronicDriver, 'unplug_vif') @mock.patch.object(manager.EngineManager, 'destroy_networks') - def _test__delete_server(self, destroy_networks_mock, unplug_mock, + def _test__delete_server(self, destroy_networks_mock, destroy_node_mock, state=None): fake_node = mock.MagicMock() fake_node.provision_state = state server = obj_utils.create_test_server(self.context) destroy_networks_mock.side_effect = None - unplug_mock.side_effect = None destroy_node_mock.side_effect = None self._start_service() @@ -69,7 +68,6 @@ class ManageServerTestCase(mgr_utils.ServiceSetUpMixin, self._stop_service() destroy_networks_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):