From 1a40c275d553e8f85395dcb811ee3249ae257d41 Mon Sep 17 00:00:00 2001 From: Liam Young Date: Sun, 11 Nov 2018 15:52:22 +0000 Subject: [PATCH] Remove independent OpenStack upgrade Ceilometer has had an openstack-origin and supported upgrading the Ceilometer version independently of the principle that it is associated with. This can cause issues with clashes in package dependencies between the two. This change makes the ability for the agent to be upgraded independently of the principle and instead has the upgrade triggered by the principle. This brings this charm inline with the other OpenStack subordinate charms. Change-Id: I641ac2168ac705191d916eaa0624214791e1745d Closes-Bug: #1802400 --- actions.yaml | 2 - actions/openstack-upgrade | 1 - actions/openstack_upgrade.py | 52 ---------- config.yaml | 29 ------ hooks/ceilometer_hooks.py | 23 +--- hooks/ceilometer_utils.py | 6 +- hooks/nova-ceilometer-relation-changed | 1 + unit_tests/test_actions_openstack_upgrade.py | 66 ------------ unit_tests/test_ceilometer_hooks.py | 104 ++++++------------- unit_tests/test_ceilometer_utils.py | 26 +---- 10 files changed, 40 insertions(+), 270 deletions(-) delete mode 120000 actions/openstack-upgrade delete mode 100755 actions/openstack_upgrade.py create mode 120000 hooks/nova-ceilometer-relation-changed delete mode 100644 unit_tests/test_actions_openstack_upgrade.py diff --git a/actions.yaml b/actions.yaml index 182074f..4caee58 100644 --- a/actions.yaml +++ b/actions.yaml @@ -2,5 +2,3 @@ pause: description: Pause the ceilometer-agent unit. This action will stop ceilometer-agent services. resume: descrpition: Resume the ceilometer-agent unit. This action will start ceilometer-agent services. -openstack-upgrade: - description: Perform openstack upgrades. Config option action-managed-upgrade must be set to True. diff --git a/actions/openstack-upgrade b/actions/openstack-upgrade deleted file mode 120000 index 6179301..0000000 --- a/actions/openstack-upgrade +++ /dev/null @@ -1 +0,0 @@ -openstack_upgrade.py \ No newline at end of file diff --git a/actions/openstack_upgrade.py b/actions/openstack_upgrade.py deleted file mode 100755 index da6f160..0000000 --- a/actions/openstack_upgrade.py +++ /dev/null @@ -1,52 +0,0 @@ -#!/usr/bin/python -# -# Copyright 2016 Canonical Ltd -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import sys - -sys.path.append('hooks/') - -from charmhelpers.contrib.openstack.utils import ( - do_action_openstack_upgrade, -) - -from ceilometer_hooks import ( - config_changed, - CONFIGS, -) - -from ceilometer_utils import ( - do_openstack_upgrade, -) - - -def openstack_upgrade(): - """Perform action-managed OpenStack upgrade. - - Upgrades packages to the configured openstack-origin version and sets - the corresponding action status as a result. - - If the charm was installed from source we cannot upgrade it. - For backwards compatibility a config flag (action-managed-upgrade) must - be set for this code to run, otherwise a full service level upgrade will - fire on config-changed.""" - - if (do_action_openstack_upgrade('ceilometer-common', - do_openstack_upgrade, - CONFIGS)): - config_changed() - -if __name__ == '__main__': - openstack_upgrade() diff --git a/config.yaml b/config.yaml index 5026378..13f2344 100644 --- a/config.yaml +++ b/config.yaml @@ -1,33 +1,4 @@ options: - openstack-origin: - type: string - default: distro - description: | - Repository from which to install. May be one of the following: - distro (default), ppa:somecustom/ppa, a deb url sources entry, - or a supported Ubuntu Cloud Archive e.g. - . - cloud:- - cloud:-/updates - cloud:-/staging - cloud:-/proposed - . - See https://wiki.ubuntu.com/OpenStack/CloudArchive for info on which - cloud archives are available and supported. - . - NOTE: updating this setting to a source that is known to provide - a later version of OpenStack will trigger a software upgrade unless - action-managed-upgrade is set to True. - action-managed-upgrade: - type: boolean - default: False - description: | - If True enables openstack upgrades for this charm via juju actions. - You will still need to set openstack-origin to the new repository but - instead of an upgrade running automatically across all units, it will - wait for you to execute the openstack-upgrade action for this charm on - each unit. If False it will revert to existing behavior of upgrading - all units on config change. nagios_context: type: string default: "juju" diff --git a/hooks/ceilometer_hooks.py b/hooks/ceilometer_hooks.py index d41b1c5..a4cd77d 100755 --- a/hooks/ceilometer_hooks.py +++ b/hooks/ceilometer_hooks.py @@ -21,7 +21,6 @@ from charmhelpers.fetch import ( apt_update ) from charmhelpers.core.hookenv import ( - config, Hooks, UnregisteredHookError, log, is_relation_made, @@ -30,11 +29,7 @@ from charmhelpers.core.hookenv import ( relation_ids, ) from charmhelpers.contrib.openstack.utils import ( - configure_installation_source, - openstack_upgrade_available, pausable_restart_on_change as restart_on_change, - os_release, - CompareOpenStackReleases, is_unit_paused_set, series_upgrade_prepare, series_upgrade_complete, @@ -44,7 +39,6 @@ from ceilometer_utils import ( services, register_configs, NOVA_SETTINGS, - do_openstack_upgrade, assess_status, get_packages, pause_unit_helper, @@ -58,8 +52,6 @@ CONFIGS = register_configs() @hooks.hook('install.real') def install(): - origin = config('openstack-origin') - configure_installation_source(origin) status_set('maintenance', 'Installing apt packages') apt_update(fatal=True) # Install -common package so we get accurate version determination @@ -95,6 +87,7 @@ def upgrade_charm(): nova_ceilometer_joined(rid) +@hooks.hook('nova-ceilometer-relation-changed') @hooks.hook('config-changed') @restart_on_change(restart_map(), stopstart=True) def config_changed(): @@ -104,19 +97,7 @@ def config_changed(): log("Unit is pause or upgrading. Skipping config_changed", "WARN") return - if not config('action-managed-upgrade'): - if openstack_upgrade_available('ceilometer-common'): - status_set('maintenance', 'Running openstack upgrade') - do_openstack_upgrade(CONFIGS) - else: - # It's possible that a partial upgrade has occurred if nova-compute - # was upgraded first. If > mitaka call apt_install to allow - # packages which are enabled for specific OpenStack version to be - # installed if they haven't been already. - os_rel = os_release('ceilometer-common', reset_cache=True) - if (CompareOpenStackReleases(os_rel) >= 'mitaka'): - apt_install(filter_installed_packages(get_packages()), - fatal=True) + apt_install(filter_installed_packages(get_packages()), fatal=True) if is_relation_made('nrpe-external-master'): update_nrpe_config() CONFIGS.write_all() diff --git a/hooks/ceilometer_utils.py b/hooks/ceilometer_utils.py index 18ac746..2a84a09 100644 --- a/hooks/ceilometer_utils.py +++ b/hooks/ceilometer_utils.py @@ -85,7 +85,7 @@ CONFIG_FILES = { 'hook_contexts': [ CeilometerServiceContext(ssl_dir=CEILOMETER_CONF_DIR), context.InternalEndpointContext(), - context.MemcacheContext()], + context.MemcacheContext(package='ceilometer-common')], 'services': CEILOMETER_AGENT_SERVICES }, } @@ -114,7 +114,9 @@ def register_configs(): configs.register(conf, CONFIG_FILES[conf]['hook_contexts']) if enable_memcache(release=release): - configs.register(MEMCACHED_CONF, [context.MemcacheContext()]) + configs.register( + MEMCACHED_CONF, + [context.MemcacheContext(package='ceilometer-common')]) return configs diff --git a/hooks/nova-ceilometer-relation-changed b/hooks/nova-ceilometer-relation-changed new file mode 120000 index 0000000..c948469 --- /dev/null +++ b/hooks/nova-ceilometer-relation-changed @@ -0,0 +1 @@ +ceilometer_hooks.py \ No newline at end of file diff --git a/unit_tests/test_actions_openstack_upgrade.py b/unit_tests/test_actions_openstack_upgrade.py deleted file mode 100644 index c704f1f..0000000 --- a/unit_tests/test_actions_openstack_upgrade.py +++ /dev/null @@ -1,66 +0,0 @@ -# Copyright 2016 Canonical Ltd -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from mock import patch -import os - -os.environ['JUJU_UNIT_NAME'] = 'ceilometer' - -with patch('ceilometer_utils.register_configs') as register_configs: - with patch('ceilometer_utils.restart_map') as restart_map: - import openstack_upgrade - -from test_utils import ( - CharmTestCase -) - -TO_PATCH = [ - 'config_changed', - 'do_openstack_upgrade', -] - - -class TestCinderUpgradeActions(CharmTestCase): - - def setUp(self): - super(TestCinderUpgradeActions, self).setUp(openstack_upgrade, - TO_PATCH) - - @patch('charmhelpers.contrib.openstack.utils.juju_log') - @patch('charmhelpers.contrib.openstack.utils.config') - @patch('charmhelpers.contrib.openstack.utils.action_set') - @patch('charmhelpers.contrib.openstack.utils.openstack_upgrade_available') - def test_openstack_upgrade_true(self, upgrade_avail, - action_set, config, log): - upgrade_avail.return_value = True - config.return_value = True - - openstack_upgrade.openstack_upgrade() - - self.assertTrue(self.do_openstack_upgrade.called) - self.assertTrue(self.config_changed.called) - - @patch('charmhelpers.contrib.openstack.utils.juju_log') - @patch('charmhelpers.contrib.openstack.utils.config') - @patch('charmhelpers.contrib.openstack.utils.action_set') - @patch('charmhelpers.contrib.openstack.utils.openstack_upgrade_available') - def test_openstack_upgrade_false(self, upgrade_avail, - action_set, config, log): - upgrade_avail.return_value = True - config.return_value = False - - openstack_upgrade.openstack_upgrade() - - self.assertFalse(self.do_openstack_upgrade.called) - self.assertFalse(self.config_changed.called) diff --git a/unit_tests/test_ceilometer_hooks.py b/unit_tests/test_ceilometer_hooks.py index 1b0bc6d..2be6c51 100644 --- a/unit_tests/test_ceilometer_hooks.py +++ b/unit_tests/test_ceilometer_hooks.py @@ -13,34 +13,26 @@ # limitations under the License. import json -from mock import patch, MagicMock +from mock import patch import ceilometer_utils -# Patch out register_configs for import of hooks -_register_configs = ceilometer_utils.register_configs -ceilometer_utils.register_configs = MagicMock() -import ceilometer_hooks as hooks - -# Renable old function -ceilometer_utils.register_configs = _register_configs +with patch('ceilometer_utils.register_configs'): + with patch('ceilometer_utils.restart_map'): + import ceilometer_hooks as hooks from test_utils import CharmTestCase TO_PATCH = [ - 'configure_installation_source', + 'CONFIGS', 'apt_install', 'apt_update', - 'config', 'filter_installed_packages', - 'CONFIGS', - 'relation_set', - 'openstack_upgrade_available', - 'do_openstack_upgrade', - 'update_nrpe_config', - 'is_relation_made', 'get_packages', - 'os_release', + 'is_relation_made', + 'is_unit_paused_set', + 'relation_set', + 'update_nrpe_config', ] @@ -48,21 +40,12 @@ class CeilometerHooksTest(CharmTestCase): def setUp(self): super(CeilometerHooksTest, self).setUp(hooks, TO_PATCH) - self.config.side_effect = self.test_config.get - - @patch('charmhelpers.core.hookenv.config') - def test_configure_source(self, mock_config): - self.test_config.set('openstack-origin', 'cloud:precise-havana') - hooks.hooks.execute(['hooks/install']) - self.configure_installation_source.\ - assert_called_with('cloud:precise-havana') @patch('charmhelpers.core.hookenv.config') def test_install_hook(self, mock_config): ceil_pkgs = ['pkg1', 'pkg2'] self.filter_installed_packages.return_value = ceil_pkgs hooks.hooks.execute(['hooks/install']) - self.assertTrue(self.configure_installation_source.called) self.apt_update.assert_called_with(fatal=True) self.apt_install.assert_called_with(ceil_pkgs, fatal=True) @@ -89,56 +72,33 @@ class CeilometerHooksTest(CharmTestCase): ceilometer_utils.NOVA_SETTINGS)) @patch('charmhelpers.core.hookenv.config') - def test_config_changed_no_upgrade(self, mock_config): - self.openstack_upgrade_available.return_value = False - self.os_release.return_value = 'liberty' + def test_config_changed(self, mock_config): + self.is_relation_made.return_value = True + self.is_unit_paused_set.return_value = False + self.filter_installed_packages.return_value = ['pkg1', 'pkg2'] hooks.hooks.execute(['hooks/config-changed']) - self.openstack_upgrade_available.\ - assert_called_with('ceilometer-common') - self.assertFalse(self.do_openstack_upgrade.called) - self.assertTrue(self.CONFIGS.write_all.called) - self.assertTrue(self.update_nrpe_config.called) - self.assertFalse(self.apt_install.called) - - @patch('charmhelpers.core.hookenv.config') - def test_config_changed_partial_upgrade(self, mock_config): - self.openstack_upgrade_available.return_value = False - self.os_release.return_value = 'mitaka' - hooks.hooks.execute(['hooks/config-changed']) - self.openstack_upgrade_available.\ - assert_called_with('ceilometer-common') - self.assertFalse(self.do_openstack_upgrade.called) - self.assertTrue(self.CONFIGS.write_all.called) - self.assertTrue(self.update_nrpe_config.called) - self.assertTrue(self.apt_install.called) - - @patch('charmhelpers.core.hookenv.config') - def test_config_changed_upgrade(self, mock_config): - self.openstack_upgrade_available.return_value = True - hooks.hooks.execute(['hooks/config-changed']) - self.openstack_upgrade_available.\ - assert_called_with('ceilometer-common') - self.assertTrue(self.do_openstack_upgrade.called) - self.assertTrue(self.CONFIGS.write_all.called) - self.assertTrue(self.update_nrpe_config.called) - - def test_config_changed_with_openstack_upgrade_action(self): - self.openstack_upgrade_available.return_value = True - self.test_config.set('action-managed-upgrade', True) - - hooks.hooks.execute(['hooks/config-changed']) - - self.assertFalse(self.do_openstack_upgrade.called) + self.update_nrpe_config.assert_called_once_with() + self.CONFIGS.write_all.assert_called_once_with() + self.apt_install.assert_called_once_with(['pkg1', 'pkg2'], fatal=True) + self.is_relation_made.assert_called_once_with('nrpe-external-master') @patch('charmhelpers.core.hookenv.config') def test_config_changed_no_nrpe(self, mock_config): - self.openstack_upgrade_available.return_value = False - self.os_release.return_value = 'mitaka' self.is_relation_made.return_value = False - + self.is_unit_paused_set.return_value = False + self.filter_installed_packages.return_value = ['pkg1', 'pkg2'] hooks.hooks.execute(['hooks/config-changed']) - self.openstack_upgrade_available.\ - assert_called_with('ceilometer-common') - self.assertFalse(self.do_openstack_upgrade.called) - self.assertTrue(self.CONFIGS.write_all.called) self.assertFalse(self.update_nrpe_config.called) + self.CONFIGS.write_all.assert_called_once_with() + self.apt_install.assert_called_once_with(['pkg1', 'pkg2'], fatal=True) + self.is_relation_made.assert_called_once_with('nrpe-external-master') + + @patch('charmhelpers.core.hookenv.config') + def test_config_changed_paused(self, mock_config): + self.is_relation_made.return_value = True + self.is_unit_paused_set.return_value = True + self.filter_installed_packages.return_value = ['pkg1', 'pkg2'] + hooks.hooks.execute(['hooks/config-changed']) + self.assertFalse(self.update_nrpe_config.called) + self.assertFalse(self.CONFIGS.write_all.called) + self.assertFalse(self.apt_install.called) diff --git a/unit_tests/test_ceilometer_utils.py b/unit_tests/test_ceilometer_utils.py index 0dfb055..6672c09 100644 --- a/unit_tests/test_ceilometer_utils.py +++ b/unit_tests/test_ceilometer_utils.py @@ -14,7 +14,7 @@ import sys -from mock import MagicMock, patch, call +from mock import MagicMock, patch # python-apt is not installed as part of test-requirements but is imported by # some charmhelpers modules so create a fake import. @@ -81,30 +81,6 @@ class CeilometerUtilsTest(CharmTestCase): '/etc/memcached.conf': ['memcached']} self.assertEqual(restart_map, expect) - def test_do_openstack_upgrade(self): - self.config.side_effect = self.test_config.get - self.test_config.set('openstack-origin', 'cloud:precise-havana') - self.get_os_codename_install_source.return_value = 'havana' - self.get_os_codename_package.return_value = 'icehouse' - configs = MagicMock() - utils.do_openstack_upgrade(configs) - configs.set_release.assert_called_with(openstack_release='havana') - self.assertTrue(self.log.called) - self.apt_update.assert_called_with(fatal=True) - dpkg_opts = [ - '--option', 'Dpkg::Options::=--force-confnew', - '--option', 'Dpkg::Options::=--force-confdef', - ] - self.apt_install.assert_has_calls([ - call(packages=utils.CEILOMETER_AGENT_PACKAGES, - options=dpkg_opts, fatal=True), - call(['python-ceilometer', 'ceilometer-common', - 'ceilometer-agent-compute'], fatal=True), - ]) - self.configure_installation_source.assert_called_with( - 'cloud:precise-havana' - ) - def test_assess_status(self): with patch.object(utils, 'assess_status_func') as asf: callee = MagicMock()