Fix removing fwaas when upgrading from Ussuri to Victoria

Without this fix, upgrading from Ussuri to Victoria won't
fully remove fwaas everywhere, leading to broken floating
IPs.

Fixes https://github.com/openstack-charmers/charmed-openstack-tester/issues/57

Change-Id: I959e8f8cafc6d4040895762c21f8ad8b6f190a3b

hooks/neutron_api_context.py

@@ -284,17 +284,28 @@ def is_port_forwarding_enabled():
     return False
 
 
-def is_fwaas_enabled():
+def is_fwaas_enabled(cmp_release=None):
     """
     Check if Firewall as a service feature should be enabled.
 
-    returns: True if enable-fwaas config item is True,
-        otherwise False.
+    This is True if both the corresponding config option is True and the
+    provided OpenStack release supports this feature.
+
+    :param cmp_release: OpenStack release to assess. Defaults to current
+                        release.
+    :type cmp_release: CompareOpenStackReleases
     :rtype: boolean
     """
     if config('enable-fwaas'):
 
-        cmp_release = CompareOpenStackReleases(os_release('neutron-server'))
+        if cmp_release is None:
+            # NOTE(lourot): This may be called from the config-changed hook,
+            # while performing an OpenStack upgrade. Thus we need to use
+            # reset_cache, otherwise os_release() won't return the new
+            # OpenStack release we have just upgraded to.
+            cmp_release = CompareOpenStackReleases(
+                os_release('neutron-server', reset_cache=True))
+
         if cmp_release < 'stein' or cmp_release > 'ussuri':
             log("The fwaas option is set to true but will be ignored "
                 "and disabled for releases outside of Stein to Ussuri.",
@@ -442,7 +453,7 @@ class NeutronCCContext(context.NeutronContext):
 
         plugins = plugin_defs[last_available]
 
-        if not config('enable-fwaas'):
+        if not is_fwaas_enabled(cmp_release):
             filtered = []
             for plugin in plugins:
                 if plugin == 'firewall' or plugin == 'firewall_v2':
@@ -500,8 +511,14 @@ class NeutronCCContext(context.NeutronContext):
         ctxt['tenant_network_types'] = self.neutron_tenant_network_types
         ctxt['overlay_network_type'] = self.neutron_overlay_network_type
         ctxt['external_network'] = config('neutron-external-network')
-        release = os_release('neutron-server')
+
+        # NOTE(lourot): This may be called from the config-changed hook, while
+        # performing an OpenStack upgrade. Thus we need to use reset_cache,
+        # otherwise os_release() won't return the new OpenStack release we
+        # have just upgraded to.
+        release = os_release('neutron-server', reset_cache=True)
         cmp_release = CompareOpenStackReleases(release)
+
         ctxt['enable_igmp_snooping'] = self.neutron_igmp_snoop
         if config('neutron-plugin') == 'vsp' and cmp_release < 'newton':
             _config = config()
@@ -686,9 +703,8 @@ class NeutronCCContext(context.NeutronContext):
                     # TODO(fnordahl): Remove fall-back in next charm release
                     service_plugins[release].append('lbaasv2')
 
-            if config("enable-fwaas"):
-                if cmp_release >= 'stein' and cmp_release <= 'ussuri':
-                    ctxt['firewall_v2'] = True
+            if is_fwaas_enabled(cmp_release):
+                ctxt['firewall_v2'] = True
 
             ctxt['service_plugins'] = self.get_service_plugins(
                 cmp_release, service_plugins)

hooks/neutron_api_utils.py

@@ -502,7 +502,12 @@ def determine_packages(source=None, openstack_release=None):
 
 def determine_purge_packages():
     '''Return a list of packages to purge for the current OS release'''
-    cmp_os_source = CompareOpenStackReleases(os_release('neutron-common'))
+    # NOTE(lourot): This may be called from the config-changed hook, while
+    # performing an OpenStack upgrade. Thus we need to use reset_cache,
+    # otherwise os_release() won't return the new OpenStack release we have
+    # just upgraded to.
+    cmp_os_source = CompareOpenStackReleases(os_release('neutron-common',
+                                                        reset_cache=True))
     purge_pkgs = PURGE_PACKAGES
     if cmp_os_source >= 'victoria':
         purge_pkgs += PURGE_EXTRA_PACKAGES_ON_TRAIN

unit_tests/test_neutron_api_utils.py

@@ -49,6 +49,7 @@ TO_PATCH = [
     'lsb_release',
     'neutron_plugin_attribute',
     'os_release',
+    'reset_os_release',
     'service_restart',
     'subprocess',
     'is_elected_leader',
@@ -376,6 +377,7 @@ class TestNeutronAPIUtils(CharmTestCase):
         self.apt_upgrade.assert_called_with(options=dpkg_opts,
                                             fatal=True,
                                             dist=True)
+        self.reset_os_release.assert_called_with()
         pkgs = nutils.determine_packages()
         pkgs.sort()
         self.apt_install.assert_called_with(packages=pkgs,
@@ -507,7 +509,7 @@ class TestNeutronAPIUtils(CharmTestCase):
         self.get_os_codename_install_source.return_value = 'juno'
         configs = MagicMock()
         nutils.do_openstack_upgrade(configs)
-        self.os_release.assert_called_with('neutron-common')
+        self.os_release.assert_called_with('neutron-common', reset_cache=True)
         self.assertTrue(self.log.called)
         self.add_source.assert_called_with('cloud:trusty-juno')
         self.apt_update.assert_called_with(fatal=True)