Merge lp:~gandelman-a/charm-helpers/openstack_utils into lp:charm-helpers

[Adam Gandelman]

Add contrib.opentstack.openstack_utils.openstack_upgrade_available()
Allow for non-fatal querying of openstack packages that are not installed
Adds relevant tests.

Adds a utility openstack_upgrade_available() to check if an upgrade is available via the configured installation source. Requires an update to the version querying utilities to be optionally non-fatal.

[James Page (james-page)]

Minor lint:

tests/contrib/openstack/test_openstack_utils.py:60:38: E241 multiple spaces after ','
tests/contrib/openstack/test_openstack_utils.py:373:1: E303 too many blank lines (3)
make: *** [lint] Error 1

review: Needs Fixing

[James Page (james-page)]

Please can we switch to core/host for juju log calls:

def juju_log(msg):
    subprocess.check_call(['juju-log', msg])

This is duplicated code now - test can also be dropped.

This is not a blocker right now but it would be great if configure_installation_source could be refactored to use more of the approx same code in the fetch helper.

review: Needs Fixing

[James Page (james-page)]

Does this work with distro upstream versioning:

  StrictVersion(available_vers) > StrictVersion(cur_vers)

For example, python works towards a major release whereas distro use ~

  2013.2.b1 vs 2013.2~b1

review: Needs Information

[lp:~gandelman-a/charm-helpers/openstack_utils]

46. By Adam Gandelman on 2013-07-09

Use apt_pkg.version_compare() instead of distutil's StrictVersions.

47. By Adam Gandelman on 2013-07-09

lint fix.

[Adam Gandelman (gandelman-a)]

Thanks for catching the DistUtils thing. Opted to use apt_pkg.version_compare there instead. Cleaned up lint, too.

juju_log is still used locally in this helper (via error_out() and elsewhere). I'd prefer to defer that, along with the configure_installation_source refactoring to a later cleanup merge.

review: Needs Resubmitting

[James Page (james-page)]

review: Approve

=== modified file 'charmhelpers/contrib/openstack/openstack_utils.py'
--- charmhelpers/contrib/openstack/openstack_utils.py	2013-05-30 23:26:24 +0000
+++ charmhelpers/contrib/openstack/openstack_utils.py	2013-07-09 18:50:52 +0000
@@ -7,6 +7,14 @@
 import os
 import sys
 
+from charmhelpers.core.hookenv import (
+    config,
+)
+
+from charmhelpers.core.host import (
+    lsb_release,
+)
+
 CLOUD_ARCHIVE_URL = "http://ubuntu-cloud.archive.canonical.com/ubuntu"
 CLOUD_ARCHIVE_KEY_ID = '5EDB1B62EC4926EA'
 
@@ -46,20 +54,9 @@
     sys.exit(1)
 
 
-def lsb_release():
-    '''Return /etc/lsb-release in a dict'''
-    lsb = open('/etc/lsb-release', 'r')
-    d = {}
-    for l in lsb:
-        k, v = l.split('=')
-        d[k.strip()] = v.strip()
-    return d
-
-
 def get_os_codename_install_source(src):
     '''Derive OpenStack release codename from a given installation source.'''
     ubuntu_rel = lsb_release()['DISTRIB_CODENAME']
-
     rel = ''
     if src == 'distro':
         try:
@@ -82,6 +79,11 @@
                 return v
 
 
+def get_os_version_install_source(src):
+    codename = get_os_codename_install_source(src)
+    return get_os_version_codename(codename)
+
+
 def get_os_codename_version(vers):
     '''Determine OpenStack codename from version number.'''
     try:
@@ -101,7 +103,7 @@
     error_out(e)
 
 
-def get_os_codename_package(pkg):
+def get_os_codename_package(pkg, fatal=True):
     '''Derive OpenStack release codename from an installed package.'''
     apt.init()
     cache = apt.Cache()
@@ -109,6 +111,8 @@
     try:
         pkg = cache[pkg]
     except:
+        if not fatal:
+            return None
         e = 'Could not determine version of installed package: %s' % pkg
         error_out(e)
 
