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

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