From 69e4eb8b00c9792d06f636724a2369b8bf7af092 Mon Sep 17 00:00:00 2001 From: Billy Olsen Date: Tue, 22 Aug 2017 19:19:06 -0700 Subject: [PATCH] Migrate openvswitch-switch file to avoid restarts The 16.10 release of the neutron-openvswitch charm changed the file management strategy of /etc/default/openvswitch-switch config file from managing the file when dpdk is enabled to always managing the file. Additionally, the template file was changed in the 16.10 release to modify the file header (commit 4463c334). These two changes guarantee that the contents of the file will change when upgrading the charm. The changing file contents causes the openvswitch-switch service to be restarted, which in turn causes a data plane outage. This commit fixes that by migrating the /etc/default/openvswitch-switch to be charm managed without restarting the openvswitch-switch service. The change will only attempt to migrate versions of the file which were created before 16.10 by searching for a marker in the rendered version of the file which was added in 16.10. Change-Id: Icc0f326991be239b88a57292740473f501181ebb Closes-Bug: #1712444 (cherry picked from commit e3ec31c91df72cf64f79f14e0223ec61382c0553) --- hooks/neutron_ovs_hooks.py | 22 ++++++++++++++++ unit_tests/16.07-dpdk-openvswitch-switch | 8 ++++++ unit_tests/16.10-openvswitch-switch | 8 ++++++ .../package-provided-openvswitch-switch | 11 ++++++++ unit_tests/test_neutron_ovs_hooks.py | 26 ++++++++++++++++++- 5 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 unit_tests/16.07-dpdk-openvswitch-switch create mode 100644 unit_tests/16.10-openvswitch-switch create mode 100644 unit_tests/package-provided-openvswitch-switch 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