Fix charm upgrade broken by commit 862c362

Currently, upgrading this charm on a host that is running
ovs >= 2.6 will break because the OVS_DEFAULT config file
is not expected to be written by the charm.

Change-Id: I33352deb3b60231347045d5f39f3508a29dda61e

hooks/neutron_ovs_hooks.py

@@ -62,21 +62,25 @@ def install():
 # 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
+    if OVS_DEFAULT in restart_map():
-    # /etc/default/openvswitch-switch file only when dpdk was enabled to always
+        # In the 16.10 release of the charms, the code changed from managing
-    # managing this file. Thus, an upgrade of the charm from a release prior
+        # the /etc/default/openvswitch-switch file only when dpdk was enabled
-    # to 16.10 or higher will always cause the contents of the file to change
+        # to always managing this file. Thus, an upgrade of the charm from a
-    # and will trigger a restart of the openvswitch-switch service, which in
+        # release prior to 16.10 or higher will always cause the contents of
-    # turn causes a temporary network outage. To prevent this outage, determine
+        # the file to change and will trigger a restart of the
-    # if the /etc/default/openvswitch-switch file needs to be migrated and if
+        # openvswitch-switch service, which in turn causes a temporary
-    # so, migrate the file but do NOT restart the openvswitch-switch service.
+        # network outage. To prevent this outage, determine if the
-    # See bug LP #1712444
+        # /etc/default/openvswitch-switch file needs to be migrated and if
-    with open(OVS_DEFAULT, 'r') as f:
+        # so, migrate the file but do NOT restart the openvswitch-switch
-        # The 'Service restart triggered ...' line was added to the OVS_DEFAULT
+        # service.
-        # template in the 16.10 version of the charm to allow restarts so we
+        # See bug LP #1712444
-        # use this as the key to see if the file needs migrating.
+        with open(OVS_DEFAULT, 'r') as f:
-        if 'Service restart triggered' not in f.read():
+            # The 'Service restart triggered ...' line was added to the
-            CONFIGS.write(OVS_DEFAULT)
+            # 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/neutron_ovs_utils.py

@@ -309,7 +309,8 @@ def resource_map():
         )
         if not use_dpdk():
             drop_config.append(DPDK_INTERFACES)
-        if ovs_has_late_dpdk_init():
+            drop_config.append(OVS_DEFAULT)
+        elif ovs_has_late_dpdk_init():
             drop_config.append(OVS_DEFAULT)
     else:
         drop_config.extend([OVS_CONF, DPDK_INTERFACES])

unit_tests/test_neutron_ovs_hooks.py

@@ -67,8 +67,9 @@ class NeutronOVSHooksTests(CharmTestCase):
         self._call_hook('install')
         self.install_packages.assert_called_with()
 
+    @patch.object(hooks, 'restart_map')
     @patch.object(hooks, 'restart_on_change')
-    def test_migrate_ovs_default_file(self, mock_restart):
+    def test_migrate_ovs_default_file(self, mock_restart, mock_restart_map):
         # 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.
@@ -82,14 +83,22 @@ class NeutronOVSHooksTests(CharmTestCase):
             with open('unit_tests/%s' % sample, 'r') as f:
                 content = f.read()
 
-            with patch('builtins.open', mock_open(read_data=content),
+            for ovs_default_exists in [False, True]:
-                       create=True):
+                if ovs_default_exists:
-                self._call_hook('upgrade-charm')
+                    mock_restart_map.return_value = {utils.OVS_DEFAULT: {}}
-                if should_migrate:
-                    self.CONFIGS.write.assert_called_with(utils.OVS_DEFAULT)
                 else:
-                    self.CONFIGS.write.assert_not_called()
+                    mock_restart_map.return_value = {}
-                self.assertEqual(0, mock_restart.call_count)
+
+                with patch('builtins.open', mock_open(read_data=content),
+                           create=True):
+                    self.CONFIGS.write.reset_mock()
+                    self._call_hook('upgrade-charm')
+                    if should_migrate and ovs_default_exists:
+                        self.CONFIGS.write.assert_called_with(
+                            utils.OVS_DEFAULT)
+                    else:
+                        self.CONFIGS.write.assert_not_called()
+                    self.assertEqual(0, mock_restart.call_count)
 
     def test_config_changed_dvr(self):
         self._call_hook('config-changed')

unit_tests/test_neutron_ovs_utils.py

@@ -256,7 +256,6 @@ class TestNeutronOVSUtils(CharmTestCase):
         _regconfs = nutils.register_configs()
         confs = ['/etc/neutron/neutron.conf',
                  '/etc/neutron/plugins/ml2/openvswitch_agent.ini',
-                 '/etc/default/openvswitch-switch',
                  '/etc/init/os-charm-phy-nic-mtu.conf']
         self.assertEqual(_regconfs.configs, confs)
 
@@ -331,11 +330,11 @@ class TestNeutronOVSUtils(CharmTestCase):
     def test_resource_map_dhcp(self, _use_dvr, _enable_local_dhcp):
         _enable_local_dhcp.return_value = True
         _use_dvr.return_value = False
-        self.os_release.return_value = 'diablo'
+        self.os_release.return_value = 'mitaka'
-        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'lucid'}
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
         _map = nutils.resource_map()
-        svcs = ['neutron-plugin-openvswitch-agent', 'neutron-metadata-agent',
+        svcs = ['neutron-metadata-agent', 'neutron-dhcp-agent',
-                'neutron-dhcp-agent']
+                'neutron-openvswitch-agent']
         confs = [nutils.NEUTRON_CONF, nutils.NEUTRON_METADATA_AGENT_CONF,
                  nutils.NEUTRON_DHCP_AGENT_CONF]
         [self.assertIn(q_conf, _map.keys()) for q_conf in confs]
@@ -367,20 +366,20 @@ class TestNeutronOVSUtils(CharmTestCase):
         _map = nutils.resource_map()
         self.assertFalse(nutils.EXT_PORT_CONF in _map.keys())
 
+    @patch.object(nutils, 'use_dpdk')
     @patch.object(nutils, 'use_dvr')
-    def test_restart_map(self, _use_dvr):
+    def test_restart_map(self, mock_use_dvr, mock_use_dpdk):
-        _use_dvr.return_value = False
+        mock_use_dvr.return_value = False
-        self.os_release.return_value = "diablo"
+        mock_use_dpdk.return_value = False
-        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'lucid'}
+        self.os_release.return_value = "mitaka"
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
+        ML2CONF = "/etc/neutron/plugins/ml2/openvswitch_agent.ini"
         _restart_map = nutils.restart_map()
-        ML2CONF = "/etc/neutron/plugins/ml2/ml2_conf.ini"
         expect = OrderedDict([
-            (nutils.NEUTRON_CONF, ['neutron-plugin-openvswitch-agent']),
+            (ML2CONF, ['neutron-openvswitch-agent']),
-            (ML2CONF, ['neutron-plugin-openvswitch-agent']),
+            (nutils.NEUTRON_CONF, ['neutron-openvswitch-agent']),
-            (nutils.OVS_DEFAULT, ['openvswitch-switch']),
-            (nutils.PHY_NIC_MTU_CONF, ['os-charm-phy-nic-mtu'])
         ])
-        self.assertEqual(expect, _restart_map)
+        self.assertEqual(expect, OrderedDict(_restart_map))
         for item in _restart_map:
             self.assertTrue(item in _restart_map)
             self.assertTrue(expect[item] == _restart_map[item])