From 169e5e5db3fa351ad1c7b0c2d31d6c387d38e1e5 Mon Sep 17 00:00:00 2001 From: Drew Freiberger Date: Thu, 22 Apr 2021 11:46:12 -0500 Subject: [PATCH 1/3] Add checks for openvswitch and helper function to enable the checks --- charmhelpers/contrib/charmsupport/nrpe.py | 32 ++++++++++ .../openstack/files/check_openvswitch.py | 62 +++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100755 charmhelpers/contrib/openstack/files/check_openvswitch.py diff --git a/charmhelpers/contrib/charmsupport/nrpe.py b/charmhelpers/contrib/charmsupport/nrpe.py index e4cb06bcd..a5185c236 100644 --- a/charmhelpers/contrib/charmsupport/nrpe.py +++ b/charmhelpers/contrib/charmsupport/nrpe.py @@ -29,6 +29,7 @@ import yaml from charmhelpers.core.hookenv import ( + DEBUG, config, hook_name, local_unit, @@ -509,6 +510,37 @@ def add_haproxy_checks(nrpe, unit_name): check_cmd='check_haproxy_queue_depth.sh') +def add_openvswitch_checks(nrpe, unit_name): + """ + Add checks for openvswitch + + :param NRPE nrpe: NRPE object to add check to + :param str unit_name: Unit name to use in check description + """ + enable_sudo_for_openvswitch_checks() + nrpe.add_check( + shortname='openvswitch', + description='Check Open vSwitch {%s}' % unit_name, + check_cmd='check_openvswitch.py') + + +def enable_sudo_for_openvswitch_checks(): + sudoers_dir = "/etc/sudoers.d" + sudoers_mode = 0o100440 + ovs_sudoers_file = "99-check_openvswitch" + ovs_sudoers_entry = "nagios ALL=(root) NOPASSWD: /usr/bin/ovs-vsctl show" + dest = os.path.join(sudoers_dir, ovs_sudoers_file) + try: + with open(dest, "w") as sudoer_file: + sudoer_file.write(ovs_sudoers_entry) + os.chmod(dest, sudoers_mode) + os.chown(dest, uid=0, gid=0) + except (OSError, IOError) as e: + log("Failed to setup sudoers file for check_openvswitch: {}".format(e)) + else: + log("Sudoers file for check_openvswitch installed: {}".format(dest), DEBUG) + + def remove_deprecated_check(nrpe, deprecated_services): """ Remove checks fro deprecated services in list diff --git a/charmhelpers/contrib/openstack/files/check_openvswitch.py b/charmhelpers/contrib/openstack/files/check_openvswitch.py new file mode 100755 index 000000000..b9c373d28 --- /dev/null +++ b/charmhelpers/contrib/openstack/files/check_openvswitch.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +# -*- coding: us-ascii -*- +"""Check for issues with NVME hardware devices.""" + +import argparse +import re +import subprocess +import sys + +from nagios_plugin3 import CriticalError, UnknownError, try_check + + +def parse_ovs_status(): + """Check for errors in 'ovs-vsctl show' output.""" + try: + cmd = ["/usr/bin/sudo", "/usr/bin/ovs-vsctl", "show"] + ovs_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT) + except subprocess.CalledProcessError as e: + raise UnknownError( + "UNKNOWN: Failed to query ovs: {}".format( + e.output.decode(errors="ignore").rstrip() + ) + ) + + ovs_vsctl_show_errors = [] + ovs_error_re = re.compile(r"^.*error: (?P.+)$", re.I) + for line in ovs_output.decode(errors="ignore").splitlines(): + m = ovs_error_re.match(line) + if m: + ovs_vsctl_show_errors.append(m.group("message")) + + if ovs_vsctl_show_errors: + numerrs = len(ovs_vsctl_show_errors) + raise CriticalError( + "CRITICAL: Found {} error(s) in ovs-vsctl show: " + "{}".format(numerrs, ", ".join(ovs_vsctl_show_errors)) + ) + + print("OK: no errors found in openvswitch") + + +def parse_args(argv=None): + """Process CLI arguments.""" + parser = argparse.ArgumentParser( + prog="check_openvswitch", + description=( + "this program checks openvswitch status and outputs an " + "appropriate Nagios status line" + ), + formatter_class=argparse.ArgumentDefaultsHelpFormatter, + ) + return parser.parse_args(argv) + + +def main(argv): + """Define main subroutine.""" + parse_args(argv) + try_check(parse_ovs_status) + + +if __name__ == "__main__": + main(sys.argv[1:]) From b60df8601a99353a7b12fc4028c7801e27699578 Mon Sep 17 00:00:00 2001 From: Drew Freiberger Date: Thu, 22 Apr 2021 15:45:55 -0500 Subject: [PATCH 2/3] Add unit tests for openvswitch nrpe check helpers --- tests/contrib/charmsupport/test_nrpe.py | 45 +++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/contrib/charmsupport/test_nrpe.py b/tests/contrib/charmsupport/test_nrpe.py index 334cf18a2..40d18334a 100644 --- a/tests/contrib/charmsupport/test_nrpe.py +++ b/tests/contrib/charmsupport/test_nrpe.py @@ -494,3 +494,48 @@ def test_copy_nrpe_checks_nrpe_files_dir(self): self.patched['copy2'].assert_called_once_with( 'filea', '/usr/local/lib/nagios/plugins/filea') + + def test_enable_sudo_for_openvswitch_checks(self): + error_msg = 'test failure' + nrpe.enable_sudo_for_openvswitch_checks() + + self.patched['open'].side_effect = IOError(error_msg) + nrpe.enable_sudo_for_openvswitch_checks() + self.patched['open'].side_effect = None + + self.patched['chmod'].side_effect = OSError(error_msg) + nrpe.enable_sudo_for_openvswitch_checks() + self.patched['chmod'].side_effect = None + + self.patched['chown'].side_effect = OSError(error_msg) + nrpe.enable_sudo_for_openvswitch_checks() + self.patched['chown'].side_effect = None + + expected_path = '/etc/sudoers.d/99-check_openvswitch' + expected_success_log = ("Sudoers file for check_openvswitch " + "installed: {}".format(expected_path)) + expected_failure_log = ("Failed to setup sudoers file for " + "check_openvswitch: {}".format(error_msg)) + self.patched['open'].assert_has_calls( + [call(expected_path, 'w')], any_order=True) + self.patched['chmod'].assert_has_calls( + [call(expected_path, 0o100440)], any_order=True) + self.patched['chown'].assert_has_calls( + [call(expected_path, uid=0, gid=0)], any_order=True) + self.patched['log'].assert_has_calls( + [call(expected_success_log, nrpe.DEBUG), + call(expected_failure_log), + call(expected_failure_log), + call(expected_failure_log)]) + self.check_call_counts(log=4, open=4, chmod=3, chown=2) + + @patch.object(nrpe, 'enable_sudo_for_openvswitch_checks') + @patch.object(nrpe.NRPE, 'add_check') + def test_add_openvswitch_checks(self, mock_nrpe_add_check, mock_enable_sudo): + foo = nrpe.NRPE() + nrpe.add_openvswitch_checks(foo, 'testunit') + mock_enable_sudo.assert_called_once() + mock_nrpe_add_check.assert_called_once_with( + shortname='openvswitch', + description='Check Open vSwitch {testunit}', + check_cmd='check_openvswitch.py') From 749a02438cc934dc76edab491ef314f2f18de6af Mon Sep 17 00:00:00 2001 From: Drew Freiberger Date: Fri, 23 Apr 2021 09:17:00 -0500 Subject: [PATCH 3/3] Updated check_openvswitch to use Interface table json --- charmhelpers/contrib/charmsupport/nrpe.py | 6 +- .../openstack/files/check_openvswitch.py | 66 ++++++++++++------- 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/charmhelpers/contrib/charmsupport/nrpe.py b/charmhelpers/contrib/charmsupport/nrpe.py index a5185c236..c78634767 100644 --- a/charmhelpers/contrib/charmsupport/nrpe.py +++ b/charmhelpers/contrib/charmsupport/nrpe.py @@ -520,7 +520,7 @@ def add_openvswitch_checks(nrpe, unit_name): enable_sudo_for_openvswitch_checks() nrpe.add_check( shortname='openvswitch', - description='Check Open vSwitch {%s}' % unit_name, + description='Check Open vSwitch {{{0}}}'.format(unit_name), check_cmd='check_openvswitch.py') @@ -528,7 +528,9 @@ def enable_sudo_for_openvswitch_checks(): sudoers_dir = "/etc/sudoers.d" sudoers_mode = 0o100440 ovs_sudoers_file = "99-check_openvswitch" - ovs_sudoers_entry = "nagios ALL=(root) NOPASSWD: /usr/bin/ovs-vsctl show" + ovs_sudoers_entry = ("# Juju owned - do not edit #\n" + "nagios ALL=(root) NOPASSWD: /usr/bin/ovs-vsctl " + "--format=json list Interface\n") dest = os.path.join(sudoers_dir, ovs_sudoers_file) try: with open(dest, "w") as sudoer_file: diff --git a/charmhelpers/contrib/openstack/files/check_openvswitch.py b/charmhelpers/contrib/openstack/files/check_openvswitch.py index b9c373d28..de45557b0 100755 --- a/charmhelpers/contrib/openstack/files/check_openvswitch.py +++ b/charmhelpers/contrib/openstack/files/check_openvswitch.py @@ -1,42 +1,62 @@ #!/usr/bin/env python3 -# -*- coding: us-ascii -*- -"""Check for issues with NVME hardware devices.""" +"""Check for known error statuses in OVS. + +This script currently checks through the Interface table in OVSDB +for errors. The script is named generically to allow for expanded +status checks in the future. +""" import argparse -import re +import json import subprocess import sys +# This depends on nagios_plugin3 being installed by charm-nrpe from nagios_plugin3 import CriticalError, UnknownError, try_check -def parse_ovs_status(): - """Check for errors in 'ovs-vsctl show' output.""" +def parse_ovs_interface_errors(): + """Check for errors in OVSDB Interface table.""" try: - cmd = ["/usr/bin/sudo", "/usr/bin/ovs-vsctl", "show"] + cmd = [ + "/usr/bin/sudo", + "/usr/bin/ovs-vsctl", + "--format=json", + "list", + "Interface", + ] ovs_output = subprocess.check_output(cmd, stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: raise UnknownError( - "UNKNOWN: Failed to query ovs: {}".format( - e.output.decode(errors="ignore").rstrip() + "UNKNOWN: OVS command '{}' failed. Output: {}".format( + " ".join(cmd), e.output.decode(errors="ignore").rstrip(), ) ) + ovs_interfaces = json.loads(ovs_output.decode(errors="ignore")) + ovs_interface_errors = [] + for i in ovs_interfaces["data"]: + # OVSDB internal data is formatted per RFC 7047 5.1 + iface = dict(zip(ovs_interfaces["headings"], i)) + error = iface["error"] + if isinstance(error, list) and len(error) == 2 and error[0] == "set": + # deserialize the set data into csv string elements + error = ",".join(error[1]) + if error: + ovs_interface_errors.append( + "Error on iface {}: {}".format(iface["name"], error) + ) - ovs_vsctl_show_errors = [] - ovs_error_re = re.compile(r"^.*error: (?P.+)$", re.I) - for line in ovs_output.decode(errors="ignore").splitlines(): - m = ovs_error_re.match(line) - if m: - ovs_vsctl_show_errors.append(m.group("message")) - - if ovs_vsctl_show_errors: - numerrs = len(ovs_vsctl_show_errors) + if ovs_interface_errors: raise CriticalError( - "CRITICAL: Found {} error(s) in ovs-vsctl show: " - "{}".format(numerrs, ", ".join(ovs_vsctl_show_errors)) + "CRITICAL: Found {} interface error(s) in OVSDB: " + "{}".format(len(ovs_interface_errors), ", ".join(ovs_interface_errors)) ) - print("OK: no errors found in openvswitch") + print( + "OK: No errors found across {} interfaces in openvswitch".format( + len(ovs_interfaces["data"]) + ) + ) def parse_args(argv=None): @@ -44,8 +64,8 @@ def parse_args(argv=None): parser = argparse.ArgumentParser( prog="check_openvswitch", description=( - "this program checks openvswitch status and outputs an " - "appropriate Nagios status line" + "this program checks openvswitch interface status and outputs " + "an appropriate Nagios status line" ), formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) @@ -55,7 +75,7 @@ def parse_args(argv=None): def main(argv): """Define main subroutine.""" parse_args(argv) - try_check(parse_ovs_status) + try_check(parse_ovs_interface_errors) if __name__ == "__main__":