@@ -126,9 +130,12 @@
         error_out(e)
 
 
-def get_os_version_package(pkg):
+def get_os_version_package(pkg, fatal=True):
     '''Derive OpenStack version number from an installed package.'''
-    codename = get_os_codename_package(pkg)
+    codename = get_os_codename_package(pkg, fatal=fatal)
+
+    if not codename:
+        return None
 
     if 'swift' in pkg:
         vers_map = swift_codenames
@@ -141,6 +148,7 @@
     #e = "Could not determine OpenStack version for package: %s" % pkg
     #error_out(e)
 
+
 def import_key(keyid):
     cmd = "apt-key adv --keyserver keyserver.ubuntu.com " \
           "--recv-keys %s" % keyid
@@ -149,6 +157,7 @@
     except subprocess.CalledProcessError:
         error_out("Error importing repo key %s" % keyid)
 
+
 def configure_installation_source(rel):
     '''Configure apt installation source.'''
     if rel == 'distro':
@@ -226,3 +235,22 @@
             "#!/bin/bash\n")
         [rc_script.write('export %s=%s\n' % (u, p))
          for u, p in env_vars.iteritems() if u != "script_path"]
+
+
+def openstack_upgrade_available(package):
+    """
+    Determines if an OpenStack upgrade is available from installation
+    source, based on version of installed package.
+
+    :param package: str: Name of installed package.
+
+    :returns: bool:    : Returns True if configured installation source offers
+                         a newer version of package.
+
+    """
+
+    src = config('openstack-origin')
+    cur_vers = get_os_version_package(package)
+    available_vers = get_os_version_install_source(src)
+    apt.init()
+    return apt.version_compare(available_vers, cur_vers) == 1

=== modified file 'tests/contrib/openstack/test_openstack_utils.py'
--- tests/contrib/openstack/test_openstack_utils.py	2013-05-30 23:31:29 +0000
+++ tests/contrib/openstack/test_openstack_utils.py	2013-07-09 18:50:52 +0000
@@ -1,13 +1,12 @@
 from copy import copy
 import os
-import apt_pkg
 import unittest
 from testtools import TestCase
-from io import BytesIO
 import charmhelpers.contrib.openstack.openstack_utils as openstack
 import subprocess
 
-from mock import MagicMock, patch, Mock, call
+from mock import MagicMock, patch, call
+
 
 # mocked return of openstack.lsb_release()
 FAKE_RELEASE = {
@@ -58,9 +57,10 @@
     ('cloud:precise-folsom/updates', url + ' precise-updates/folsom main'),
     ('cloud:precise-grizzly/proposed', url + ' precise-proposed/grizzly main'),
     ('cloud:precise-grizzly', url + ' precise-updates/grizzly main'),
-    ('cloud:precise-grizzly/updates',  url +  ' precise-updates/grizzly main'),
+    ('cloud:precise-grizzly/updates', url + ' precise-updates/grizzly main'),
 ]
 
+
 class OpenStackHelpersTestCase(TestCase):
     def _apt_cache(self):
         # mocks out the apt cache
@@ -101,6 +101,13 @@
         self.assertEquals(openstack.get_os_codename_install_source(src),
                           'havana')
 
+    @patch.object(openstack, 'get_os_version_codename')
+    @patch.object(openstack, 'get_os_codename_install_source')
+    def test_os_version_from_install_source(self, codename, version):
+        codename.return_value = 'grizzly'
+        openstack.get_os_version_install_source('cloud:precise-grizzly')
+        version.assert_called_with('grizzly')
+
     @patch('charmhelpers.contrib.openstack.openstack_utils.lsb_release')
     def test_os_codename_from_bad_install_source(self, mocked_lsb):
         '''Test mapping install source to OpenStack release name'''
@@ -151,7 +158,7 @@
                                   vers['os_release'])
 
     @patch('charmhelpers.contrib.openstack.openstack_utils.error_out')
