From b8b5acd44d5ecf7490b31802cf2459c215a95822 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Mon, 20 Mar 2017 11:37:16 +0000 Subject: [PATCH] Fix alphanumeric comparisons for openstack and ubuntu releases - sync charmhelpers with fix-alpha helpers - fix up code where the alpha comparisons are done - fix tests which assumed mocks would just work on os_release() Change-Id: I9f4a3b15e53c757c2ae5ffb2eb45b6cdaecf4c8e Related-Bug: #1659575 --- charm-helpers-tests.yaml | 2 +- hooks/keystone_hooks.py | 7 +++++-- hooks/keystone_utils.py | 13 ++++++++----- tests/basic_deployment.py | 12 ++++++++---- tox.ini | 2 +- unit_tests/test_actions_git_reinstall.py | 1 + unit_tests/test_keystone_hooks.py | 7 +++++++ unit_tests/test_keystone_utils.py | 4 ++++ 8 files changed, 35 insertions(+), 13 deletions(-) diff --git a/charm-helpers-tests.yaml b/charm-helpers-tests.yaml index f987a1e6..b0de9df6 100644 --- a/charm-helpers-tests.yaml +++ b/charm-helpers-tests.yaml @@ -1,7 +1,7 @@ branch: lp:charm-helpers destination: tests/charmhelpers include: - - osplatform - contrib.amulet - contrib.openstack.amulet - core + - osplatform diff --git a/hooks/keystone_hooks.py b/hooks/keystone_hooks.py index c5976fae..0e9e95b4 100755 --- a/hooks/keystone_hooks.py +++ b/hooks/keystone_hooks.py @@ -70,6 +70,7 @@ from charmhelpers.contrib.openstack.utils import ( os_release, pausable_restart_on_change as restart_on_change, is_unit_paused_set, + CompareOpenStackReleases, ) from keystone_utils import ( @@ -398,7 +399,8 @@ def db_changed(): else: CONFIGS.write(KEYSTONE_CONF) leader_init_db_if_ready(use_current_context=True) - if os_release('keystone-common') >= 'liberty': + if CompareOpenStackReleases( + os_release('keystone-common')) >= 'liberty': CONFIGS.write(POLICY_JSON) @@ -411,7 +413,8 @@ def pgsql_db_changed(): else: CONFIGS.write(KEYSTONE_CONF) leader_init_db_if_ready(use_current_context=True) - if os_release('keystone-common') >= 'liberty': + if CompareOpenStackReleases( + os_release('keystone-common')) >= 'liberty': CONFIGS.write(POLICY_JSON) diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py index b0b3a6f2..8534529f 100644 --- a/hooks/keystone_utils.py +++ b/hooks/keystone_utils.py @@ -73,6 +73,7 @@ from charmhelpers.contrib.openstack.utils import ( is_unit_paused_set, make_assess_status_func, os_application_version_set, + CompareOpenStackReleases, ) from charmhelpers.contrib.python.packages import ( @@ -124,6 +125,7 @@ from charmhelpers.core.host import ( pwgen, lsb_release, write_file, + CompareHostReleases, ) from charmhelpers.contrib.peerstorage import ( @@ -416,7 +418,7 @@ def resource_map(): """ resource_map = deepcopy(BASE_RESOURCE_MAP) - if os_release('keystone') < 'liberty': + if CompareOpenStackReleases(os_release('keystone')) < 'liberty': resource_map.pop(POLICY_JSON) if os.path.exists('/etc/apache2/conf-available'): resource_map.pop(APACHE_CONF) @@ -478,7 +480,7 @@ def run_in_apache(): """Return true if keystone API is run under apache2 with mod_wsgi in this release. """ - return os_release('keystone') >= 'liberty' + return CompareOpenStackReleases(os_release('keystone')) >= 'liberty' def disable_unused_apache_sites(): @@ -2102,13 +2104,14 @@ def get_requested_grants(settings): def setup_ipv6(): """Check ipv6-mode validity and setup dependencies""" ubuntu_rel = lsb_release()['DISTRIB_CODENAME'].lower() - if ubuntu_rel < "trusty": + if CompareHostReleases(ubuntu_rel) < "trusty": raise Exception("IPv6 is not supported in the charms for Ubuntu " "versions less than Trusty 14.04") # Need haproxy >= 1.5.3 for ipv6 so for Trusty if we are <= Kilo we need to # use trusty-backports otherwise we can use the UCA. - if ubuntu_rel == 'trusty' and os_release('keystone') < 'liberty': + if (ubuntu_rel == 'trusty' and + CompareOpenStackReleases(os_release('keystone')) < 'liberty'): add_source('deb http://archive.ubuntu.com/ubuntu trusty-backports ' 'main') apt_update() @@ -2324,7 +2327,7 @@ def git_post_install(projects_yaml): bin_dir = os.path.join(git_pip_venv_dir(projects_yaml), 'bin') # The charm runs the keystone API under apache2 for openstack liberty # onward. Prior to liberty upstart is used. - if os_release('keystone') < 'liberty': + if CompareOpenStackReleases(os_release('keystone')) < 'liberty': keystone_context = { 'service_description': 'Keystone API server', 'service_name': 'Keystone', diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index da0b22a2..55140891 100644 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -420,16 +420,20 @@ class KeystoneBasicDeployment(OpenStackAmuletDeployment): self.validate_keystone_users(self.keystone_v2) def is_liberty_or_newer(self): - os_release = self._get_openstack_release_string() - if os_release >= 'liberty': + # os_release = self._get_openstack_release_string() + os_release = self._get_openstack_release() + # if os_release >= 'liberty': + if os_release >= self.trusty_liberty: return True else: u.log.info('Skipping test, {} < liberty'.format(os_release)) return False def is_mitaka_or_newer(self): - os_release = self._get_openstack_release_string() - if os_release >= 'mitaka': + # os_release = self._get_openstack_release_string() + os_release = self._get_openstack_release() + # if os_release >= 'mitaka': + if os_release >= self.xenial_mitaka: return True else: u.log.info('Skipping test, {} < mitaka'.format(os_release)) diff --git a/tox.ini b/tox.ini index 1463d531..1610be31 100644 --- a/tox.ini +++ b/tox.ini @@ -14,7 +14,7 @@ install_command = pip install --allow-unverified python-apt {opts} {packages} commands = ostestr {posargs} whitelist_externals = juju -passenv = HOME TERM AMULET_* CS_API_URL +passenv = HOME TERM AMULET_* CS_API_* [testenv:py27] basepython = python2.7 diff --git a/unit_tests/test_actions_git_reinstall.py b/unit_tests/test_actions_git_reinstall.py index 0875b484..d2898bd4 100644 --- a/unit_tests/test_actions_git_reinstall.py +++ b/unit_tests/test_actions_git_reinstall.py @@ -29,6 +29,7 @@ with patch('hooks.charmhelpers.contrib.hardening.harden.harden') as mock_dec: lambda *args, **kwargs: f(*args, **kwargs)) with patch('hooks.keystone_utils.register_configs') as register_configs: with patch('hooks.keystone_utils.os_release') as os_release: + os_release.return_value = 'juno' import git_reinstall from test_utils import ( diff --git a/unit_tests/test_keystone_hooks.py b/unit_tests/test_keystone_hooks.py index dbd77f73..ee0e08df 100644 --- a/unit_tests/test_keystone_hooks.py +++ b/unit_tests/test_keystone_hooks.py @@ -128,6 +128,7 @@ class KeystoneRelationTests(CharmTestCase): @patch.object(utils, 'git_install_requested') @patch.object(unison, 'ensure_user') def test_install_hook(self, ensure_user, git_requested, os_release): + os_release.return_value = 'havana' git_requested.return_value = False self.run_in_apache.return_value = False repo = 'cloud:precise-grizzly' @@ -149,6 +150,7 @@ class KeystoneRelationTests(CharmTestCase): @patch.object(unison, 'ensure_user') def test_install_hook_apache2(self, ensure_user, git_requested, os_release): + os_release.return_value = 'havana' git_requested.return_value = False self.run_in_apache.return_value = True repo = 'cloud:xenial-newton' @@ -170,6 +172,7 @@ class KeystoneRelationTests(CharmTestCase): @patch.object(utils, 'git_install_requested') @patch.object(unison, 'ensure_user') def test_install_hook_git(self, ensure_user, git_requested, os_release): + os_release.return_value = 'havana' git_requested.return_value = True repo = 'cloud:trusty-juno' openstack_origin_git = { @@ -332,6 +335,7 @@ class KeystoneRelationTests(CharmTestCase): def test_db_changed(self, configs, mock_ensure_ssl_cert_master, leader_init): + self.os_release.return_value = 'havana' mock_ensure_ssl_cert_master.return_value = False self._shared_db_test(configs, 'keystone/3') self.assertEquals([call('/etc/keystone/keystone.conf')], @@ -344,6 +348,7 @@ class KeystoneRelationTests(CharmTestCase): def test_postgresql_db_changed(self, configs, mock_ensure_ssl_cert_master, leader_init): + self.os_release.return_value = 'havana' mock_ensure_ssl_cert_master.return_value = False self._postgresql_db_test(configs) self.assertEquals([call('/etc/keystone/keystone.conf')], @@ -1006,6 +1011,7 @@ class KeystoneRelationTests(CharmTestCase): git_requested, os_release, update): + os_release.return_value = 'havana' mock_is_db_initialised.return_value = True mock_is_db_ready.return_value = True mock_is_elected_leader.return_value = False @@ -1169,6 +1175,7 @@ class KeystoneRelationTests(CharmTestCase): mock_relation_ids, mock_log, git_requested, os_release, update): + os_release.return_value = 'havana' mock_relation_ids.return_value = [] mock_ensure_ssl_cert_master.return_value = False # Ensure always returns diff diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py index c418bdb1..bd517d94 100644 --- a/unit_tests/test_keystone_utils.py +++ b/unit_tests/test_keystone_utils.py @@ -135,6 +135,7 @@ class TestKeystoneUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.utils.config') def test_determine_packages(self, _config): + self.os_release.return_value = 'havana' _config.return_value = None result = utils.determine_packages() ex = utils.BASE_PACKAGES + ['keystone', 'python-keystoneclient'] @@ -142,6 +143,7 @@ class TestKeystoneUtils(CharmTestCase): @patch('charmhelpers.contrib.openstack.utils.config') def test_determine_packages_git(self, _config): + self.os_release.return_value = 'havana' _config.return_value = openstack_origin_git result = utils.determine_packages() ex = utils.BASE_PACKAGES + utils.BASE_GIT_PACKAGES @@ -193,6 +195,7 @@ class TestKeystoneUtils(CharmTestCase): disable_unused_apache_sites.assert_called_with() def test_migrate_database(self): + self.os_release.return_value = 'havana' utils.migrate_database() self.service_stop.assert_called_with('keystone') @@ -820,6 +823,7 @@ class TestKeystoneUtils(CharmTestCase): @patch('subprocess.check_call') def test_git_post_install(self, check_call, rmtree, copytree, symlink, exists, join): + self.os_release.return_value = 'havana' projects_yaml = openstack_origin_git join.return_value = 'joined-string' self.git_pip_venv_dir.return_value = '/mnt/openstack-git/venv'