From 5dedd0b22a2ab929d65e91ae6df1ffa2feece5f1 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Wed, 11 Jan 2017 12:24:27 -0500 Subject: [PATCH] xenapi: support the hotplug of a neutron port Nova already has support for hotplugging neutorn ports in the libvirt driver. This extends the support to the XenAPI driver, implement attaching/detaching VIFs I have made a patch to run releated testcase in xenserver CI https://review.openstack.org/#/c/416797/ Change-Id: I22f3fe52d07100592015007653c7f8c47c25d22c Implements: blueprint xenapi-vif-hotplug --- doc/source/support-matrix.ini | 2 +- nova/tests/unit/virt/xenapi/test_driver.py | 15 ++++ nova/tests/unit/virt/xenapi/test_vif.py | 65 +++++++++++++- nova/tests/unit/virt/xenapi/test_vmops.py | 90 +++++++++++++++++++ nova/virt/xenapi/driver.py | 43 +++++++++ nova/virt/xenapi/vif.py | 75 ++++++++++++++-- nova/virt/xenapi/vmops.py | 44 +++++++++ ...p-xenapi-vif-hotplug-2a2b913c49123fe0.yaml | 4 + 8 files changed, 330 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/bp-xenapi-vif-hotplug-2a2b913c49123fe0.yaml diff --git a/doc/source/support-matrix.ini b/doc/source/support-matrix.ini index e3ff1d39fd..105d6369a6 100644 --- a/doc/source/support-matrix.ini +++ b/doc/source/support-matrix.ini @@ -138,7 +138,7 @@ notes=The attach interface operation provides a means to hotplug In a cloud model it would be more typical to just spin up a new instance with more interfaces. cli=nova interface-attach -driver-impl-xenserver=missing +driver-impl-xenserver=complete driver-impl-libvirt-kvm-x86=complete driver-impl-libvirt-kvm-ppc64=complete driver-impl-libvirt-kvm-s390x=complete diff --git a/nova/tests/unit/virt/xenapi/test_driver.py b/nova/tests/unit/virt/xenapi/test_driver.py index 0b89222a96..75c9f15b28 100644 --- a/nova/tests/unit/virt/xenapi/test_driver.py +++ b/nova/tests/unit/virt/xenapi/test_driver.py @@ -200,3 +200,18 @@ class XenAPIDriverTestCase(stubs.XenAPITestBaseNoDB): self.assertTrue(mock_cleanup.called) self.assertTrue(mock_ensure.called) + + @mock.patch.object(xenapi_driver.vmops.VMOps, 'attach_interface') + def test_attach_interface(self, mock_attach_interface): + driver = self._get_driver() + driver.attach_interface('fake_context', 'fake_instance', + 'fake_image_meta', 'fake_vif') + mock_attach_interface.assert_called_once_with('fake_instance', + 'fake_vif') + + @mock.patch.object(xenapi_driver.vmops.VMOps, 'detach_interface') + def test_detach_interface(self, mock_detach_interface): + driver = self._get_driver() + driver.detach_interface('fake_context', 'fake_instance', 'fake_vif') + mock_detach_interface.assert_called_once_with('fake_instance', + 'fake_vif') diff --git a/nova/tests/unit/virt/xenapi/test_vif.py b/nova/tests/unit/virt/xenapi/test_vif.py index 320a92a115..248b6b2f7c 100644 --- a/nova/tests/unit/virt/xenapi/test_vif.py +++ b/nova/tests/unit/virt/xenapi/test_vif.py @@ -15,12 +15,14 @@ import mock +from nova.compute import power_state from nova import exception from nova.network import model from nova import test from nova.tests.unit.virt.xenapi import stubs from nova.virt.xenapi import network_utils from nova.virt.xenapi import vif +from nova.virt.xenapi import vm_utils fake_vif = { @@ -126,14 +128,17 @@ class XenVIFDriverTestCase(XenVIFDriverTestBase): self.base_driver._create_vif, "fake_vif", "missing_vif_rec", "fake_vm_ref") + @mock.patch.object(vif.XenVIFDriver, 'hot_unplug') @mock.patch.object(vif.XenVIFDriver, '_get_vif_ref', return_value='fake_vif_ref') - def test_unplug(self, mock_get_vif_ref): + def test_unplug(self, mock_get_vif_ref, mock_hot_unplug): instance = {'name': "fake_instance"} vm_ref = "fake_vm_ref" self.base_driver.unplug(instance, fake_vif, vm_ref) expected = [mock.call('VIF.destroy', 'fake_vif_ref')] self.assertEqual(expected, self._session.call_xenapi.call_args_list) + mock_hot_unplug.assert_called_once_with( + fake_vif, instance, 'fake_vm_ref', 'fake_vif_ref') @mock.patch.object(vif.XenVIFDriver, '_get_vif_ref', return_value='missing_vif_ref') @@ -173,6 +178,7 @@ class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase): super(XenAPIOpenVswitchDriverTestCase, self).setUp() self.ovs_driver = vif.XenAPIOpenVswitchDriver(self._session) + @mock.patch.object(vif.XenAPIOpenVswitchDriver, 'hot_plug') @mock.patch.object(vif.XenVIFDriver, '_create_vif', return_value='fake_vif_ref') @mock.patch.object(vif.XenAPIOpenVswitchDriver, @@ -181,7 +187,7 @@ class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase): @mock.patch.object(vif.vm_utils, 'lookup', return_value='fake_vm_ref') def test_plug(self, mock_lookup, mock_get_vif_ref, mock_create_vif_interim_network, - mock_create_vif): + mock_create_vif, mock_hot_plug): instance = {'name': "fake_instance_name"} ret_vif_ref = self.ovs_driver.plug( instance, fake_vif, vm_ref=None, device=1) @@ -190,6 +196,8 @@ class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase): self.assertTrue(mock_create_vif_interim_network.called) self.assertTrue(mock_create_vif.called) self.assertEqual('fake_vif_ref', ret_vif_ref) + mock_hot_plug.assert_called_once_with(fake_vif, instance, + 'fake_vm_ref', 'fake_vif_ref') @mock.patch.object(vif.XenAPIOpenVswitchDriver, '_delete_linux_bridge') @mock.patch.object(vif.XenAPIOpenVswitchDriver, '_delete_linux_port') @@ -299,3 +307,56 @@ class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase): network_ref = self.ovs_driver.create_vif_interim_network(fake_vif) self.assertTrue(mock_network_create.called) self.assertEqual(network_ref, 'new_network_ref') + + @mock.patch.object(vif.XenAPIOpenVswitchDriver, 'post_start_actions') + @mock.patch.object(vm_utils, 'get_power_state') + def test_hot_plug_power_on(self, mock_get_power_state, + mock_post_start_actions): + vif_ref = "fake_vif_ref" + vif = "fake_vif" + instance = "fake_instance" + vm_ref = "fake_vm_ref" + mock_get_power_state.return_value = power_state.RUNNING + mock_VIF_plug = self.mock_patch_object( + self._session.VIF, 'plug', return_val=None) + self.ovs_driver.hot_plug(vif, instance, vm_ref, vif_ref) + mock_VIF_plug.assert_called_once_with(vif_ref) + mock_post_start_actions.assert_called_once_with(instance, vif_ref) + mock_get_power_state.assert_called_once_with(self._session, vm_ref) + + @mock.patch.object(vm_utils, 'get_power_state') + def test_hot_plug_power_off(self, mock_get_power_state): + vif_ref = "fake_vif_ref" + vif = "fake_vif" + instance = "fake_instance" + vm_ref = "fake_vm_ref" + mock_get_power_state.return_value = power_state.SHUTDOWN + mock_VIF_plug = self.mock_patch_object( + self._session.VIF, 'plug', return_val=None) + self.ovs_driver.hot_plug(vif, instance, vm_ref, vif_ref) + mock_VIF_plug.assert_not_called() + mock_get_power_state.assert_called_once_with(self._session, vm_ref) + + @mock.patch.object(vm_utils, 'get_power_state') + def test_hot_unplug_power_on(self, mock_get_power_state): + vm_ref = 'fake_vm_ref' + vif_ref = 'fake_vif_ref' + instance = 'fake_instance' + mock_get_power_state.return_value = power_state.RUNNING + mock_VIF_unplug = self.mock_patch_object( + self._session.VIF, 'unplug', return_val=None) + self.ovs_driver.hot_unplug(fake_vif, instance, vm_ref, vif_ref) + mock_VIF_unplug.assert_called_once_with(vif_ref) + mock_get_power_state.assert_called_once_with(self._session, vm_ref) + + @mock.patch.object(vm_utils, 'get_power_state') + def test_hot_unplug_power_off(self, mock_get_power_state): + vm_ref = 'fake_vm_ref' + vif_ref = 'fake_vif_ref' + instance = 'fake_instance' + mock_get_power_state.return_value = power_state.SHUTDOWN + mock_VIF_unplug = self.mock_patch_object( + self._session.VIF, 'unplug', return_val=None) + self.ovs_driver.hot_unplug(fake_vif, instance, vm_ref, vif_ref) + mock_VIF_unplug.assert_not_called() + mock_get_power_state.assert_called_once_with(self._session, vm_ref) diff --git a/nova/tests/unit/virt/xenapi/test_vmops.py b/nova/tests/unit/virt/xenapi/test_vmops.py index 1a22243bcc..5ec3abe14b 100644 --- a/nova/tests/unit/virt/xenapi/test_vmops.py +++ b/nova/tests/unit/virt/xenapi/test_vmops.py @@ -23,6 +23,7 @@ except ImportError: from eventlet import greenthread import mock from os_xenapi.client import session as xenapi_session +import six from nova.compute import power_state from nova.compute import task_states @@ -1726,3 +1727,92 @@ class GetVdisForInstanceTestCase(VMOpsTestBase): # our stub method is called which asserts the password is scrubbed self.assertTrue(debug_mock.called) self.assertTrue(fake_debug.matched) + + +class AttachInterfaceTestCase(VMOpsTestBase): + """Test VIF hot plug/unplug""" + def setUp(self): + super(AttachInterfaceTestCase, self).setUp() + self.vmops.vif_driver = mock.Mock() + self.fake_vif = {'id': '12345'} + self.fake_instance = mock.Mock() + self.fake_instance.uuid = '6478' + + @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref') + def test_attach_interface(self, mock_get_vm_opaque_ref): + mock_get_vm_opaque_ref.return_value = 'fake_vm_ref' + with mock.patch.object(self._session.VM, 'get_allowed_VIF_devices')\ + as fake_devices: + fake_devices.return_value = [2, 3, 4] + self.vmops.attach_interface(self.fake_instance, self.fake_vif) + fake_devices.assert_called_once_with('fake_vm_ref') + mock_get_vm_opaque_ref.assert_called_once_with(self.fake_instance) + self.vmops.vif_driver.plug.assert_called_once_with( + self.fake_instance, self.fake_vif, vm_ref='fake_vm_ref', + device=2) + + @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref') + def test_attach_interface_no_devices(self, mock_get_vm_opaque_ref): + mock_get_vm_opaque_ref.return_value = 'fake_vm_ref' + with mock.patch.object(self._session.VM, 'get_allowed_VIF_devices')\ + as fake_devices: + fake_devices.return_value = [] + self.assertRaises(exception.InterfaceAttachFailed, + self.vmops.attach_interface, + self.fake_instance, self.fake_vif) + + @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref') + def test_attach_interface_plug_failed(self, mock_get_vm_opaque_ref): + mock_get_vm_opaque_ref.return_value = 'fake_vm_ref' + with mock.patch.object(self._session.VM, 'get_allowed_VIF_devices')\ + as fake_devices: + fake_devices.return_value = [2, 3, 4] + self.vmops.vif_driver.plug.side_effect =\ + exception.VirtualInterfacePlugException('Failed to plug VIF') + self.assertRaises(exception.VirtualInterfacePlugException, + self.vmops.attach_interface, + self.fake_instance, self.fake_vif) + self.vmops.vif_driver.plug.assert_called_once_with( + self.fake_instance, self.fake_vif, vm_ref='fake_vm_ref', + device=2) + self.vmops.vif_driver.unplug.assert_called_once_with( + self.fake_instance, self.fake_vif, 'fake_vm_ref') + + @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref') + def test_attach_interface_reraise_exception(self, mock_get_vm_opaque_ref): + mock_get_vm_opaque_ref.return_value = 'fake_vm_ref' + with mock.patch.object(self._session.VM, 'get_allowed_VIF_devices')\ + as fake_devices: + fake_devices.return_value = [2, 3, 4] + self.vmops.vif_driver.plug.side_effect =\ + exception.VirtualInterfacePlugException('Failed to plug VIF') + self.vmops.vif_driver.unplug.side_effect =\ + exception.VirtualInterfaceUnplugException( + 'Failed to unplug VIF') + ex = self.assertRaises(exception.VirtualInterfacePlugException, + self.vmops.attach_interface, + self.fake_instance, self.fake_vif) + self.assertEqual('Failed to plug VIF', six.text_type(ex)) + self.vmops.vif_driver.plug.assert_called_once_with( + self.fake_instance, self.fake_vif, vm_ref='fake_vm_ref', + device=2) + self.vmops.vif_driver.unplug.assert_called_once_with( + self.fake_instance, self.fake_vif, 'fake_vm_ref') + + @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref') + def test_detach_interface(self, mock_get_vm_opaque_ref): + mock_get_vm_opaque_ref.return_value = 'fake_vm_ref' + self.vmops.detach_interface(self.fake_instance, self.fake_vif) + mock_get_vm_opaque_ref.assert_called_once_with(self.fake_instance) + self.vmops.vif_driver.unplug.assert_called_once_with( + self.fake_instance, self.fake_vif, 'fake_vm_ref') + + @mock.patch.object(vmops.VMOps, '_get_vm_opaque_ref') + def test_detach_interface_exception(self, mock_get_vm_opaque_ref): + mock_get_vm_opaque_ref.return_value = 'fake_vm_ref' + self.vmops.vif_driver.unplug.side_effect =\ + exception.VirtualInterfaceUnplugException('Failed to unplug VIF') + + self.assertRaises(exception.VirtualInterfaceUnplugException, + self.vmops.detach_interface, + self.fake_instance, self.fake_vif) diff --git a/nova/virt/xenapi/driver.py b/nova/virt/xenapi/driver.py index a40d1588f6..ece6875c14 100644 --- a/nova/virt/xenapi/driver.py +++ b/nova/virt/xenapi/driver.py @@ -66,6 +66,13 @@ def invalid_option(option_name, recommended_value): class XenAPIDriver(driver.ComputeDriver): """A connection to XenServer or Xen Cloud Platform.""" + capabilities = { + "has_imagecache": False, + "supports_recreate": False, + "supports_migrate_to_same_host": False, + "supports_attach_interface": True, + "supports_device_tagging": False, + } def __init__(self, virtapi, read_only=False): super(XenAPIDriver, self).__init__(virtapi) @@ -654,3 +661,39 @@ class XenAPIDriver(driver.ComputeDriver): :returns: dict of nova uuid => dict of usage info """ return self._vmops.get_per_instance_usage() + + def attach_interface(self, context, instance, image_meta, vif): + """Use hotplug to add a network interface to a running instance. + + The counter action to this is :func:`detach_interface`. + + :param context: The request context. + :param nova.objects.instance.Instance instance: + The instance which will get an additional network interface. + :param nova.objects.ImageMeta image_meta: + The metadata of the image of the instance. + :param nova.network.model.VIF vif: + The object which has the information about the interface to attach. + + :raise nova.exception.NovaException: If the attach fails. + + :return: None + """ + self._vmops.attach_interface(instance, vif) + + def detach_interface(self, context, instance, vif): + """Use hotunplug to remove a network interface from a running instance. + + The counter action to this is :func:`attach_interface`. + + :param context: The request context. + :param nova.objects.instance.Instance instance: + The instance which gets a network interface removed. + :param nova.network.model.VIF vif: + The object which has the information about the interface to detach. + + :raise nova.exception.NovaException: If the detach fails. + + :return: None + """ + self._vmops.detach_interface(instance, vif) diff --git a/nova/virt/xenapi/vif.py b/nova/virt/xenapi/vif.py index d0a4c03c8f..c0a9695746 100644 --- a/nova/virt/xenapi/vif.py +++ b/nova/virt/xenapi/vif.py @@ -19,6 +19,7 @@ from oslo_log import log as logging +from nova.compute import power_state import nova.conf from nova import exception from nova.i18n import _ @@ -72,6 +73,8 @@ class XenVIFDriver(object): LOG.debug("vif didn't exist, no need to unplug vif %s", vif, instance=instance) return + # hot unplug the VIF first + self.hot_unplug(vif, instance, vm_ref, vif_ref) self._session.call_xenapi('VIF.destroy', vif_ref) except Exception as e: LOG.warning( @@ -80,6 +83,44 @@ class XenVIFDriver(object): raise exception.NovaException( reason=_("Failed to unplug vif %s") % vif) + def hot_plug(self, vif, instance, vm_ref, vif_ref): + """hotplug virtual interface to running instance. + :param nova.network.model.VIF vif: + The object which has the information about the interface to attach. + :param nova.objects.instance.Instance instance: + The instance which will get an additional network interface. + :param string vm_ref: + The instance's reference from hypervisor's point of view. + :param string vif_ref: + The interface's reference from hypervisor's point of view. + :return: None + """ + pass + + def hot_unplug(self, vif, instance, vm_ref, vif_ref): + """hot unplug virtual interface from running instance. + :param nova.network.model.VIF vif: + The object which has the information about the interface to detach. + :param nova.objects.instance.Instance instance: + The instance which will remove additional network interface. + :param string vm_ref: + The instance's reference from hypervisor's point of view. + :param string vif_ref: + The interface's reference from hypervisor's point of view. + :return: None + """ + pass + + def post_start_actions(self, instance, vif_ref): + """post actions when the instance is power on. + :param nova.objects.instance.Instance instance: + The instance which will execute extra actions after power on + :param string vif_ref: + The interface's reference from hypervisor's point of view. + :return: None + """ + pass + class XenAPIBridgeDriver(XenVIFDriver): """VIF Driver for XenAPI that uses XenAPI to create Networks.""" @@ -181,10 +222,6 @@ class XenAPIBridgeDriver(XenVIFDriver): def unplug(self, instance, vif, vm_ref): super(XenAPIBridgeDriver, self).unplug(instance, vif, vm_ref) - def post_start_actions(self, instance, vif_ref): - """no further actions needed for this driver type""" - pass - class XenAPIOpenVswitchDriver(XenVIFDriver): """VIF driver for Open vSwitch with XenAPI.""" @@ -221,7 +258,12 @@ class XenAPIOpenVswitchDriver(XenVIFDriver): # OVS on the hypervisor monitors this key and uses it to # set the iface-id attribute vif_rec['other_config'] = {'nicira-iface-id': vif['id']} - return self._create_vif(vif, vif_rec, vm_ref) + vif_ref = self._create_vif(vif, vif_rec, vm_ref) + + # call XenAPI to plug vif + self.hot_plug(vif, instance, vm_ref, vif_ref) + + return vif_ref def unplug(self, instance, vif, vm_ref): """unplug vif: @@ -289,6 +331,29 @@ class XenAPIOpenVswitchDriver(XenVIFDriver): raise exception.VirtualInterfaceUnplugException( reason=_("Failed to delete bridge")) + def hot_plug(self, vif, instance, vm_ref, vif_ref): + # hot plug vif only when VM's power state is running + LOG.debug("Hot plug vif, vif: %s", vif, instance=instance) + state = vm_utils.get_power_state(self._session, vm_ref) + if state != power_state.RUNNING: + LOG.debug("Skip hot plug VIF, VM is not running, vif: %s", vif, + instance=instance) + return + + self._session.VIF.plug(vif_ref) + self.post_start_actions(instance, vif_ref) + + def hot_unplug(self, vif, instance, vm_ref, vif_ref): + # hot unplug vif only when VM's power state is running + LOG.debug("Hot unplug vif, vif: %s", vif, instance=instance) + state = vm_utils.get_power_state(self._session, vm_ref) + if state != power_state.RUNNING: + LOG.debug("Skip hot unplug VIF, VM is not running, vif: %s", vif, + instance=instance) + return + + self._session.VIF.unplug(vif_ref) + def _get_qbr_name(self, iface_id): return ("qbr" + iface_id)[:network_model.NIC_NAME_LEN] diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 9054051e4c..4f27be4e4d 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -2484,3 +2484,47 @@ class VMOps(object): volume_utils.forget_sr(self._session, sr_ref) return sr_uuid_map + + def attach_interface(self, instance, vif): + LOG.debug("Attach interface, vif info: %s", vif, instance=instance) + vm_ref = self._get_vm_opaque_ref(instance) + + @utils.synchronized('xenapi-vif-' + vm_ref) + def _attach_interface(instance, vm_ref, vif): + # find device for use with XenAPI + allowed_devices = self._session.VM.get_allowed_VIF_devices(vm_ref) + if allowed_devices is None or len(allowed_devices) == 0: + raise exception.InterfaceAttachFailed( + _('attach network interface %(vif_id)s to instance ' + '%(instance_uuid)s failed, no allowed devices.'), + vif_id=vif['id'], instance_uuid=instance.uuid) + device = allowed_devices[0] + try: + # plug VIF + self.vif_driver.plug(instance, vif, vm_ref=vm_ref, + device=device) + # set firewall filtering + self.firewall_driver.setup_basic_filtering(instance, [vif]) + except exception.NovaException: + with excutils.save_and_reraise_exception(): + LOG.exception(_LE('attach network interface %s failed.'), + vif['id'], instance=instance) + try: + self.vif_driver.unplug(instance, vif, vm_ref) + except exception.NovaException: + # if unplug failed, no need to raise exception + LOG.warning(_LW('Unplug VIF %s failed.'), + vif['id'], instance=instance) + + _attach_interface(instance, vm_ref, vif) + + def detach_interface(self, instance, vif): + LOG.debug("Detach interface, vif info: %s", vif, instance=instance) + + try: + vm_ref = self._get_vm_opaque_ref(instance) + self.vif_driver.unplug(instance, vif, vm_ref) + except exception.NovaException: + with excutils.save_and_reraise_exception(): + LOG.exception(_LE('detach network interface %s failed.'), + vif['id'], instance=instance) diff --git a/releasenotes/notes/bp-xenapi-vif-hotplug-2a2b913c49123fe0.yaml b/releasenotes/notes/bp-xenapi-vif-hotplug-2a2b913c49123fe0.yaml new file mode 100644 index 0000000000..6acee6fc10 --- /dev/null +++ b/releasenotes/notes/bp-xenapi-vif-hotplug-2a2b913c49123fe0.yaml @@ -0,0 +1,4 @@ +--- +features: + - The XenServer compute driver now supports hot-plugging + virtual network interfaces.