bootstack-charmers team mailing list archive

[Joe Guo <joe.guo@xxxxxxxxxxxxx>]

Joe Guo has proposed merging ~guoqiao/charm-juju-lint:dev into charm-juju-lint:master.

Commit message:
This patch aims to:
1. make the charm work as minimal viable product.
2. complete nagios nrpe check

Requested reviews:
  Zachary Zehring (zzehring)

For more details, see:
https://code.launchpad.net/~guoqiao/charm-juju-lint/+git/charm-juju-lint/+merge/377959
-- 
Your team Canonical BootStack Charmers is subscribed to branch charm-juju-lint:master.

diff --git a/.gitignore b/.gitignore
index 6912fef..c2d0d6b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1 +1,3 @@
 deps/
+__pycache__/
+.tox/
diff --git a/config.yaml b/config.yaml
index ba623bf..286fbfe 100644
--- a/config.yaml
+++ b/config.yaml
@@ -60,3 +60,22 @@ options:
     default: "ppa:juju-linters/ppa"
   install_keys:
     default: "null"
+
+  nagios_context:
+    type: string
+    default: "juju"
+    description: |
+      Used by the nrpe-external-master subordinate charm.
+      A string that will be prepended to instance name to set the host name
+      in nagios. So for instance the hostname would be something like:
+      .
+          juju-myservice-0
+      .
+      If you're running multiple environments with the same services in them
+      this allows you to differentiate between them.
+  nagios_servicegroups:
+    type: string
+    default: ""
+    description: |
+      A comma-separated list of nagios servicegroups.
+      If left empty, the nagios_context will be used as the servicegroup.
diff --git a/layer.yaml b/layer.yaml
index 89f92b1..9021a46 100644
--- a/layer.yaml
+++ b/layer.yaml
@@ -1,13 +1,17 @@
 includes:
   - layer:basic
   - layer:apt
+  - interface:nrpe-external-master
 options:
   basic:
     packages:
-      - python3-virtualenv
         # Install "juju" Python package build dependencies.
       - libssl-dev
       - libffi-dev
+    python_packages:
+      # do not put it in wheelhouse.txt, since its deps need compilaton
+      - juju==2.7.1
+    use_venv: false
   apt:
     packages:
       - juju-lint
diff --git a/metadata.yaml b/metadata.yaml
index 383b7b9..f1c0fc7 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -18,9 +18,10 @@ series:
   - xenial
   - trusty
 subordinate: false
-# provides:
-#   provides-relation:
-#     interface: interface-name
+provides:
+  nrpe-external-master:
+    interface: nrpe-external-master
+    scope: container
 # requires:
 #   requires-relation:
 #     interface: interface-name
