From c791cc4e04c4c8061ce0bf7c9a57e90de3001f56 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 11 May 2018 11:15:07 -0400 Subject: [PATCH] xenapi: drop deprecated vif_driver config option The vif_driver option was deprecated in Ocata: I599f3449f18d2821403961fb9d52e9a14dd3366b And can now be removed. The only supported networking backend is neutron + ovs. Related to blueprint remove-nova-network Co-Authored-By: Naichuan Sun Change-Id: Ia977f115335f00bc36249fa67437b4336d524251 --- .../configuration/hypervisor-xen-api.rst | 1 - nova/conf/xenserver.py | 34 ------ nova/tests/unit/virt/xenapi/stubs.py | 8 -- nova/tests/unit/virt/xenapi/test_vif.py | 32 ------ nova/tests/unit/virt/xenapi/test_xenapi.py | 58 +++++++--- nova/virt/xenapi/fake.py | 15 +++ nova/virt/xenapi/vif.py | 108 ------------------ nova/virt/xenapi/vmops.py | 5 +- ...er-vif-driver-option-850f8dcfe54bca7c.yaml | 11 ++ 9 files changed, 74 insertions(+), 198 deletions(-) create mode 100644 releasenotes/notes/remove-xenserver-vif-driver-option-850f8dcfe54bca7c.yaml diff --git a/doc/source/admin/configuration/hypervisor-xen-api.rst b/doc/source/admin/configuration/hypervisor-xen-api.rst index a5aff3354593..958fc46517cf 100644 --- a/doc/source/admin/configuration/hypervisor-xen-api.rst +++ b/doc/source/admin/configuration/hypervisor-xen-api.rst @@ -361,7 +361,6 @@ To enable the XenAPI driver, add the following configuration options to connection_username = root connection_password = your_password ovs_integration_bridge = br-int - vif_driver = nova.virt.xenapi.vif.XenAPIOpenVswitchDriver These connection details are used by OpenStack Compute service to contact your hypervisor and are the same details you use to connect XenCenter, the XenServer diff --git a/nova/conf/xenserver.py b/nova/conf/xenserver.py index f96664ade48c..f80d2af6e2b4 100644 --- a/nova/conf/xenserver.py +++ b/nova/conf/xenserver.py @@ -412,40 +412,6 @@ time is involved until the instance(s) can become available and instances do not go to running state within this specified wait time, the launch expires and the instance(s) are set to 'error' state. -"""), - cfg.StrOpt('vif_driver', - default='nova.virt.xenapi.vif.XenAPIOpenVswitchDriver', - deprecated_for_removal=True, - deprecated_since='15.0.0', - deprecated_reason=""" -There are only two in-tree vif drivers for XenServer. XenAPIBridgeDriver is for -nova-network which is deprecated and XenAPIOpenVswitchDriver is for Neutron -which is the default configuration for Nova since the 15.0.0 Ocata release. In -the future the "use_neutron" configuration option will be used to determine -which vif driver to use. -""", - help=""" -The XenAPI VIF driver using XenServer Network APIs. - -Provide a string value representing the VIF XenAPI vif driver to use for -plugging virtual network interfaces. - -Xen configuration uses bridging within the backend domain to allow -all VMs to appear on the network as individual hosts. Bridge -interfaces are used to create a XenServer VLAN network in which -the VIFs for the VM instances are plugged. If no VIF bridge driver -is plugged, the bridge is not made available. This configuration -option takes in a value for the VIF driver. - -Possible values: - -* nova.virt.xenapi.vif.XenAPIOpenVswitchDriver (default) -* nova.virt.xenapi.vif.XenAPIBridgeDriver (deprecated) - -Related options: - -* ``vlan_interface`` -* ``ovs_integration_bridge`` """), # TODO(dharinic): Make this, a stevedore plugin cfg.StrOpt('image_upload_handler', diff --git a/nova/tests/unit/virt/xenapi/stubs.py b/nova/tests/unit/virt/xenapi/stubs.py index df1f5078cbaa..d1357d599b96 100644 --- a/nova/tests/unit/virt/xenapi/stubs.py +++ b/nova/tests/unit/virt/xenapi/stubs.py @@ -397,10 +397,6 @@ def get_fake_session(error=None): class XenAPITestBase(test.TestCase): def setUp(self): super(XenAPITestBase, self).setUp() - # TODO(mriedem): The tests need to be fixed to work with the - # XenAPIOpenVswitchDriver vif driver. - self.flags(vif_driver='nova.virt.xenapi.vif.XenAPIBridgeDriver', - group='xenserver') self.useFixture(ReplaceModule('XenAPI', fake)) fake.reset() @@ -408,9 +404,5 @@ class XenAPITestBase(test.TestCase): class XenAPITestBaseNoDB(test.NoDBTestCase): def setUp(self): super(XenAPITestBaseNoDB, self).setUp() - # TODO(mriedem): The tests need to be fixed to work with the - # XenAPIOpenVswitchDriver vif driver. - self.flags(vif_driver='nova.virt.xenapi.vif.XenAPIBridgeDriver', - group='xenserver') self.useFixture(ReplaceModule('XenAPI', fake)) fake.reset() diff --git a/nova/tests/unit/virt/xenapi/test_vif.py b/nova/tests/unit/virt/xenapi/test_vif.py index e2472138037e..66eddeb6c638 100644 --- a/nova/tests/unit/virt/xenapi/test_vif.py +++ b/nova/tests/unit/virt/xenapi/test_vif.py @@ -17,7 +17,6 @@ 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 @@ -151,37 +150,6 @@ class XenVIFDriverTestCase(XenVIFDriverTestBase): instance, fake_vif, vm_ref) -class XenAPIBridgeDriverTestCase(XenVIFDriverTestBase, object): - def setUp(self): - super(XenAPIBridgeDriverTestCase, self).setUp() - self.bridge_driver = vif.XenAPIBridgeDriver(self._session) - - @mock.patch.object(vif.XenAPIBridgeDriver, '_ensure_vlan_bridge', - return_value='fake_network_ref') - @mock.patch.object(vif.XenVIFDriver, '_create_vif', - return_value='fake_vif_ref') - def test_plug_create_vlan(self, mock_create_vif, mock_ensure_vlan_bridge): - instance = {'name': "fake_instance_name"} - network = model.Network() - network._set_meta({'should_create_vlan': True}) - vif = model.VIF() - vif._set_meta({'rxtx_cap': 1}) - vif['network'] = network - vif['address'] = "fake_address" - vm_ref = "fake_vm_ref" - device = 1 - ret_vif_ref = self.bridge_driver.plug(instance, vif, vm_ref, device) - self.assertEqual('fake_vif_ref', ret_vif_ref) - - @mock.patch.object(vif.vm_utils, 'lookup', return_value=None) - def test_plug_exception(self, mock_lookup): - instance = {'name': "fake_instance_name"} - self.assertRaises(exception.VirtualInterfacePlugException, - self.bridge_driver.plug, instance, fake_vif, - vm_ref=None, device=1) - mock_lookup.assert_called_once_with(self._session, instance['name']) - - class XenAPIOpenVswitchDriverTestCase(XenVIFDriverTestBase): def setUp(self): super(XenAPIOpenVswitchDriverTestCase, self).setUp() diff --git a/nova/tests/unit/virt/xenapi/test_xenapi.py b/nova/tests/unit/virt/xenapi/test_xenapi.py index ce243ee3ff21..a95d65ac4aae 100644 --- a/nova/tests/unit/virt/xenapi/test_xenapi.py +++ b/nova/tests/unit/virt/xenapi/test_xenapi.py @@ -45,6 +45,7 @@ from nova import context from nova import crypto from nova import db from nova import exception +from nova.network import model as network_model from nova import objects from nova.objects import base from nova.objects import fields as obj_fields @@ -796,10 +797,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase, group='xenserver') self._test_spawn(IMAGE_IPXE_ISO, None, None) - - # ipxe inject shouldn't be called - mock_call_plugin_serialized.assert_called_once_with( - 'partition_utils.py', 'make_partition', 'fakedev', '2048', '-') + self._check_call_plugin_serialized(mock_call_plugin_serialized) @mock.patch.object(session.XenAPISession, 'call_plugin_serialized') def test_spawn_ipxe_iso_no_boot_menu_url( @@ -809,10 +807,48 @@ class XenAPIVMTestCase(stubs.XenAPITestBase, group='xenserver') self._test_spawn(IMAGE_IPXE_ISO, None, None) + self._check_call_plugin_serialized(mock_call_plugin_serialized) + + def _check_call_plugin_serialized(self, mock_call_plugin_serialized): + vifs = xenapi_fake.get_all_records('VIF') + iface_id = vifs[list(vifs)[0]]['other_config']['neutron-port-id'] + + def _get_qbr_name(iface_id): + return ("qbr" + iface_id)[:network_model.NIC_NAME_LEN] + + def _get_veth_pair_names(iface_id): + return (("qvb%s" % iface_id)[:network_model.NIC_NAME_LEN], + ("qvo%s" % iface_id)[:network_model.NIC_NAME_LEN]) + + def _get_patch_port_pair_names(iface_id): + return (("vif%s" % iface_id)[:network_model.NIC_NAME_LEN], + ("tap%s" % iface_id)[:network_model.NIC_NAME_LEN]) # ipxe inject shouldn't be called - mock_call_plugin_serialized.assert_called_once_with( - 'partition_utils.py', 'make_partition', 'fakedev', '2048', '-') + call1 = mock.call('partition_utils.py', 'make_partition', 'fakedev', + '2048', '-') + linux_br_name = _get_qbr_name(iface_id) + qvb_name, qvo_name = _get_veth_pair_names(iface_id) + patch_port1, tap_name = _get_patch_port_pair_names(iface_id) + + args = {'cmd': 'ip_link_get_dev', + 'args': {'device_name': linux_br_name} + } + call2 = mock.call('xenhost.py', 'network_config', args) + + args = {'cmd': 'ip_link_get_dev', + 'args': {'device_name': qvo_name} + } + call3 = mock.call('xenhost.py', 'network_config', args) + + args = {'cmd': 'ip_link_get_dev', + 'args': {'device_name': tap_name} + } + call4 = mock.call('xenhost.py', 'network_config', args) + mock_call_plugin_serialized.assert_has_calls([call1, + call2, + call3, + call4]) @mock.patch.object(session.XenAPISession, 'call_plugin_serialized') def test_spawn_ipxe_iso_unknown_network_name( @@ -822,10 +858,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase, group='xenserver') self._test_spawn(IMAGE_IPXE_ISO, None, None) - - # ipxe inject shouldn't be called - mock_call_plugin_serialized.assert_called_once_with( - 'partition_utils.py', 'make_partition', 'fakedev', '2048', '-') + self._check_call_plugin_serialized(mock_call_plugin_serialized) def test_spawn_empty_dns(self): # Test spawning with an empty dns list. @@ -1057,9 +1090,8 @@ class XenAPIVMTestCase(stubs.XenAPITestBase, self._create_instance() for vif_ref in xenapi_fake.get_all('VIF'): vif_rec = xenapi_fake.get_record('VIF', vif_ref) - self.assertEqual(vif_rec['qos_algorithm_type'], 'ratelimit') - self.assertEqual(vif_rec['qos_algorithm_params']['kbps'], - str(3 * 10 * 1024)) + self.assertEqual(vif_rec['qos_algorithm_type'], '') + self.assertEqual(vif_rec['qos_algorithm_params'], {}) @mock.patch.object(crypto, 'ssh_encrypt_text') @mock.patch.object(stubs.FakeSessionForVMTests, diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py index 969eff8b22e1..050ae7409b5b 100644 --- a/nova/virt/xenapi/fake.py +++ b/nova/virt/xenapi/fake.py @@ -760,6 +760,9 @@ class SessionBase(object): def _plugin_xenhost_host_uptime(self, method, args): return jsonutils.dumps({"uptime": "fake uptime"}) + def _plugin_xenhost_network_config(self, method, args): + return pickle.dumps({"fake_network": "fake conf"}) + def _plugin_xenhost_get_pci_device_details(self, method, args): """Simulate the ouput of three pci devices. @@ -1007,6 +1010,18 @@ class SessionBase(object): _db_content[cls], func[len('get_by_'):], params[1], return_singleton=return_singleton) + if func == 'get_VIFs': + self._check_arg_count(params, 2) + # FIXME(mriedem): figure out how to use _get_by_field for VIFs, + # or just stop relying on this fake DB and use mock + return _db_content['VIF'].keys() + + if func == 'get_bridge': + self._check_arg_count(params, 2) + # FIXME(mriedem): figure out how to use _get_by_field for bridge, + # or just stop relying on this fake DB and use mock + return 'fake_bridge' + if len(params) == 2: field = func[len('get_'):] ref = params[1] diff --git a/nova/virt/xenapi/vif.py b/nova/virt/xenapi/vif.py index 203ea440e265..766375043471 100644 --- a/nova/virt/xenapi/vif.py +++ b/nova/virt/xenapi/vif.py @@ -131,114 +131,6 @@ class XenVIFDriver(object): pass -class XenAPIBridgeDriver(XenVIFDriver): - """VIF Driver for XenAPI that uses XenAPI to create Networks.""" - - # NOTE(huanxie): This driver uses linux bridge as backend for XenServer, - # it only supports nova network, for using neutron, you should use - # XenAPIOpenVswitchDriver - - def plug(self, instance, vif, vm_ref=None, device=None): - if not vm_ref: - vm_ref = vm_utils.lookup(self._session, instance['name']) - if not vm_ref: - raise exception.VirtualInterfacePlugException( - "Cannot find instance %s, discard vif plug" % instance['name']) - - # if VIF already exists, return this vif_ref directly - vif_ref = self._get_vif_ref(vif, vm_ref) - if vif_ref: - LOG.debug("VIF %s already exists when plug vif", - vif_ref, instance=instance) - return vif_ref - - if not device: - device = 0 - - if vif['network'].get_meta('should_create_vlan'): - network_ref = self._ensure_vlan_bridge(vif['network']) - else: - network_ref = network_utils.find_network_with_bridge( - self._session, vif['network']['bridge']) - vif_rec = {} - vif_rec['device'] = str(device) - vif_rec['network'] = network_ref - vif_rec['VM'] = vm_ref - vif_rec['MAC'] = vif['address'] - vif_rec['MTU'] = '1500' - vif_rec['other_config'] = {} - if vif.get_meta('rxtx_cap'): - vif_rec['qos_algorithm_type'] = 'ratelimit' - vif_rec['qos_algorithm_params'] = {'kbps': - str(int(vif.get_meta('rxtx_cap')) * 1024)} - else: - vif_rec['qos_algorithm_type'] = '' - vif_rec['qos_algorithm_params'] = {} - return self._create_vif(vif, vif_rec, vm_ref) - - def _ensure_vlan_bridge(self, network): - """Ensure that a VLAN bridge exists.""" - - vlan_num = network.get_meta('vlan') - bridge = network['bridge'] - bridge_interface = (CONF.vlan_interface or - network.get_meta('bridge_interface')) - # Check whether bridge already exists - # Retrieve network whose name_label is "bridge" - network_ref = network_utils.find_network_with_name_label( - self._session, bridge) - if network_ref is None: - # If bridge does not exists - # 1 - create network - description = 'network for nova bridge %s' % bridge - network_rec = {'name_label': bridge, - 'name_description': description, - 'other_config': {}} - network_ref = self._session.call_xenapi('network.create', - network_rec) - # 2 - find PIF for VLAN NOTE(salvatore-orlando): using double - # quotes inside single quotes as xapi filter only support - # tokens in double quotes - expr = ('field "device" = "%s" and field "VLAN" = "-1"' % - bridge_interface) - pifs = self._session.call_xenapi('PIF.get_all_records_where', - expr) - - # Multiple PIF are ok: we are dealing with a pool - if len(pifs) == 0: - raise Exception(_('Found no PIF for device %s') % - bridge_interface) - for pif_ref in pifs.keys(): - self._session.call_xenapi('VLAN.create', - pif_ref, - str(vlan_num), - network_ref) - else: - # Check VLAN tag is appropriate - network_rec = self._session.call_xenapi('network.get_record', - network_ref) - # Retrieve PIFs from network - for pif_ref in network_rec['PIFs']: - # Retrieve VLAN from PIF - pif_rec = self._session.call_xenapi('PIF.get_record', - pif_ref) - pif_vlan = int(pif_rec['VLAN']) - # Raise an exception if VLAN != vlan_num - if pif_vlan != vlan_num: - raise Exception(_("PIF %(pif_uuid)s for network " - "%(bridge)s has VLAN id %(pif_vlan)d. " - "Expected %(vlan_num)d"), - {'pif_uuid': pif_rec['uuid'], - 'bridge': bridge, - 'pif_vlan': pif_vlan, - 'vlan_num': vlan_num}) - - return network_ref - - def unplug(self, instance, vif, vm_ref): - super(XenAPIBridgeDriver, self).unplug(instance, vif, vm_ref) - - class XenAPIOpenVswitchDriver(XenVIFDriver): """VIF driver for Open vSwitch with XenAPI.""" diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index ecc9cf4a3781..fa52bd9f6410 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -58,6 +58,7 @@ from nova.virt import driver as virt_driver from nova.virt import firewall from nova.virt.xenapi import agent as xapi_agent from nova.virt.xenapi.image import utils as image_utils +from nova.virt.xenapi import vif as xapi_vif from nova.virt.xenapi import vm_utils from nova.virt.xenapi import volume_utils from nova.virt.xenapi import volumeops @@ -147,8 +148,8 @@ class VMOps(object): self.firewall_driver = firewall.load_driver( DEFAULT_FIREWALL_DRIVER, xenapi_session=self._session) - vif_impl = importutils.import_class(CONF.xenserver.vif_driver) - self.vif_driver = vif_impl(xenapi_session=self._session) + self.vif_driver = xapi_vif.XenAPIOpenVswitchDriver( + xenapi_session=self._session) self.default_root_dev = '/dev/sda' image_handler_cfg = CONF.xenserver.image_handler diff --git a/releasenotes/notes/remove-xenserver-vif-driver-option-850f8dcfe54bca7c.yaml b/releasenotes/notes/remove-xenserver-vif-driver-option-850f8dcfe54bca7c.yaml new file mode 100644 index 000000000000..8ba02faf8df2 --- /dev/null +++ b/releasenotes/notes/remove-xenserver-vif-driver-option-850f8dcfe54bca7c.yaml @@ -0,0 +1,11 @@ +--- +upgrade: + - | + The ``[xenserver]/vif_driver`` configuration option was deprecated in + the 15.0.0 Ocata release and has now been removed. The only supported + vif driver is now ``XenAPIOpenVswitchDriver`` used with Neutron as the + backend networking service configured to run the + ``neutron-openvswitch-agent`` service. See the `XenServer configuration + guide`_ for more details on networking setup. + + .. _XenServer configuration guide: https://docs.openstack.org/nova/latest/admin/configuration/hypervisor-xen-api.html