diff --git a/config.yaml b/config.yaml index a593f518..d9ef9cce 100644 --- a/config.yaml +++ b/config.yaml @@ -212,6 +212,10 @@ options: Open vSwitch networking options. . NOTE: This configuration option will be ignored on Trusty and older. + . + NOTE: Changing this value will have no effect on runtime configuration. A + manual restart of the `sriov-netplan-shim` service or reboot of the + system is required to apply configuration. sriov-device-mappings: type: string default: @@ -259,6 +263,10 @@ options: supported by OVS or the Linux kernel). . This option must not be enabled with either enable-sriov or enable-dpdk. + . + NOTE: Changing this value will not perform hardware specific adaption. A + manual restart of the hardware specific adaption service or reboot of the + system is required to apply configuration. networking-tools-source: type: string default: ppa:openstack-charmers/networking-tools diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index 524ab412..638377f3 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/hooks/neutron_ovs_hooks.py @@ -55,7 +55,6 @@ from neutron_ovs_utils import ( OVS_DEFAULT, USE_FQDN_KEY, configure_ovs, - configure_sriov, get_shared_secret, register_configs, restart_map, @@ -71,6 +70,7 @@ from neutron_ovs_utils import ( pause_unit_helper, resume_unit_helper, determine_purge_packages, + purge_sriov_systemd_files, use_fqdn_hint, ) @@ -95,6 +95,10 @@ def install(): # for the implications of modifications to the /etc/default/openvswitch-switch. @hooks.hook('upgrade-charm') def upgrade_charm(): + # Tidy up any prior installation of obsolete sriov startup + # scripts + purge_sriov_systemd_files() + if OVS_DEFAULT in restart_map(): # In the 16.10 release of the charms, the code changed from managing # the /etc/default/openvswitch-switch file only when dpdk was enabled @@ -159,10 +163,6 @@ def config_changed(): _restart_before_runtime_config_when_required() configure_ovs() - # NOTE(fnordahl): configure_sriov must be run after CONFIGS.write_all() - # to allow us to enable boot time execution of init script - configure_sriov() - for rid in relation_ids('neutron-plugin'): neutron_plugin_joined( relation_id=rid, diff --git a/hooks/neutron_ovs_utils.py b/hooks/neutron_ovs_utils.py index 3e071d41..c0a9208c 100644 --- a/hooks/neutron_ovs_utils.py +++ b/hooks/neutron_ovs_utils.py @@ -18,7 +18,6 @@ import os from itertools import chain import shutil import subprocess -import yaml from charmhelpers.contrib.openstack.neutron import neutron_plugin_attribute from copy import deepcopy @@ -30,7 +29,6 @@ from charmhelpers.contrib.openstack.utils import ( make_assess_status_func, is_unit_paused_set, os_application_version_set, - remote_restart, CompareOpenStackReleases, os_release, ) @@ -75,7 +73,6 @@ from charmhelpers.core.host import ( group_exists, user_exists, is_container, - restart_on_change ) from charmhelpers.core.kernel import ( modprobe, @@ -92,8 +89,6 @@ from charmhelpers.fetch import ( add_source, ) -from pci import PCINetDevices - # The interface is said to be satisfied if anyone of the interfaces in the # list has a complete context. @@ -145,6 +140,7 @@ DPDK_INTERFACES = '/etc/dpdk/interfaces' NEUTRON_SRIOV_AGENT_CONF = os.path.join(NEUTRON_CONF_DIR, 'plugins/ml2/sriov_agent.ini') USE_FQDN_KEY = 'neutron-ovs-charm-use-fqdn' +SRIOV_NETPLAN_SHIM_CONF = '/etc/sriov-netplan-shim/interfaces.yaml' def use_fqdn_hint(): @@ -417,6 +413,30 @@ def resource_map(): resource_map.update(sriov_resource_map) resource_map[NEUTRON_CONF]['services'].append( sriov_agent_name) + if enable_sriov() or use_hw_offload(): + # We do late initialization of this as a call to + # ``context.SRIOVContext`` requires the ``sriov-netplan-shim`` package + # to already be installed on the system. + # + # Note that we also do not want the charm to manage the service, but + # only update the configuration for boot-time initialization. + # LP: #1908351 + try: + resource_map.update(OrderedDict([ + (SRIOV_NETPLAN_SHIM_CONF, { + # We deliberately omit service here as we only want changes + # to be applied at boot time. + 'services': [], + 'contexts': [SRIOVContext_adapter()], + }), + ])) + except NameError: + # The resource_map is built at module import time and as such this + # function is called multiple times prior to the charm actually + # being installed. As the SRIOVContext depends on a Python module + # provided by the ``sriov-netplan-shim`` package gracefully ignore + # this to allow the package to be installed. + pass # Use MAAS1.9 for MTU and external port config on xenial and above if CompareHostReleases(lsb_release()['DISTRIB_CODENAME']) >= 'xenial': @@ -719,134 +739,6 @@ def _get_interfaces_from_mappings(sriov_mappings): return interfaces -SRIOV_NETPLAN_SHIM_CONF = '/etc/sriov-netplan-shim/interfaces.yaml' - - -# TODO(jamespage) -# Drop all of this once MAAS and netplan can actually do SR-IOV -# configuration natively. -def configure_sriov(): - '''Configure SR-IOV devices based on provided configuration options - - This function writes out the configuration file for sriov-netplan-shim - which actually takes care of SR-IOV VF device configuration both - during configuration and post reboot. - ''' - @restart_on_change({SRIOV_NETPLAN_SHIM_CONF: ['sriov-netplan-shim']}) - def _write_interfaces_yaml(): - charm_config = config() - devices = PCINetDevices() - sriov_numvfs = charm_config.get('sriov-numvfs') - section = 'interfaces' - interfaces_yaml = { - section: {} - } - numvfs_changed = False - # automatic configuration of all SR-IOV devices - if sriov_numvfs == 'auto': - interfaces = _get_interfaces_from_mappings( - charm_config.get('sriov-device-mappings')) - log('Configuring SR-IOV device VF functions in auto mode') - for device in devices.pci_devices: - if device and device.sriov: - if interfaces and device.interface_name not in interfaces: - log("Excluding configuration of SR-IOV" - " device {}.".format(device.interface_name)) - continue - log("Configuring SR-IOV device" - " {} with {} VF's".format(device.interface_name, - device.sriov_totalvfs)) - interfaces_yaml[section][device.interface_name] = { - 'num_vfs': int(device.sriov_totalvfs) - } - numvfs_changed = ( - (device.sriov_totalvfs != device.sriov_numvfs) or - numvfs_changed - ) - else: - # Single int blanket configuration - try: - log('Configuring SR-IOV device VF functions' - ' with blanket setting') - for device in devices.pci_devices: - if device and device.sriov: - numvfs = min(int(sriov_numvfs), device.sriov_totalvfs) - if int(sriov_numvfs) > device.sriov_totalvfs: - log('Requested value for sriov-numvfs ({}) too ' - 'high for interface {}. Falling back to ' - 'interface totalvfs ' - 'value: {}'.format(sriov_numvfs, - device.interface_name, - device.sriov_totalvfs)) - log("Configuring SR-IOV device {} with {} " - "VFs".format(device.interface_name, numvfs)) - interfaces_yaml[section][device.interface_name] = { - 'num_vfs': numvfs - } - numvfs_changed = ( - (numvfs != device.sriov_numvfs) or - numvfs_changed - ) - except ValueError: - # :[ :numvfs] configuration - sriov_numvfs = sriov_numvfs.split() - for device_config in sriov_numvfs: - log('Configuring SR-IOV device VF functions per interface') - interface_name, numvfs = device_config.split(':') - device = devices.get_device_from_interface_name( - interface_name) - if device and device.sriov: - if int(numvfs) > device.sriov_totalvfs: - log('Requested value for sriov-numfs ({}) too ' - 'high for interface {}. Falling back to ' - 'interface totalvfs ' - 'value: {}'.format(numvfs, - device.interface_name, - device.sriov_totalvfs)) - numvfs = device.sriov_totalvfs - log("Configuring SR-IOV device {} with {} " - "VF's".format(device.interface_name, numvfs)) - interfaces_yaml[section][device.interface_name] = { - 'num_vfs': int(numvfs) - } - numvfs_changed = ( - (numvfs != device.sriov_numvfs) or - numvfs_changed - ) - with open(SRIOV_NETPLAN_SHIM_CONF, 'w') as _conf: - yaml.dump(interfaces_yaml, _conf) - return numvfs_changed - - # NOTE(jamespage) - # Hardware offload makes use of SR-IOV VF representor ports so - # makes use of the general SR-IOV configuration support in this - # charm - if not (enable_sriov() or use_hw_offload()): - return - - # Tidy up any prior installation of obsolete sriov startup - # scripts - purge_sriov_systemd_files() - - numvfs_changed = _write_interfaces_yaml() - - # Trigger remote restart in parent application - remote_restart('neutron-plugin', 'nova-compute') - - if not use_hw_offload() and numvfs_changed: - # Restart of SRIOV agent is required after changes to system runtime - # VF configuration - cmp_release = CompareOpenStackReleases( - os_release('neutron-common', base='icehouse')) - if cmp_release >= 'mitaka': - service_restart('neutron-sriov-agent') - else: - service_restart('neutron-plugin-sriov-agent') - - if use_hw_offload() and numvfs_changed: - service_restart('mlnx-switchdev-mode') - - def get_shared_secret(): ctxt = neutron_ovs_context.SharedSecretContext()() if 'shared_secret' in ctxt: @@ -918,6 +810,21 @@ def enable_sriov(): return (cmp_release >= 'xenial' and config('enable-sriov')) +class SRIOVContext_adapter(object): + """Adapt the SRIOVContext for use in a classic charm. + + :returns: Dictionary with entry point to context map. + :rtype: Dict[str,SRIOVContext] + """ + interfaces = [] + + def __init__(self): + self._sriov_device = context.SRIOVContext() + + def __call__(self): + return {'sriov_device': self._sriov_device} + + # TODO: update into charm-helpers to add port_type parameter def dpdk_add_bridge_port(name, port, pci_address=None): ''' Add a port to the named openvswitch bridge ''' diff --git a/templates/mitaka/interfaces.yaml b/templates/mitaka/interfaces.yaml new file mode 100644 index 00000000..c7a27d24 --- /dev/null +++ b/templates/mitaka/interfaces.yaml @@ -0,0 +1,7 @@ +interfaces: + {% for _, pcidnvfs in sriov_device.get_map.items() -%} + {{ pcidnvfs.device.interface_name }}: + match: + pciaddress: '{{ pcidnvfs.device.pci_address }}' + num_vfs: {{ pcidnvfs.numvfs }} + {% endfor -%} diff --git a/unit_tests/test_neutron_ovs_hooks.py b/unit_tests/test_neutron_ovs_hooks.py index cd4c0d42..d3f17cc2 100644 --- a/unit_tests/test_neutron_ovs_hooks.py +++ b/unit_tests/test_neutron_ovs_hooks.py @@ -41,7 +41,6 @@ TO_PATCH = [ 'relation_ids', 'relation_set', 'configure_ovs', - 'configure_sriov', 'use_dvr', 'use_l3ha', 'install_packages', diff --git a/unit_tests/test_neutron_ovs_utils.py b/unit_tests/test_neutron_ovs_utils.py index b82abf19..17c1ae6a 100644 --- a/unit_tests/test_neutron_ovs_utils.py +++ b/unit_tests/test_neutron_ovs_utils.py @@ -13,9 +13,7 @@ # limitations under the License. import hashlib -import io import subprocess -import yaml from mock import MagicMock, patch, call from collections import OrderedDict @@ -28,7 +26,6 @@ import neutron_ovs_context from test_utils import ( CharmTestCase, - patch_open, ) import charmhelpers import charmhelpers.core.hookenv as hookenv @@ -63,8 +60,6 @@ TO_PATCH = [ 'status_set', 'use_dpdk', 'os_application_version_set', - 'remote_restart', - 'PCINetDevices', 'enable_ipfix', 'disable_ipfix', 'ovs_has_late_dpdk_init', @@ -473,9 +468,11 @@ class TestNeutronOVSUtils(CharmTestCase): [self.assertIn(q_conf, _map.keys()) for q_conf in confs] self.assertEqual(_map[nutils.NEUTRON_CONF]['services'], svcs) + @patch.object(nutils, 'SRIOVContext_adapter') @patch.object(nutils, 'enable_sriov') @patch.object(nutils, 'use_dvr') - def test_resource_map_kilo_sriov(self, _use_dvr, _enable_sriov): + def test_resource_map_kilo_sriov(self, _use_dvr, _enable_sriov, + _sriovcontext_adapter): _use_dvr.return_value = False _enable_sriov.return_value = True self.os_release.return_value = 'kilo' @@ -500,9 +497,11 @@ class TestNeutronOVSUtils(CharmTestCase): [self.assertIn(q_conf, _map.keys()) for q_conf in confs] self.assertEqual(_map[nutils.NEUTRON_CONF]['services'], svcs) + @patch.object(nutils, 'SRIOVContext_adapter') @patch.object(nutils, 'enable_sriov') @patch.object(nutils, 'use_dvr') - def test_resource_map_mitaka_sriov(self, _use_dvr, _enable_sriov): + def test_resource_map_mitaka_sriov(self, _use_dvr, _enable_sriov, + _sriovcontext_adapter): _use_dvr.return_value = False _enable_sriov.return_value = True self.os_release.return_value = 'mitaka' @@ -924,159 +923,6 @@ class TestNeutronOVSUtils(CharmTestCase): # ports=None whilst port checks are disabled. f.assert_called_once_with('assessor', services='s1', ports=None) - def _configure_sriov_base(self, config): - self.mock_config = MagicMock() - self.config.side_effect = None - self.config.return_value = self.mock_config - self.mock_config.get.side_effect = lambda x: config.get(x) - - self.mock_eth_device = MagicMock() - self.mock_eth_device.sriov = False - self.mock_eth_device.interface_name = 'eth0' - self.mock_eth_device.sriov_totalvfs = 0 - - self.mock_sriov_device = MagicMock() - self.mock_sriov_device.sriov = True - self.mock_sriov_device.interface_name = 'ens0' - self.mock_sriov_device.sriov_totalvfs = 64 - - self.mock_sriov_device2 = MagicMock() - self.mock_sriov_device2.sriov = True - self.mock_sriov_device2.interface_name = 'ens49' - self.mock_sriov_device2.sriov_totalvfs = 64 - - self.pci_devices = { - 'eth0': self.mock_eth_device, - 'ens0': self.mock_sriov_device, - 'ens49': self.mock_sriov_device2, - } - - mock_pci_devices = MagicMock() - mock_pci_devices.pci_devices = [ - self.mock_eth_device, - self.mock_sriov_device, - self.mock_sriov_device2 - ] - self.PCINetDevices.return_value = mock_pci_devices - - mock_pci_devices.get_device_from_interface_name.side_effect = \ - lambda x: self.pci_devices.get(x) - - @patch('shutil.copy') - @patch('os.chmod') - def test_configure_sriov_auto(self, _os_chmod, _sh_copy): - self.os_release.return_value = 'mitaka' - self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'} - _config = { - 'enable-sriov': True, - 'sriov-numvfs': 'auto' - } - self._configure_sriov_base(_config) - - with patch_open() as (_open, _file): - mock_stringio = io.StringIO() - _file.write.side_effect = mock_stringio.write - nutils.configure_sriov() - self.assertEqual( - yaml.load(mock_stringio.getvalue()), - { - 'interfaces': { - 'ens0': {'num_vfs': 64}, - 'ens49': {'num_vfs': 64} - } - } - ) - - self.mock_sriov_device.set_sriov_numvfs.assert_not_called() - self.mock_sriov_device2.set_sriov_numvfs.assert_not_called() - self.assertTrue(self.remote_restart.called) - - @patch('shutil.copy') - @patch('os.chmod') - def test_configure_sriov_auto_mapping(self, _os_chmod, _sh_copy): - self.os_release.return_value = 'mitaka' - self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'} - _config = { - 'enable-sriov': True, - 'sriov-numvfs': 'auto', - 'sriov-device-mappings': 'net1:ens49' - } - self._configure_sriov_base(_config) - - with patch_open() as (_open, _file): - mock_stringio = io.StringIO() - _file.write.side_effect = mock_stringio.write - nutils.configure_sriov() - self.assertEqual( - yaml.load(mock_stringio.getvalue()), - { - 'interfaces': { - 'ens49': {'num_vfs': 64} - } - } - ) - - self.mock_sriov_device.set_sriov_numvfs.assert_not_called() - self.mock_sriov_device2.set_sriov_numvfs.assert_not_called() - self.assertTrue(self.remote_restart.called) - - @patch('shutil.copy') - @patch('os.chmod') - def test_configure_sriov_numvfs(self, _os_chmod, _sh_copy): - self.os_release.return_value = 'mitaka' - self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'} - _config = { - 'enable-sriov': True, - 'sriov-numvfs': '32', - } - self._configure_sriov_base(_config) - - with patch_open() as (_open, _file): - mock_stringio = io.StringIO() - _file.write.side_effect = mock_stringio.write - nutils.configure_sriov() - self.assertEqual( - yaml.load(mock_stringio.getvalue()), - { - 'interfaces': { - 'ens0': {'num_vfs': 32}, - 'ens49': {'num_vfs': 32} - } - } - ) - - self.mock_sriov_device.set_sriov_numvfs.assert_not_called() - self.mock_sriov_device2.set_sriov_numvfs.assert_not_called() - self.assertTrue(self.remote_restart.called) - - @patch('shutil.copy') - @patch('os.chmod') - def test_configure_sriov_numvfs_per_device(self, _os_chmod, _sh_copy): - self.os_release.return_value = 'mitaka' - self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'} - _config = { - 'enable-sriov': True, - 'sriov-numvfs': 'ens0:32 sriov23:64' - } - self._configure_sriov_base(_config) - - with patch_open() as (_open, _file): - mock_stringio = io.StringIO() - _file.write.side_effect = mock_stringio.write - nutils.configure_sriov() - self.assertEqual( - yaml.load(mock_stringio.getvalue()), - { - 'interfaces': { - 'ens0': {'num_vfs': 32}, - } - } - ) - - self.mock_sriov_device.set_sriov_numvfs.assert_not_called() - self.mock_sriov_device2.set_sriov_numvfs.assert_not_called() - self.assertTrue(self.remote_restart.called) - @patch.object(nutils, 'subprocess') @patch.object(nutils, 'shutil') def test_install_tmpfilesd_lxd(self, mock_shutil, mock_subprocess):