openstack
/
charm-hacluster
Code Issues Proposed changes

Remove the corosync_rings check in eoan+ 

Corosync 2.99 altered the status output for udp/udpu rings to
be hardcoded to 'OK'. This breaks the check_corosync_rings nrpe
check which is looking for 'ring $number active with no faults'.
Since the value has been hardcoded to show 'OK', the check itself
does not provide any real meaningful value.

Change-Id: I642ecf11946b1ea791a27c54f0bec54adbfecb83
Closes-Bug: #1902919
This commit is contained in:
Billy Olsen 2020-11-06 14:20:00 -07:00
parent 0ce34b17be
commit 3080d64281
2 changed files with 34 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
hooks/hooks.py
View File

@ -56,6 +56,7 @@ from charmhelpers.core.host import (
service_running,
lsb_release,
CompareHostReleases,
get_distrib_codename,
)
from charmhelpers.contrib.network.ip import (
@ -616,10 +617,24 @@ def update_nrpe_config():
if nrpe.NRPE.does_nrpe_conf_dir_exist():
# corosync/crm checks
nrpe_setup.add_check(
shortname='corosync_rings',
description='Check Corosync rings {}'.format(current_unit),
check_cmd='check_corosync_rings')
# LP #1902919 - corosync version 2.99 changed the ring status output
# for udp/udpu to hardcode the status to always report 'OK'. This
# results in the check providing no value over what is provided by the
# crm_status check. A version check on the package would be more ideal,
# however populating the apt-cache object is expensive to run on each
# config-changed hook, so use the faster check of comparing the
# release name.
ring_check = {
'shortname': 'corosync_rings',
'description': 'Check Corosync rings {}'.format(current_unit),
'check_cmd': 'check_corosync_rings',
}
if CompareHostReleases(get_distrib_codename()) < 'eoan':
nrpe_setup.add_check(**ring_check)
else:
nrpe_setup.remove_check(**ring_check)
nrpe_setup.add_check(
shortname='crm_status',
description='Check crm status {}'.format(current_unit),

20
unit_tests/test_hacluster_hooks.py
View File

@ -545,6 +545,7 @@ class TestHooks(test_utils.CharmTestCase):
relation_id='relid1',
**{'pacemaker-key': 'pcmkrkey'})
@mock.patch.object(hooks, 'get_distrib_codename')
@mock.patch.object(hooks, 'apt_install')
@mock.patch('hooks.nrpe', autospec=True)
@mock.patch('hooks.os')
@ -552,13 +553,16 @@ class TestHooks(test_utils.CharmTestCase):
@mock.patch.object(hooks, 'status_set')
@mock.patch.object(hooks, 'config')
def test_update_nrpe_config(self, config, status_set, mock_glob, mock_os,
nrpe, apt_install):
nrpe, apt_install, mock_distrib_codename):
cfg = {'failed_actions_threshold': 0,
'res_failcount_warn': 0,
'res_failcount_crit': 5}
config.side_effect = lambda key: cfg.get(key)
mock_distrib_codename.side_effect = ['bionic', 'eoan', 'focal']
ring_check_expected = iter([True, False, False])
# Set up valid values to try for 'failed_actions_alert_type'
alert_type_params = ["IGNore", "warning", "CRITICAL"]
for alert_type in alert_type_params:
@ -579,10 +583,16 @@ class TestHooks(test_utils.CharmTestCase):
cfg['res_failcount_warn'],
cfg['res_failcount_crit']))
mock_nrpe_setup.add_check.assert_any_call(
shortname='corosync_rings',
description='Check Corosync rings nagios/1',
check_cmd='check_corosync_rings')
if next(ring_check_expected):
mock_nrpe_setup.add_check.assert_any_call(
shortname='corosync_rings',
description='Check Corosync rings nagios/1',
check_cmd='check_corosync_rings')
else:
mock_nrpe_setup.remove_check.assert_any_call(
shortname='corosync_rings',
description='Check Corosync rings nagios/1',
check_cmd='check_corosync_rings')
mock_nrpe_setup.add_check.assert_any_call(
shortname='crm_status',
description='Check crm status nagios/1',