diff --git a/reactive/charm_juju_lint.py b/reactive/charm_juju_lint.py
index c3ad41c..23004eb 100644
--- a/reactive/charm_juju_lint.py
+++ b/reactive/charm_juju_lint.py
@@ -16,16 +16,16 @@ this program.  If not, see <https://www.gnu.org/licenses/>.
 """
 
 import json
+import subprocess
 from os import (
-    getenv,
-    mkdir,
+    makedirs,
     path,
 )
-import virtualenv
 
 from charmhelpers.core import hookenv
 from charmhelpers.core.host import rsync
 from charmhelpers.fetch.python import packages
+from charmhelpers.contrib.charmsupport import nrpe
 from charms.reactive import (
     when,
     when_not,
@@ -34,35 +34,33 @@ from charms.reactive import (
 
 USR_LIB = "/usr/lib/juju-lint"
 VAR_LIB = "/var/lib/juju-lint"
+LINT_RESULTS_FILE = path.join(VAR_LIB, 'lint-results.txt')
+NAGIOS_PLUGINS_DIR = '/usr/local/lib/nagios/plugins/'
 
 
 @when_not("charm-juju-lint.installed")
 def install_charm_juju_lint():
     create_install_dirs()
-    virtualenv.create_environment(USR_LIB)
-    packages.pip_install("juju", venv=USR_LIB)
     deploy_scripts()
     hookenv.status_set("active", "Unit is ready")
     set_flag("charm-juju-lint.installed")
 
 
 def create_install_dirs():
-    install_dirs = (USR_LIB, VAR_LIB)
+    install_dirs = (USR_LIB, VAR_LIB, NAGIOS_PLUGINS_DIR)
     for install_dir in install_dirs:
-        if not path.isdir(install_dir):
-            try:
-                mkdir(install_dir)
-            except OSError as error:
-                print("{} directory creation failed: {}".format(
-                    install_dir, error
-                ))
+        makedirs(install_dir, exist_ok=True)
 
 
 def deploy_scripts():
-    rsync(
-        path.join(getenv("CHARM_DIR"), "scripts", "auto_lint.py"),
-        path.join(USR_LIB, "auto_lint.py")
-    )
+    # install scripts
+    charm_dir = hookenv.charm_dir()
+    # NOTE: the trailing slash(/) here is important for rsync
+    # which means all content in dir other than dir itself.
+    scripts_dir = path.join(charm_dir, 'scripts/')
+    rsync(path.join(scripts_dir, "auto_lint.py"), path.join(USR_LIB, "auto_lint.py"))
+    # install all scripts/check_xxx files into nagios plugins dir
+    rsync(scripts_dir, NAGIOS_PLUGINS_DIR, options=["--include='check_*'"])
 
 
 @when("charm-juju-lint.installed", "config.changed")
@@ -95,3 +93,34 @@ def create_crontab():
     )
     with open("/etc/cron.d/juju-lint", "w") as fp:
         fp.write(cron_job)
+
+    # call it now so we have a lint-results.txt
+    # for check_juju_lint_results.py to check against
+    # it also acts as a smoke test for the cron job
+    try:
+        subprocess.check_call([path.join(USR_LIB, "auto_lint.py")])
+    except subprocess.CalledProcessError as e:
+        hookenv.log('fail to run auto_lint.py: {}'.format(e), 'ERROR')
+        hookenv.status_set('blocked', 'fail to run auto_lint.py')
+
+
+@when_not("juju-lint.lint-results-file.available")
+def check_lint_results_file():
+    if path.isfile(LINT_RESULTS_FILE):
+        set_flag("juju-lint.lint-results-file.available")
+        hookenv.status_set('active', 'Unit is ready')
+
+
+@when("juju-lint.lint-results-file.available", "nrpe-external-master.available")
+def update_nrpe_checks():
+    nrpe_client = nrpe.NRPE()
+
+    shortname = 'juju_lint_results'
+    nrpe_client.add_check(
+        shortname=shortname,
+        description='check juju lint results file genrated by juju-lint',
+        # class nrpe.Check will convert cmd to full path.
+        check_cmd='check_juju_lint_results.py --lint-results-file {}'.format(LINT_RESULTS_FILE)
+    )
+    hookenv.log('check_{} added'.format(shortname))
+    nrpe_client.write()
diff --git a/scripts/auto_lint.py b/scripts/auto_lint.py
index 3aacb16..d553d6d 100755
--- a/scripts/auto_lint.py
+++ b/scripts/auto_lint.py
@@ -1,4 +1,4 @@
-#!/usr/lib/juju-lint/bin/python3
+#!/usr/bin/env python3
 
 """
 # auto_lint.py
@@ -24,6 +24,7 @@ from os import path
 from subprocess import (
     CalledProcessError,
     check_output,
+    STDOUT,
 )
 from sys import exit
 
@@ -65,12 +66,15 @@ def lint_juju(auto_lint_config):
         path.join(VAR_LIB, "juju-status.json")
     ]
     try:
-        lint_results = check_output(command)
+        # juju-lint uses python logging as output which is printing to stderr
+        # to get lint results, we need to redirect stderr to STDOUT
+        lint_results = check_output(command, stderr=STDOUT).decode('utf8')
     except CalledProcessError as error:
-        print("juju-lint failed with the following error:", error)
-    else:
-        with open(path.join(VAR_LIB, "lint-results.txt"), "w") as fp:
-            fp.write(lint_results)
+        # write error into lint results file, so nagios can know
+        lint_results = "ERROR: juju-lint failed: {}".format(error.output.decode('utf8'))
+
+    with open(path.join(VAR_LIB, "lint-results.txt"), "w") as fp:
+        fp.write(lint_results)
 
 
 def main():
