Remove pause/resume logic
This is a subordinate charm and since a recent commit [1] it shares a list of its services with the principal charm nova-compute, which has now the responsibility to pause and resume services. [2] The ceilometer-agent-compute service has a dependency to the nova-compute service anyway, so it is impossible for this charm to resume its service if its principal charm nova-compute is paused. This is what also led to errors in ceilometer-agent's post-series-upgrade hook. This hook attempted to resume its service although the principal service was still paused. Removing this logic entirely solves this issue. Validated by running openstack-upgrade and series-upgrade tests. [3] [1]: https://opendev.org/openstack/charm-ceilometer-agent/commit/be45f779 [2]: https://opendev.org/openstack/charm-nova-compute/commit/8fb37dc0 [3]: https://github.com/openstack-charmers/charmed-openstack-tester Closes-Bug: #1952882 Change-Id: Ia22b53b52b541250f7f803c6708968d75e64475c
This commit is contained in:
Aurelien Lourot
parent
d6a3078145
commit
43a2312794
|
|
@ -1,4 +0,0 @@
|
|||
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.
|
||||
|
|
@ -1,75 +0,0 @@
|
|||
#!/usr/bin/env python3
|
||||
#
|
||||
# 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 os
|
||||
import sys
|
||||
|
||||
|
||||
_path = os.path.dirname(os.path.realpath(__file__))
|
||||
_hooks = os.path.abspath(os.path.join(_path, '../hooks'))
|
||||
_root = os.path.abspath(os.path.join(_path, '..'))
|
||||
|
||||
|
||||
def _add_path(path):
|
||||
if path not in sys.path:
|
||||
sys.path.insert(1, path)
|
||||
|
||||
|
||||
_add_path(_hooks)
|
||||
_add_path(_root)
|
||||
|
||||
|
||||
from charmhelpers.core.hookenv import action_fail
|
||||
from ceilometer_utils import (
|
||||
pause_unit_helper,
|
||||
resume_unit_helper,
|
||||
register_configs,
|
||||
)
|
||||
|
||||
|
||||
def pause(args):
|
||||
"""Pause the Ceilometer services.
|
||||
@raises Exception should the service fail to stop.
|
||||
"""
|
||||
pause_unit_helper(register_configs())
|
||||
|
||||
|
||||
def resume(args):
|
||||
"""Resume the Ceilometer services.
|
||||
@raises Exception should the service fail to start."""
|
||||
resume_unit_helper(register_configs())
|
||||
|
||||
|
||||
# A dictionary of all the defined actions to callables (which take
|
||||
# parsed arguments).
|
||||
ACTIONS = {"pause": pause, "resume": resume}
|
||||
|
||||
|
||||
def main(args):
|
||||
action_name = os.path.basename(args[0])
|
||||
try:
|
||||
action = ACTIONS[action_name]
|
||||
except KeyError:
|
||||
return "Action %s undefined" % action_name
|
||||
else:
|
||||
try:
|
||||
action(args)
|
||||
except Exception as e:
|
||||
action_fail(str(e))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main(sys.argv))
|
||||
|
|
@ -1 +0,0 @@
|
|||
actions.py
|
||||
|
|
@ -1 +0,0 @@
|
|||
actions.py
|
||||
|
|
@ -48,9 +48,6 @@ from charmhelpers.core.host import (
|
|||
)
|
||||
from charmhelpers.contrib.openstack.utils import (
|
||||
pausable_restart_on_change as restart_on_change,
|
||||
is_unit_paused_set,
|
||||
series_upgrade_prepare,
|
||||
series_upgrade_complete,
|
||||
)
|
||||
from ceilometer_utils import (
|
||||
restart_map,
|
||||
|
|
@ -60,8 +57,6 @@ from ceilometer_utils import (
|
|||
assess_status,
|
||||
get_packages,
|
||||
releases_packages_map,
|
||||
pause_unit_helper,
|
||||
resume_unit_helper,
|
||||
remove_old_packages,
|
||||
)
|
||||
from charmhelpers.contrib.charmsupport import nrpe
|
||||
|
|
@ -107,11 +102,21 @@ def upgrade_charm():
|
|||
apt_install(
|
||||
filter_installed_packages(get_packages()),
|
||||
fatal=True)
|
||||
|
||||
packages_removed = remove_old_packages()
|
||||
if packages_removed and not is_unit_paused_set():
|
||||
if packages_removed:
|
||||
# NOTE(lourot): this code path is run when python2 packages have been
|
||||
# purged and python3 packages installed. In this case it is best to
|
||||
# restart the ceilometer-agent-compute service.
|
||||
log("Package purge detected, restarting services", "INFO")
|
||||
for s in services():
|
||||
# NOTE(lourot): if the principal nova-compute unit is paused, the
|
||||
# ceilometer-agent-compute service, depending on the nova-compute
|
||||
# service, will refuse to start, but this call won't raise. It'll
|
||||
# just return False. This can safely be ignored: all services will
|
||||
# finally be started when unpausing nova-compute.
|
||||
service_restart(s)
|
||||
|
||||
# NOTE(jamespage): Ensure any changes to nova presented data are made
|
||||
# during charm upgrades.
|
||||
for rid in relation_ids('nova-ceilometer'):
|
||||
|
|
@ -122,12 +127,6 @@ def upgrade_charm():
|
|||
@hooks.hook('config-changed')
|
||||
@restart_on_change(restart_map(), stopstart=True)
|
||||
def config_changed():
|
||||
# if we are paused, delay doing any config changed hooks.
|
||||
# It is forced on the resume.
|
||||
if is_unit_paused_set():
|
||||
log("Unit is pause or upgrading. Skipping config_changed", "WARN")
|
||||
return
|
||||
|
||||
apt_install(filter_installed_packages(get_packages()), fatal=True)
|
||||
if is_relation_made('nrpe-external-master'):
|
||||
update_nrpe_config()
|
||||
|
|
@ -146,20 +145,6 @@ def update_nrpe_config():
|
|||
nrpe_setup.write()
|
||||
|
||||
|
||||
@hooks.hook('pre-series-upgrade')
|
||||
def pre_series_upgrade():
|
||||
log("Running prepare series upgrade hook", "INFO")
|
||||
series_upgrade_prepare(
|
||||
pause_unit_helper, CONFIGS)
|
||||
|
||||
|
||||
@hooks.hook('post-series-upgrade')
|
||||
def post_series_upgrade():
|
||||
log("Running complete series upgrade hook", "INFO")
|
||||
series_upgrade_complete(
|
||||
resume_unit_helper, CONFIGS)
|
||||
|
||||
|
||||
@hooks.hook('amqp-relation-joined')
|
||||
def amqp_joined(relation_id=None):
|
||||
relation_set(relation_id=relation_id,
|
||||
|
|
|
|||
|
|
@ -24,8 +24,6 @@ from ceilometer_contexts import (
|
|||
from charmhelpers.contrib.openstack.utils import (
|
||||
get_os_codename_package,
|
||||
make_assess_status_func,
|
||||
pause_unit,
|
||||
resume_unit,
|
||||
os_application_version_set,
|
||||
token_cache_pkgs,
|
||||
enable_memcache,
|
||||
|
|
@ -288,40 +286,6 @@ def assess_status_func(configs):
|
|||
services=services(), ports=None)
|
||||
|
||||
|
||||
def pause_unit_helper(configs):
|
||||
"""Helper function to pause a unit, and then call assess_status(...) in
|
||||
effect, so that the status is correctly updated.
|
||||
Uses charmhelpers.contrib.openstack.utils.pause_unit() to do the work.
|
||||
@param configs: a templating.OSConfigRenderer() object
|
||||
@returns None - this function is executed for its side-effect
|
||||
"""
|
||||
_pause_resume_helper(pause_unit, configs)
|
||||
|
||||
|
||||
def resume_unit_helper(configs):
|
||||
"""Helper function to resume a unit, and then call assess_status(...) in
|
||||
effect, so that the status is correctly updated.
|
||||
Uses charmhelpers.contrib.openstack.utils.resume_unit() to do the work.
|
||||
@param configs: a templating.OSConfigRenderer() object
|
||||
@returns None - this function is executed for its side-effect
|
||||
"""
|
||||
_pause_resume_helper(resume_unit, configs)
|
||||
|
||||
|
||||
def _pause_resume_helper(f, configs):
|
||||
"""Helper function that uses the make_assess_status_func(...) from
|
||||
charmhelpers.contrib.openstack.utils to create an assess_status(...)
|
||||
function that can be used with the pause/resume of the unit
|
||||
@param f: the function to be used with the assess_status(...) function
|
||||
@returns None - this function is executed for its side-effect
|
||||
"""
|
||||
# TODO(ajkavanagh) - ports= has been left off because of the race hazard
|
||||
# that exists due to service_start()
|
||||
f(assess_status_func(configs),
|
||||
services=services(),
|
||||
ports=None)
|
||||
|
||||
|
||||
def _get_current_release():
|
||||
return (get_os_codename_package('ceilometer-common', fatal=False)
|
||||
or 'icehouse')
|
||||
|
|
|
|||
|
|
@ -1,78 +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 unittest import mock
|
||||
from unittest.mock import patch
|
||||
|
||||
from test_utils import CharmTestCase
|
||||
|
||||
with patch('ceilometer_utils.register_configs') as configs:
|
||||
configs.return_value = 'test-config'
|
||||
import actions
|
||||
|
||||
|
||||
class PauseTestCase(CharmTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(PauseTestCase, self).setUp(
|
||||
actions, ["pause_unit_helper"])
|
||||
|
||||
def test_pauses_services(self):
|
||||
actions.pause([])
|
||||
self.pause_unit_helper.assert_called_once_with('test-config')
|
||||
|
||||
|
||||
class ResumeTestCase(CharmTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(ResumeTestCase, self).setUp(
|
||||
actions, ["resume_unit_helper"])
|
||||
|
||||
def test_pauses_services(self):
|
||||
actions.resume([])
|
||||
self.resume_unit_helper.assert_called_once_with('test-config')
|
||||
|
||||
|
||||
class MainTestCase(CharmTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(MainTestCase, self).setUp(actions, ["action_fail"])
|
||||
|
||||
def test_invokes_action(self):
|
||||
dummy_calls = []
|
||||
|
||||
def dummy_action(args):
|
||||
dummy_calls.append(True)
|
||||
|
||||
with mock.patch.dict(actions.ACTIONS, {"foo": dummy_action}):
|
||||
actions.main(["foo"])
|
||||
self.assertEqual(dummy_calls, [True])
|
||||
|
||||
def test_unknown_action(self):
|
||||
"""Unknown actions aren't a traceback."""
|
||||
exit_string = actions.main(["foo"])
|
||||
self.assertEqual("Action foo undefined", exit_string)
|
||||
|
||||
def test_failing_action(self):
|
||||
"""Actions which traceback trigger action_fail() calls."""
|
||||
dummy_calls = []
|
||||
|
||||
self.action_fail.side_effect = dummy_calls.append
|
||||
|
||||
def dummy_action(args):
|
||||
raise ValueError("uh oh")
|
||||
|
||||
with mock.patch.dict(actions.ACTIONS, {"foo": dummy_action}):
|
||||
actions.main(["foo"])
|
||||
self.assertEqual(dummy_calls, ["uh oh"])
|
||||
|
|
@ -32,7 +32,6 @@ TO_PATCH = [
|
|||
'releases_packages_map',
|
||||
'services',
|
||||
'is_relation_made',
|
||||
'is_unit_paused_set',
|
||||
'relation_set',
|
||||
'update_nrpe_config',
|
||||
]
|
||||
|
|
@ -92,7 +91,6 @@ class CeilometerHooksTest(CharmTestCase):
|
|||
@patch('charmhelpers.core.hookenv.config')
|
||||
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.update_nrpe_config.assert_called_once_with()
|
||||
|
|
@ -103,20 +101,9 @@ class CeilometerHooksTest(CharmTestCase):
|
|||
@patch('charmhelpers.core.hookenv.config')
|
||||
def test_config_changed_no_nrpe(self, mock_config):
|
||||
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.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)
|
||||
|
|
|
|||
|
|
@ -113,25 +113,6 @@ class CeilometerUtilsTest(CharmTestCase):
|
|||
make_assess_status_func.assert_called_once_with(
|
||||
'test-config', REQUIRED_INTERFACES, services='s1', ports=None)
|
||||
|
||||
def test_pause_unit_helper(self):
|
||||
with patch.object(utils, '_pause_resume_helper') as prh:
|
||||
utils.pause_unit_helper('random-config')
|
||||
prh.assert_called_once_with(utils.pause_unit, 'random-config')
|
||||
with patch.object(utils, '_pause_resume_helper') as prh:
|
||||
utils.resume_unit_helper('random-config')
|
||||
prh.assert_called_once_with(utils.resume_unit, 'random-config')
|
||||
|
||||
@patch.object(utils, 'services')
|
||||
def test_pause_resume_helper(self, services):
|
||||
f = MagicMock()
|
||||
services.return_value = 's1'
|
||||
with patch.object(utils, 'assess_status_func') as asf:
|
||||
asf.return_value = 'assessor'
|
||||
utils._pause_resume_helper(f, 'some-config')
|
||||
asf.assert_called_once_with('some-config')
|
||||
# ports=None whilst port checks are disabled.
|
||||
f.assert_called_once_with('assessor', services='s1', ports=None)
|
||||
|
||||
def test_determine_purge_packages(self):
|
||||
'Ensure no packages are identified for purge prior to rocky'
|
||||
self.get_os_codename_package.return_value = 'queens'
|
||||
|
|
|
|||
Loading…
Reference in New Issue