Merge lp:~corey.bryant/charm-helpers/swift-mitaka into lp:charm-helpers

[Corey Bryant]

[Corey Bryant (corey.bryant)]

The code changes in this mp only target the swift paths so it should have no impact on non-swift charms.

I've tested this code with swift-proxy and swift-storage deploys on icehouse and mitaka-proposed, in addition to upgrades from icehouse->juno->kilo->liberty. Note that an attempt to upgrade from liberty->mitaka remains at the liberty version of swift. I think this makes sense since liberty and mitaka both have 2.5.0 at the moment.

[lp:~corey.bryant/charm-helpers/swift-mitaka]

518. By Corey Bryant on 2016-01-13

Fix lint errors and add unit test updates

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2016-01-08 13:27:39 +0000
+++ charmhelpers/contrib/openstack/utils.py	2016-01-13 21:14:08 +0000
@@ -103,29 +103,28 @@
     ('2016.1', 'mitaka'),
 ])
 
-# The ugly duckling
+# The ugly duckling - must list releases oldest to newest
 SWIFT_CODENAMES = OrderedDict([
-    ('1.4.3', 'diablo'),
-    ('1.4.8', 'essex'),
-    ('1.7.4', 'folsom'),
-    ('1.8.0', 'grizzly'),
-    ('1.7.7', 'grizzly'),
-    ('1.7.6', 'grizzly'),
-    ('1.10.0', 'havana'),
-    ('1.9.1', 'havana'),
-    ('1.9.0', 'havana'),
-    ('1.13.1', 'icehouse'),
-    ('1.13.0', 'icehouse'),
-    ('1.12.0', 'icehouse'),
-    ('1.11.0', 'icehouse'),
-    ('2.0.0', 'juno'),
-    ('2.1.0', 'juno'),
-    ('2.2.0', 'juno'),
-    ('2.2.1', 'kilo'),
-    ('2.2.2', 'kilo'),
-    ('2.3.0', 'liberty'),
-    ('2.4.0', 'liberty'),
-    ('2.5.0', 'liberty'),
+    ('diablo',
+        ['1.4.3']),
+    ('essex',
+        ['1.4.8']),
+    ('folsom',
+        ['1.7.4']),
+    ('grizzly',
+        ['1.7.6', '1.7.7', '1.8.0']),
+    ('havana',
+        ['1.9.0', '1.9.1', '1.10.0']),
+    ('icehouse',
+        ['1.11.0', '1.12.0', '1.13.0', '1.13.1']),
+    ('juno',
+        ['2.0.0', '2.1.0', '2.2.0']),
+    ('kilo',
+        ['2.2.1', '2.2.2']),
+    ('liberty',
+        ['2.3.0', '2.4.0', '2.5.0']),
+    ('mitaka',
+        ['2.5.0']),
 ])
 
 # >= Liberty version->codename mapping
@@ -227,6 +226,33 @@
     error_out(e)
 
 
+def get_os_version_codename_swift(codename):
+    '''Determine OpenStack version number of swift from codename.'''
+    for k, v in six.iteritems(SWIFT_CODENAMES):
+        if k == codename:
+            return v[-1]
+    e = 'Could not derive swift version for '\
+        'codename: %s' % codename
+    error_out(e)
+
+
+def get_swift_codename(version):
+    '''Determine OpenStack codename that corresponds to swift version.'''
+    codenames = [k for k, v in six.iteritems(SWIFT_CODENAMES) if version in v]
+    if len(codenames) > 1:
+        # If more than one release codename contains this version we determine
+        # the actual codename based on the highest available install source.
+        for codename in reversed(codenames):
+            releases = UBUNTU_OPENSTACK_RELEASE
+            release = [k for k, v in six.iteritems(releases) if codename in v]
+            ret = subprocess.check_output(['apt-cache', 'policy', 'swift'])
+            if codename in ret or release[0] in ret:
+                return codename
+    elif len(codenames) == 1:
+        return codenames[0]
+    return None
+
+
 def get_os_codename_package(package, fatal=True):
     '''Derive OpenStack release codename from an installed package.'''
     import apt_pkg as apt
@@ -270,7 +296,7 @@
         # < Liberty co-ordinated project versions
         try:
             if 'swift' in pkg.name:
-                return SWIFT_CODENAMES[vers]
+                return get_swift_codename(vers)
             else:
                 return OPENSTACK_CODENAMES[vers]
         except KeyError:
@@ -289,12 +315,14 @@
 
     if 'swift' in pkg:
         vers_map = SWIFT_CODENAMES
