diff --git a/hooks/neutron_ovs_hooks.py b/hooks/neutron_ovs_hooks.py index d82774ab..bee98f9c 100755 --- a/hooks/neutron_ovs_hooks.py +++ b/hooks/neutron_ovs_hooks.py @@ -38,6 +38,7 @@ from neutron_ovs_utils import ( DHCP_PACKAGES, DVR_PACKAGES, METADATA_PACKAGES, + OVS_DEFAULT, configure_ovs, configure_sriov, git_install, @@ -63,6 +64,27 @@ def install(): git_install(config('openstack-origin-git')) +# NOTE(wolsen): Do NOT add restart_on_change decorator without consideration +# for the implications of modifications to the /etc/default/openvswitch-switch. +@hooks.hook('upgrade-charm') +def upgrade_charm(): + # In the 16.10 release of the charms, the code changed from managing the + # /etc/default/openvswitch-switch file only when dpdk was enabled to always + # managing this file. Thus, an upgrade of the charm from a release prior + # to 16.10 or higher will always cause the contents of the file to change + # and will trigger a restart of the openvswitch-switch service, which in + # turn causes a temporary network outage. To prevent this outage, determine + # if the /etc/default/openvswitch-switch file needs to be migrated and if + # so, migrate the file but do NOT restart the openvswitch-switch service. + # See bug LP #1712444 + with open(OVS_DEFAULT, 'r') as f: + # The 'Service restart triggered ...' line was added to the OVS_DEFAULT + # template in the 16.10 version of the charm to allow restarts so we + # use this as the key to see if the file needs migrating. + if 'Service restart triggered' not in f.read(): + CONFIGS.write(OVS_DEFAULT) + + @hooks.hook('neutron-plugin-relation-changed') @hooks.hook('config-changed') @restart_on_change(restart_map()) diff --git a/unit_tests/16.07-dpdk-openvswitch-switch b/unit_tests/16.07-dpdk-openvswitch-switch new file mode 100644 index 00000000..f5937c6e --- /dev/null +++ b/unit_tests/16.07-dpdk-openvswitch-switch @@ -0,0 +1,8 @@ +# This is a POSIX shell fragment -*- sh -*- +############################################################################### +# [ WARNING ] +# Configuration file maintained by Juju. Local changes may be overwritten. +# Configuration managed by neutron-openvswitch charm +############################################################################### + +DPDK_OPTS='--dpdk -c 0x1 -n 4 --socket-mem 1024 -w 00:01:00.0 --vhost-owner libvirt-qemu:kvm --vhost-perm 0660' diff --git a/unit_tests/16.10-openvswitch-switch b/unit_tests/16.10-openvswitch-switch new file mode 100644 index 00000000..4659a58e --- /dev/null +++ b/unit_tests/16.10-openvswitch-switch @@ -0,0 +1,8 @@ +# This is a POSIX shell fragment -*- sh -*- +############################################################################### +# [ WARNING ] +# Configuration file maintained by Juju. Local changes may be overwritten. +# Configuration managed by neutron-openvswitch charm +# Service restart triggered by remote application: +# +############################################################################### diff --git a/unit_tests/package-provided-openvswitch-switch b/unit_tests/package-provided-openvswitch-switch new file mode 100644 index 00000000..86f7c9ea --- /dev/null +++ b/unit_tests/package-provided-openvswitch-switch @@ -0,0 +1,11 @@ +# This is a POSIX shell fragment -*- sh -*- + +# FORCE_COREFILES: If 'yes' then core files will be enabled. +# FORCE_COREFILES=yes + +# OVS_CTL_OPTS: Extra options to pass to ovs-ctl. This is, for example, +# a suitable place to specify --ovs-vswitchd-wrapper=valgrind. +# OVS_CTL_OPTS= + +# DPDK options - see /usr/share/doc/openvswitch-common/INSTALL.DPDK.md.gz +# DPDK_OPTS='--dpdk -c 0x1 -n 4' diff --git a/unit_tests/test_neutron_ovs_hooks.py b/unit_tests/test_neutron_ovs_hooks.py index 26b596c7..714838b5 100644 --- a/unit_tests/test_neutron_ovs_hooks.py +++ b/unit_tests/test_neutron_ovs_hooks.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from mock import MagicMock, patch +from mock import MagicMock, patch, mock_open import yaml from test_utils import CharmTestCase @@ -91,6 +91,30 @@ class NeutronOVSHooksTests(CharmTestCase): self.install_packages.assert_called_with() self.git_install.assert_called_with(projects_yaml) + @patch.object(hooks, 'restart_on_change') + def test_migrate_ovs_default_file(self, mock_restart): + # Tests that the /etc/default/openvswitch-switch file is/isn't + # migrated on the upgrade-charm hook and that no restarts are + # attempted of the openvswitch-switch service. + tests = [ + ('package-provided-openvswitch-switch', True), + ('16.07-dpdk-openvswitch-switch', True), + ('16.10-openvswitch-switch', False), + ] + for sample, should_migrate in tests: + self.CONFIGS.write.reset_mock() + with open('unit_tests/%s' % sample, 'r') as f: + content = f.read() + + with patch('__builtin__.open', mock_open(read_data=content), + create=True): + self._call_hook('upgrade-charm') + if should_migrate: + self.CONFIGS.write.assert_called_with(utils.OVS_DEFAULT) + else: + self.CONFIGS.write.assert_not_called() + self.assertEqual(0, mock_restart.call_count) + @patch.object(hooks, 'git_install_requested') def test_config_changed(self, git_requested): git_requested.return_value = False