diff --git a/scripts/check_juju_lint_results.py b/scripts/check_juju_lint_results.py
new file mode 100755
index 0000000..7d9caab
--- /dev/null
+++ b/scripts/check_juju_lint_results.py
@@ -0,0 +1,79 @@
+#!/usr/bin/env python3
+
+"""
+# check_juju_lint_results.py
+
+Nagios plugin script to check juju lint results.
+
+Copyright (C) 2020 Canonical Ltd.
+Copyright (C) 2020 Joe Guo <joe.guo@xxxxxxxxxxxxx>
+
+This program is free software: you can redistribute it and/or modify it under
+the terms of the GNU General Public License version 3, as published by the Free
+Software Foundation.
+
+This program is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+PARTICULAR PURPOSE.  See the GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License along with
+this program.  If not, see <https://www.gnu.org/licenses/>.
+"""
+
+import argparse
+import sys
+from os.path import join
+
+VAR_LIB = "/var/lib/juju-lint"
+LINT_RESULTS_FILE = join(VAR_LIB, 'lint-results.txt')
+
+NAGIOS_STATUS_OK = 0
+NAGIOS_STATUS_WARNING = 1
+NAGIOS_STATUS_CRITICAL = 2
+NAGIOS_STATUS_UNKNOWN = 3
+
+NAGIOS_STATUS = {
+    NAGIOS_STATUS_OK: 'OK',
+    NAGIOS_STATUS_WARNING: 'WARNING',
+    NAGIOS_STATUS_CRITICAL: 'CRITICAL',
+    NAGIOS_STATUS_UNKNOWN: 'UNKNOWN',
+}
+
+
+def nagios_exit(status, message):
+    assert status in NAGIOS_STATUS, "Invalid Nagios status code"
+    # prefix status name to message
+    output = '{}: {}'.format(NAGIOS_STATUS[status], message)
+    print(output)  # nagios requires print to stdout, no stderr
+    sys.exit(status)
+
+
+def main():
+    parser = argparse.ArgumentParser(
+        description='check juju lint results',
+        # show default in help
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter)
+
+    parser.add_argument(
+        '-f', '--lint-results-file',
+        type=argparse.FileType(mode='r'),
+        dest='lint_results_file',
+        default=LINT_RESULTS_FILE,
+        help='juju lint results file to check, use `-` for stdin')
+
+    args = parser.parse_args()
+    lint_results = args.lint_results_file.read()
+
+    if 'ERROR' in lint_results or 'CRITICAL' in lint_results:
+        message = 'juju-lint errors\n{}'.format(lint_results)
+        nagios_exit(NAGIOS_STATUS_CRITICAL, message)
+    elif 'WARNING' in lint_results:
+        message = 'juju-lint warnings\n{}'.format(lint_results)
+        nagios_exit(NAGIOS_STATUS_WARNING, message)
+    else:
+        message = 'juju lint is happy\n{}'.format(lint_results)
+        nagios_exit(NAGIOS_STATUS_OK, message)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/test-requirements.txt b/test-requirements.txt
new file mode 100644
index 0000000..3930480
--- /dev/null
+++ b/test-requirements.txt
@@ -0,0 +1 @@
+flake8
diff --git a/tox.ini b/tox.ini
new file mode 100644
index 0000000..4e4d2a2
--- /dev/null
+++ b/tox.ini
@@ -0,0 +1,17 @@
+[tox]
+envlist = py3,pep8
+skipsdist = True
+
+[testenv]
+basepython = python3
+deps =
+    -rwheelhouse.txt
+    -rtest-requirements.txt
+
+[testenv:py3]
+commands =
+    python -m unittest unit_tests.test_juju_lint
+
+[testenv:pep8]
+commands =
+    python -m flake8 --verbose --max-line-length=120
diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py
new file mode 100644
index 0000000..958e747
--- /dev/null
+++ b/unit_tests/__init__.py
@@ -0,0 +1,5 @@
+#!/usr/bin/env python3
+import sys
+from os.path import abspath, dirname
+CURRENT_DIR = dirname(abspath(__file__))
+sys.path.insert(0, CURRENT_DIR)
diff --git a/unit_tests/lint_results/CRITICAL.txt b/unit_tests/lint_results/CRITICAL.txt
new file mode 100644
index 0000000..3e29a1e
--- /dev/null
+++ b/unit_tests/lint_results/CRITICAL.txt
@@ -0,0 +1,3 @@
+CRITICAL: this is critical
+ERROR: this is error
+WARNING: this is warning
diff --git a/unit_tests/lint_results/ERROR.txt b/unit_tests/lint_results/ERROR.txt
new file mode 100644
index 0000000..3103101
--- /dev/null
+++ b/unit_tests/lint_results/ERROR.txt
@@ -0,0 +1,4 @@
+ERROR: this is error
+WARNING: this is warning
+INFO: this is info
+DEBUG: this is debug
diff --git a/unit_tests/lint_results/INFO.txt b/unit_tests/lint_results/INFO.txt
new file mode 100644
index 0000000..1896f76
--- /dev/null
+++ b/unit_tests/lint_results/INFO.txt
@@ -0,0 +1,2 @@
+INFO: this is info
+DEBUG: this is debug
diff --git a/unit_tests/lint_results/WARNING.txt b/unit_tests/lint_results/WARNING.txt
new file mode 100644
index 0000000..aa26f36
--- /dev/null
+++ b/unit_tests/lint_results/WARNING.txt
@@ -0,0 +1,3 @@
+WARNING: this is warning
+INFO: this is info
+DEBUG: this is debug
diff --git a/unit_tests/test_juju_lint.py b/unit_tests/test_juju_lint.py
new file mode 100755
index 0000000..44925e9
--- /dev/null
+++ b/unit_tests/test_juju_lint.py
@@ -0,0 +1,49 @@
+#!/usr/bin/env python3
+
+import unittest
+import subprocess
+from os.path import abspath, dirname, join
+
+CURRENT_DIR = dirname(abspath(__file__))
+CHARM_DIR = dirname(CURRENT_DIR)
+SCRIPT_DIR = join(CHARM_DIR, 'scripts')
+LINT_RESULTS_DIR = join(CURRENT_DIR, 'lint_results')
+CHECK_JUJU_LINT_RESULTS_PATH = join(SCRIPT_DIR, 'check_juju_lint_results.py')
+
+
+def get_lint_results_file(name):
+    """
+    Get full file path from name.
+
+    ERROR.txt --> /path/to/unit_tests/lint_results/ERROR.txt
+    """
+    return join(LINT_RESULTS_DIR, name)
+
+
+def get_check_exit_code(name):
+    """
+    Return check script exit code for a file name.
+    """
+
+    cmd = '{} --lint-results-file {}'.format(
+        CHECK_JUJU_LINT_RESULTS_PATH, get_lint_results_file(name))
+    try:
+        subprocess.check_call(cmd.split())
+    except subprocess.CalledProcessError as e:
+        return e.returncode
+    return 0
+
+
+class CheckScriptTestCase(unittest.TestCase):
+
+    def test_info(self):
+        self.assertEqual(get_check_exit_code('INFO.txt'), 0)
+
+    def test_warning(self):
+        self.assertEqual(get_check_exit_code('WARNING.txt'), 1)
+
+    def test_error(self):
+        self.assertEqual(get_check_exit_code('ERROR.txt'), 2)
+
+    def test_critial(self):
+        self.assertEqual(get_check_exit_code('CRITICAL.txt'), 2)
diff --git a/wheelhouse.txt b/wheelhouse.txt
new file mode 100644
index 0000000..2543182
--- /dev/null
+++ b/wheelhouse.txt
@@ -0,0 +1,9 @@
+# layer-basic/wheelhouse.txt already have these
+# but too old, override with new versions
+charms.reactive==1.3.0
+charmhelpers==0.20.7
+
+# this is the pip package for python-libjuju, required by charm-juju-lint
+# its deps need compilation so we have to exclude it from wheelhouse.txt
+# instead, we list it under layer-basic:python_packages option in layer.yaml
+# juju==2.7.1