Merge "Migrate openvswitch-switch file to avoid restarts"

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())

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'

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: 
+#                                                  
+###############################################################################

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'

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