-    def test_os_codename_from_bad_package_vrsion(self, mocked_error):
+    def test_os_codename_from_bad_package_version(self, mocked_error):
         '''Test deriving OpenStack codename for a poorly versioned package'''
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
@@ -173,6 +180,15 @@
             _err = 'Could not determine version of installed package: foo'
             mocked_error.assert_called_with(_err)
 
+    def test_os_codename_from_bad_package_nonfatal(self):
+        '''Test OpenStack codename from an uninstalled package is non-fatal'''
+        with patch('apt_pkg.Cache') as cache:
+            cache.return_value = self._apt_cache()
+            self.assertEquals(
+                None,
+                openstack.get_os_codename_package('foo', fatal=False)
+            )
+
     @patch('charmhelpers.contrib.openstack.openstack_utils.error_out')
     def test_os_version_from_package(self, mocked_error):
         '''Test deriving OpenStack version from an installed package'''
@@ -198,6 +214,15 @@
             _err = 'Could not determine version of installed package: foo'
             mocked_error.assert_called_with(_err)
 
+    def test_os_version_from_bad_package_nonfatal(self):
+        '''Test OpenStack version from an uninstalled package is non-fatal'''
+        with patch('apt_pkg.Cache') as cache:
+            cache.return_value = self._apt_cache()
+            self.assertEquals(
+                None,
+                openstack.get_os_version_package('foo', fatal=False)
+            )
+
     def test_juju_log(self):
         '''Test shelling out to juju-log'''
         with patch('subprocess.check_call') as mocked_subprocess:
@@ -270,8 +295,10 @@
         for src, url in UCA_SOURCES:
             openstack.configure_installation_source(src)
             _import.assert_called_with(openstack.CLOUD_ARCHIVE_KEY_ID)
-            _open.assert_called_with('/etc/apt/sources.list.d/cloud-archive.list',
-                                     'w')
+            _open.assert_called_with(
+                '/etc/apt/sources.list.d/cloud-archive.list',
+                'w'
+            )
             _file.__enter__().write.assert_called_with(url)
 
     @patch('charmhelpers.contrib.openstack.openstack_utils.error_out')
@@ -310,8 +337,8 @@
     def test_save_scriptrc(self, _open):
         '''Test generation of scriptrc from environment'''
         scriptrc = ['#!/bin/bash\n',
-                   'export setting1=foo\n',
-                   'export setting2=bar\n']
+                    'export setting1=foo\n',
+                    'export setting2=bar\n']
         _file = MagicMock(spec=file)
         _open.return_value = _file
         os.environ['JUJU_UNIT_NAME'] = 'testing-foo/0'
@@ -321,6 +348,32 @@
         for line in scriptrc:
             _file.__enter__().write.assert_has_calls(call(line))
 
+    @patch.object(openstack, 'lsb_release')
+    @patch.object(openstack, 'get_os_version_package')
+    @patch.object(openstack, 'config')
+    def test_openstack_upgrade_detection_true(self, config, vers_pkg, lsb):
+        """Test it detects when an openstack package has available upgrade"""
+        lsb.return_value = FAKE_RELEASE
+        config.return_value = 'cloud:precise-havana'
+        vers_pkg.return_value = '2013.1.1'
+        self.assertTrue(openstack.openstack_upgrade_available('nova-common'))
+        # milestone to major release detection
+        vers_pkg.return_value = '2013.2~b1'
+        self.assertTrue(openstack.openstack_upgrade_available('nova-common'))
+
+    @patch.object(openstack, 'lsb_release')
+    @patch.object(openstack, 'get_os_version_package')
+    @patch.object(openstack, 'config')
+    def test_openstack_upgrade_detection_false(self, config, vers_pkg, lsb):
+        """Test it detects when an openstack upgrade is not necessary"""
+        lsb.return_value = FAKE_RELEASE
+        config.return_value = 'cloud:precise-folsom'
+        vers_pkg.return_value = '2013.1.1'
+        self.assertFalse(openstack.openstack_upgrade_available('nova-common'))
+        # milestone to majro release detection
+        vers_pkg.return_value = '2013.1~b1'
+        self.assertFalse(openstack.openstack_upgrade_available('nova-common'))
+
 
 if __name__ == '__main__':
     unittest.main()