+        for cname, version in six.iteritems(vers_map):
+            if cname == codename:
+                return version[-1]
     else:
         vers_map = OPENSTACK_CODENAMES
-
-    for version, cname in six.iteritems(vers_map):
-        if cname == codename:
-            return version
+        for version, cname in six.iteritems(vers_map):
+            if cname == codename:
+                return version
     # e = "Could not determine OpenStack version for package: %s" % pkg
     # error_out(e)
 
@@ -460,11 +488,16 @@
     cur_vers = get_os_version_package(package)
     if "swift" in package:
         codename = get_os_codename_install_source(src)
-        available_vers = get_os_version_codename(codename, SWIFT_CODENAMES)
+        avail_vers = get_os_version_codename_swift(codename)
     else:
-        available_vers = get_os_version_install_source(src)
+        avail_vers = get_os_version_install_source(src)
     apt.init()
-    return apt.version_compare(available_vers, cur_vers) == 1
+    if "swift" in package:
+        major_cur_vers = cur_vers.split('.', 1)[0]
+        major_avail_vers = avail_vers.split('.', 1)[0]
+        major_diff = apt.version_compare(major_avail_vers, major_cur_vers)
+        return avail_vers > cur_vers and (major_diff == 1 or major_diff == 0)
+    return apt.version_compare(avail_vers, cur_vers) == 1
 
 
 def ensure_block_device(block_device):

=== modified file 'tests/contrib/openstack/test_openstack_utils.py'
--- tests/contrib/openstack/test_openstack_utils.py	2016-01-08 13:27:39 +0000
+++ tests/contrib/openstack/test_openstack_utils.py	2016-01-13 21:14:08 +0000
@@ -248,6 +248,34 @@
         expected_err = 'Could not derive OpenStack version for codename: foo'
         mocked_error.assert_called_with(expected_err)
 
+    def test_os_version_swift_from_codename(self):
+        '''Test mapping a swift codename to numerical version'''
+        self.assertEquals(openstack.get_os_version_codename_swift('liberty'),
+                          '2.5.0')
+
+    def test_get_swift_codename_single_version_kilo(self):
+        self.assertEquals(openstack.get_swift_codename('2.2.2'), 'kilo')
+
+    @patch('charmhelpers.contrib.openstack.utils.error_out')
+    def test_os_version_swift_from_bad_codename(self, mocked_error):
+        '''Test mapping a bad swift codename to numerical version'''
+        openstack.get_os_version_codename_swift('foo')
+        expected_err = 'Could not derive swift version for codename: foo'
+        mocked_error.assert_called_with(expected_err)
+
+    def test_get_swift_codename_multiple_versions_liberty(self):
+        with patch('subprocess.check_output') as _subp:
+            _subp.return_value = "... trusty-updates/liberty/main ..."
+            self.assertEquals(openstack.get_swift_codename('2.5.0'), 'liberty')
+
+    def test_get_swift_codename_multiple_versions_mitaka(self):
+        with patch('subprocess.check_output') as _subp:
+            _subp.return_value = "... trusty-updates/mitaka/main ..."
+            self.assertEquals(openstack.get_swift_codename('2.5.0'), 'mitaka')
+
+    def test_get_swift_codename_none(self):
+        self.assertEquals(openstack.get_swift_codename('1.2.3'), None)
+
     def test_os_codename_from_package(self):
         '''Test deriving OpenStack codename from an installed package'''
         with patch('apt_pkg.Cache') as cache:
@@ -507,8 +535,10 @@
 
     @patch.object(openstack, 'lsb_release')
     @patch.object(openstack, 'get_os_version_package')
+    @patch.object(openstack, 'get_os_version_codename_swift')
     @patch.object(openstack, 'config')
-    def test_openstack_upgrade_detection_true(self, config, vers_pkg, lsb):
+    def test_openstack_upgrade_detection_true(self, config, vers_swift,
+                                              vers_pkg, lsb):
         """Test it detects when an openstack package has available upgrade"""
         lsb.return_value = FAKE_RELEASE
         config.return_value = 'cloud:precise-havana'
@@ -518,6 +548,7 @@
         vers_pkg.return_value = '2013.2~b1'
         self.assertTrue(openstack.openstack_upgrade_available('nova-common'))
         vers_pkg.return_value = '1.9.0'
+        vers_swift.return_value = '2.5.0'
         self.assertTrue(openstack.openstack_upgrade_available('swift-proxy'))
 
     @patch.object(openstack, 'lsb_release')