Merge ~afreiberger/charm-openstack-service-checks/+git/charm-openstack-service-checks:lint-20.10 into ~llama-charmers/charm-openstack-service-checks:master

[Drew Freiberger]

[Drew Freiberger (afreiberger)]

Someone besides me, please run full tests on this before final approval/merging. unit/lint passing, and I've run functional to death on my environment.

You'll note that I punted D102 and D103s because I didn't want to waste any more release rotation time on helper library and test library document strings more than I already have. There are two files that would trigger on those checks and they can be future TODOs.

[Alvaro Uria (aluria)]

I'm running into a weird issue. The deploy_openstack func test fails because app.status for neutron-api returns "blocked". However, "juju status -m functest-<uuid> neutron-api --format yaml" returns an app status "active".

I've been trying to troubleshoot it without success. Juju version is 2.8.2. I'm going to try to upgrade to 2.8.5 and test again.

[Alvaro Uria (aluria)]

Upgrading the controller to 2.8.5 made the neutron-api unit go from waiting to active as it was expected. I'm running all tests now under the upgraded controller.

[Alvaro Uria (aluria)]

lgtm

review: Approve

diff --git a/src/actions/actions.py b/src/actions/actions.py
index 1e679b6..f165aaa 100755
--- a/src/actions/actions.py
+++ b/src/actions/actions.py
@@ -12,28 +12,30 @@
 # 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.
+"""Define charm actions."""
 
 import os
 import sys
+from traceback import format_exc
+
 import charmhelpers.core.hookenv as hookenv
 import charmhelpers.core.unitdata as unitdata
-from traceback import format_exc
 
 # Load modules from $CHARM_DIR/lib
 sys.path.append("lib")
 
 import charms.reactive  # NOQA: E402
 from charms.layer import basic  # NOQA: E402
+from charms.reactive.flags import clear_flag  # NOQA: E402
 
 basic.bootstrap_charm_deps()
 basic.init_config_states()
 
-from charms.reactive.flags import clear_flag  # NOQA: E402
-
 
 def refresh_endpoint_checks(*args):
-    """Clear the openstack-service-checks.endpoints.configured flag
-    so that next time update-status runs, the Keystone catalog is re-read
+    """Clear the openstack-service-checks.endpoints.configured flag.
+
+    Ensures next time update-status runs, the Keystone catalog is re-read
     and nrpe checks refreshed.
     """
     clear_flag("openstack-service-checks.endpoints.configured")
@@ -47,6 +49,7 @@ ACTIONS = {
 
 
 def main(args):
+    """Parse the action hook and call the related function."""
     action_name = os.path.basename(args[0])
     try:
         action = ACTIONS[action_name]
diff --git a/src/files/plugins/check_cinder_services.py b/src/files/plugins/check_cinder_services.py
index 004b1be..5939db7 100755
--- a/src/files/plugins/check_cinder_services.py
+++ b/src/files/plugins/check_cinder_services.py
@@ -1,14 +1,17 @@
 #!/usr/bin/env python3
+"""Perform cinder services nagios checks."""
 
 import argparse
-import nagios_plugin3
 import os
-import os_client_config
 import subprocess
 
+import nagios_plugin3
+
+import os_client_config
+
 
 def check_status(service):
-    """Checks attributes of services and reports issues (or OK message).
+    """Check attributes of services and reports issues (or OK message).
 
     Attributes are (among others):
         - binary: cinder-volume, cinder-scheduler
@@ -28,7 +31,7 @@ def check_status(service):
 
 
 def check_cinder_services(args, cinder):
-    """Retrieves list of services and returns appropriate nagios return code."""
+    """Retrieve list of services and returns appropriate nagios return code."""
     services = cinder.get("/os-services").json()["services"]
     if not services:
         output = "UNKNOWN: No cinder services found"
diff --git a/src/files/plugins/check_contrail_analytics_alarms.py b/src/files/plugins/check_contrail_analytics_alarms.py
index aa12a3e..feae07a 100755
--- a/src/files/plugins/check_contrail_analytics_alarms.py
+++ b/src/files/plugins/check_contrail_analytics_alarms.py
@@ -1,17 +1,20 @@
 #!/usr/bin/env python3
+"""Perform Contrail Analytics nagios checks."""
 
 import argparse
 import collections
 import datetime
 import ipaddress
 import os
-import os_client_config
 import re
-import requests
 import subprocess
 
 import nagios_plugin3
 
+import os_client_config
+
+import requests
+
 DEFAULT_IGNORED = r""
 Alarm = collections.namedtuple("Alarm", "ts, desc")
 
@@ -111,7 +114,7 @@ def check_contrail_alarms(contrail_vip, token, **kwargs):
 
 
 def load_os_envvars(args):
-    # grab environment vars
+    """Process environment variables."""
     command = ["/bin/bash", "-c", "source {} && env".format(args.env)]
     proc = subprocess.Popen(command, stdout=subprocess.PIPE)
     for line in proc.stdout:
@@ -121,6 +124,7 @@ def load_os_envvars(args):
 
 
 def validate_ipv4(ipv4_addr):
+    """Test if IPv4 address is valid."""
     try:
         ipaddress.IPv4Address(ipv4_addr)
     except ipaddress.AddressValueError:
@@ -130,6 +134,7 @@ def validate_ipv4(ipv4_addr):
 
 
 def main():
+    """Define main routine, process CLI args, and run checks."""
     parser = argparse.ArgumentParser(description="Check Contrail alarms")
     parser.add_argument(
         "--env",
diff --git a/src/files/plugins/check_nova_services.py b/src/files/plugins/check_nova_services.py
index 5da5955..4ac1588 100755
--- a/src/files/plugins/check_nova_services.py
+++ b/src/files/plugins/check_nova_services.py
@@ -1,18 +1,23 @@
 #!/usr/bin/env python3
+"""Define nagios check to determine if openstack compute services are impacted."""
 
 import argparse
-import nagios_plugin3
 import os
-import os_client_config
 import subprocess
 
+import nagios_plugin3
+
+import os_client_config
+
 
 def check_hosts_up(args, aggregate, hosts, services_compute):
-    # function to check an agg
-    # in: list of hosts
-    # in: services_compute
-    # in: args
-    # out: dict, msg_text, status
+    """Check that aggregates have a minimum number of hosts active.
+
+    :in: list of hosts
+    :in: services_compute
+    :in: args
+    :return: dict, msg_text, status
+    """
     status_crit = False
     status_warn = False
     counts = {"down": 0, "disabled": 0, "ok": 0}
@@ -52,6 +57,7 @@ def check_hosts_up(args, aggregate, hosts, services_compute):
 
 
 def check_nova_services(args, nova):
+    """Define nagios service checks for nova services."""
     aggregates = nova.get("/os-aggregates").json()["aggregates"]
     services = nova.get("/os-services").json()["services"]
     services_compute = [x for x in services if x["binary"] == "nova-compute"]
diff --git a/src/files/plugins/check_octavia.py b/src/files/plugins/check_octavia.py
index c7e4d02..c0081fd 100755
--- a/src/files/plugins/check_octavia.py
+++ b/src/files/plugins/check_octavia.py
@@ -1,13 +1,14 @@
 #!/usr/bin/env python3
+"""Define nagios checks for octavia services."""
 
 import argparse
 import collections
-from datetime import datetime, timedelta
 import json
 import os
 import re
 import subprocess
 import sys
+from datetime import datetime, timedelta
 
 import openstack
 
@@ -28,9 +29,7 @@ NAGIOS_STATUS = {
 
 
 def filter_checks(alarms, ignored=DEFAULT_IGNORED):
-    """
-    Reduce all checks down to an overall check based on the highest level
-    not ignored
+    """Reduce results down to an overall check based on the highest level not ignored.
 
     :param List[Tuple] alarms: list of alarms (lvl, message)
     :param str ignored:        regular expression of messages to ignore
@@ -59,7 +58,7 @@ def filter_checks(alarms, ignored=DEFAULT_IGNORED):
 
 
 def nagios_exit(args, results):
-    # parse ignored list
+    """Filter ignored checks and ensure proper nagios check return code."""
     unique = sorted(filter(None, set(args.ignored.split(","))))
     ignored_re = r"|".join("(?:{})".format(_) for _ in unique)
 
@@ -71,8 +70,7 @@ def nagios_exit(args, results):
 
 
 def check_loadbalancers(connection):
-    """check loadbalancers status."""
-
+    """Check loadbalancers status."""
     lb_mgr = connection.load_balancer
     lb_all = lb_mgr.load_balancers()
 
@@ -128,7 +126,7 @@ def check_loadbalancers(connection):
 
 
 def check_pools(connection):
-    """check pools status."""
+    """Check pools status."""
     lb_mgr = connection.load_balancer
     pools_all = lb_mgr.pools()
 
@@ -171,8 +169,7 @@ def check_pools(connection):
 
 
 def check_amphorae(connection):
-    """check amphorae status."""
-
+    """Check amphorae status."""
     lb_mgr = connection.load_balancer
 
     resp = lb_mgr.get("/v2/octavia/amphorae")
@@ -210,6 +207,7 @@ def check_amphorae(connection):
 
 
 def check_image(connection, tag, days):
+    """Check that there is an image with the proper octavia image tag."""
     img_mgr = connection.image
     images = list(img_mgr.images(tag=tag))
 
@@ -245,8 +243,12 @@ def check_image(connection, tag, days):
 
 
 def process_checks(args):
-    # use closure to make all checks have same signature
-    # so we can handle them in same way
+    """Process all octavia checks in a standardized manner.
+
+    Use closure to make all checks have same signature
+    so we can handle them in same way
+    """
+
     def _check_image(_connection):
         return check_image(_connection, args.amp_image_tag, args.amp_image_days)
 
@@ -262,6 +264,7 @@ def process_checks(args):
 
 
 def main():
+    """Define main routine, parse CLI args, and run checks."""
     parser = argparse.ArgumentParser(
         description="Check Octavia status",
         formatter_class=argparse.ArgumentDefaultsHelpFormatter,
diff --git a/src/files/plugins/check_rally.py b/src/files/plugins/check_rally.py
index fac835b..73dbf8f 100755
--- a/src/files/plugins/check_rally.py
+++ b/src/files/plugins/check_rally.py
@@ -1,10 +1,12 @@
 #!/usr/bin/env python3
+"""Perform rally testing for nagios alerting."""
+
 import collections
+import json
 import os
 import re
 import sys
 
-import json
 
 # ie. {0} tempest.test.test1 ... success
 TEMPEST_TEST_RE = r"{\d+} [.\w]+ ... (\w+)"
@@ -12,6 +14,7 @@ INPUT_FILE = "/home/nagiososc/rally.status"
 
 
 def print_results(results):
+    """Display results of the checks."""
     status_message = collections.defaultdict(lambda: "UNKNOWN")  # 3
     status_message.update(
         {
@@ -65,22 +68,21 @@ def print_results(results):
             break
 
     if len(summary) > 0:
-        print(
-            "{}: ".format(nagios_status)
-            + ", ".join(
-                [
-                    "{}[{}]".format(status_msg, summary[status_msg])
-                    for status_msg in sorted(
-                        summary, key=lambda status: summary[status], reverse=True
-                    )
-                ]
-            )
+        details = ", ".join(
+            [
+                "{}[{}]".format(status_msg, summary[status_msg])
+                for status_msg in sorted(
+                    summary, key=lambda status: summary[status], reverse=True
+                )
+            ]
         )
+        print("{}: {}".format(nagios_status, details))
     print("\n".join(output))
     return rc
 
 
 def main(results_filename):
+    """Define main routine, parse arguments, run checks."""
     if not os.path.exists(results_filename):
         print("UNKNOWN: {} does not exist".format(results_filename))
         return 3
diff --git a/src/files/run_rally.py b/src/files/run_rally.py
index 3f0802e..4543e64 100755
--- a/src/files/run_rally.py
+++ b/src/files/run_rally.py
@@ -1,11 +1,12 @@
 #!/usr/bin/env python3
+"""Run rally tests."""
+
 import datetime
 import json
 import os
+import shutil
 import subprocess
 import sys
-
-import shutil
 import tempfile
 
 OUTPUT_FILE = "/home/nagiososc/rally.status"
@@ -13,6 +14,7 @@ HISTORY_FOLDER = "/home/nagiososc/rallystatuses"
 
 
 def get_backup_output_filename():
+    """Create path and define backup filename based on date."""
     if not os.path.isdir(HISTORY_FOLDER):
         os.mkdir(HISTORY_FOLDER, mode=0o755)
 
@@ -56,6 +58,7 @@ def _load_envvars(novarc="/var/lib/nagios/nagios.novarc"):
 
 
 def main(testfile="/home/nagiososc/ostests.txt"):
+    """Perform rally test main routine."""
     if not _load_envvars():
         print("UNKNOWN: could not load OS_ envvars")
         sys.exit(3)
diff --git a/src/lib/lib_openstack_service_checks.py b/src/lib/lib_openstack_service_checks.py
index 405120d..85ed3eb 100644
--- a/src/lib/lib_openstack_service_checks.py
+++ b/src/lib/lib_openstack_service_checks.py
@@ -1,4 +1,7 @@
+"""Helper library for openstack-service-checks charm."""
+
 import collections
+import configparser
 import glob
 import os
 import pwd
@@ -6,31 +9,41 @@ import re
 import subprocess
 from urllib.parse import urlparse
 
-import configparser
 
 from charmhelpers import fetch
-from charmhelpers.core.templating import render
+from charmhelpers.contrib.charmsupport.nrpe import NRPE
 from charmhelpers.contrib.openstack.utils import config_flags_parser
 from charmhelpers.core import hookenv, host, unitdata
-from charmhelpers.contrib.charmsupport.nrpe import NRPE
+from charmhelpers.core.templating import render
+
 from charms.reactive import any_file_changed
+
 import keystoneauth1
+
 from keystoneclient import session
 
 
 class OSCCredentialsError(Exception):
+    """Define OSCCredentialError exception."""
+
     pass
 
 
 class OSCKeystoneError(Exception):
+    """Define OSCKeystoneError exception."""
+
     @property
     def workload_status(self):
+        """Implement workload_status method from Exception class."""
         raise NotImplementedError
 
 
 class OSCKeystoneServerError(OSCKeystoneError):
+    """Define OSCKeystoneServerError exception."""
+
     @property
     def workload_status(self):
+        """Implement workload_status method from Exception class."""
         return (
             "Keystone server error was encountered trying to list keystone "
             "resources. Check keystone server health. "
@@ -39,8 +52,11 @@ class OSCKeystoneServerError(OSCKeystoneError):
 
 
 class OSCKeystoneClientError(OSCKeystoneError):
+    """Define OSCKeystoneClientError exception."""
+
     @property
     def workload_status(self):
+        """Implement workload_status method from Exception class."""
         return (
             "Keystone client request error was encountered trying to "
             "keystone resources. Check keystone auth creds and url."
@@ -49,8 +65,11 @@ class OSCKeystoneClientError(OSCKeystoneError):
 
 
 class OSCSslError(OSCKeystoneError):
+    """Define OSCSslError exception."""
+
     @property
     def workload_status(self):
+        """Implement workload_status method from Exception class."""
         return (
             "SSL error was encountered when requesting Keystone for "
             "resource list.  Check trusted_ssl_ca config option. "
@@ -59,22 +78,27 @@ class OSCSslError(OSCKeystoneError):
 
 
 class OSCHelper:
+    """Define OSCHelper object."""
+
     def __init__(self):
+        """Initialize charm configs and null keystone client into Helper object."""
         self.charm_config = hookenv.config()
         self._keystone_client = None
 
     def store_keystone_credentials(self, creds):
-        """store keystone credentials"""
+        """Store keystone credentials."""
         kv = unitdata.kv()
         kv.set("keystonecreds", creds)
         kv.set("rallyinstalled", False)
 
     @property
     def novarc(self):
+        """Define path to novarc config file for checks."""
         return "/var/lib/nagios/nagios.novarc"
 
     @property
     def contrail_analytics_vip(self):
+        """Expose the contrail_analytics_vip charm config value."""
         return self.charm_config["contrail_analytics_vip"]
 
     @property
@@ -179,7 +203,7 @@ class OSCHelper:
         return creds
 
     def get_keystone_credentials(self):
-        """Retrieve keystone credentials from either config or relation data
+        """Retrieve keystone credentials from either config or relation data.
 
         If config 'os-crendentials' is set, return that info otherwise look
         for a keystonecreds relation data'
@@ -356,7 +380,9 @@ class OSCHelper:
         self.create_endpoint_checks()
 
     def _split_url(self, netloc, scheme):
-        """http(s)://host:port or http(s)://host will return a host and a port
+        """Split URL and return host and port tuple.
+
+        http(s)://host:port or http(s)://host will return a host and a port
 
         Even if a port is not specified, this helper will return a host and a port
         (guessing it from the protocol used, if needed)
@@ -709,7 +735,9 @@ class OSCHelper:
             config.write(fd)
 
     def reconfigure_tempest(self):
-        """Expects an external network already configured, and enables cinder tests
+        """Enable cinder tests.
+
+        Expects an external network already configured
 
         Sample:
         RALLY_VERIFIER=7b9d06ef-e651-4da3-a56b-ecac67c595c5
@@ -717,8 +745,8 @@ class OSCHelper:
         RALLY_DEPLOYMENT=a75657c6-9eea-4f00-9117-2580fe056a80
         RALLY_ENV=a75657c6-9eea-4f00-9117-2580fe056a80
         """
-        RALLY_CONF = ["/home", self._rallyuser, "snap", "fcbtest", "current", ".rally"]
-        rally_globalconfig = os.path.join(*RALLY_CONF, "globals")
+        rally_conf = ["/home", self._rallyuser, "snap", "fcbtest", "current", ".rally"]
+        rally_globalconfig = os.path.join(*rally_conf, "globals")
         if not os.path.isfile(rally_globalconfig):
             return False
 
@@ -730,7 +758,7 @@ class OSCHelper:
                     uuids[key] = value
 
         tempest_path = os.path.join(
-            *RALLY_CONF,
+            *rally_conf,
             "verification",
             "verifier-{RALLY_VERIFIER}".format(**uuids),
             "for-deployment-{RALLY_DEPLOYMENT}".format(**uuids),
diff --git a/src/reactive/openstack_service_checks.py b/src/reactive/openstack_service_checks.py
index ede8c79..9c99652 100644
--- a/src/reactive/openstack_service_checks.py
+++ b/src/reactive/openstack_service_checks.py
@@ -1,6 +1,7 @@
 # -*- coding: us-ascii -*-
 """
-install_osc +------------------------> render_config+--------------> do_restart
+install_osc +------------------------> render_config+--------------> do_restart.
+
                                           ^       +                      ^
 conf_kst_user+---> save_creds +-----------+       v                      |
                                                   create_endpoints +-----+
@@ -19,6 +20,7 @@ import base64
 import subprocess
 
 from charmhelpers.core import hookenv, host, unitdata
+
 from charms.reactive import (
     any_flags_set,
     clear_flag,
@@ -29,8 +31,8 @@ from charms.reactive import (
 )
 
 from lib_openstack_service_checks import (
-    OSCHelper,
     OSCCredentialsError,
+    OSCHelper,
     OSCKeystoneError,
 )
 
@@ -40,13 +42,14 @@ helper = OSCHelper()
 
 @when("config.changed")
 def config_changed():
+    """Clear configured flag to trigger update of configs."""
     clear_flag("openstack-service-checks.configured")
 
 
 @when_not("openstack-service-checks.installed")
 @when("nrpe-external-master.available")
 def install_openstack_service_checks():
-    """Entry point to start configuring the unit
+    """Start configuring the unit.
 
     Triggered if related to the nrpe-external-master relation.
     Some relation data can be initialized if the application is related to
@@ -59,7 +62,7 @@ def install_openstack_service_checks():
 @when_not("identity-credentials.available")
 @when("identity-credentials.connected")
 def configure_ident_username(keystone):
-    """Requests a user to the Identity Service"""
+    """Request a user to be created by the Identity Service."""
     username = "nagios"
     keystone.request_credentials(username)
     clear_flag("openstack-service-checks.stored-creds")
@@ -125,7 +128,7 @@ def update_keystone_store():
 
 
 def get_credentials():
-    """Get credential info from either config or relation data
+    """Get credential info from either config or relation data.
 
     If config 'os-credentials' is set, return it. Otherwise look for a
     keystonecreds relation data.
@@ -146,7 +149,7 @@ def get_credentials():
 @when("openstack-service-checks.installed")
 @when_not("openstack-service-checks.configured")
 def render_config():
-    """Render nrpe checks from the templates
+    """Render nrpe checks from the templates.
 
     This code is only triggered after the nrpe relation is set. If a relation
     with keystone is later set, it will be re-triggered. On the other hand,
@@ -227,12 +230,14 @@ def configure_nrpe_endpoints():
 
 @when("identity-notifications.available.updated")
 def endpoints_changed():
+    """Clear configured flag if endpoints are updated."""
     clear_flag("openstack-service-checks.endpoints.configured")
 
 
 @when("openstack-service-checks.configured")
 @when_not("openstack-service-checks.started")
 def do_restart():
+    """Restart services when configuration has changed and not started."""
     hookenv.log("Reloading nagios-nrpe-server")
     host.service_restart("nagios-nrpe-server")
     set_flag("openstack-service-checks.started")
@@ -241,11 +246,13 @@ def do_restart():
 @when("openstack-service-checks.started")
 @when("openstack-service-checks.endpoints.configured")
 def set_active():
+    """Update unit status to active."""
     hookenv.status_set("active", "Unit is ready")
 
 
 @when("nrpe-external-master.available")
 def do_reconfigure_nrpe():
+    """Trigger NRPE relation reconfiguration."""
     os_credentials_flag = "config.changed.os-credentials"
     flags = [
         "config.changed.check_{}_urls".format(interface)
@@ -270,7 +277,7 @@ def do_reconfigure_nrpe():
 
 @when_not("nrpe-external-master.available")
 def missing_nrpe():
-    """Avoid a user action to be missed or overwritten by another hook"""
+    """Set a blocked status if awaiting nrpe relation."""
     if hookenv.hook_name() != "update-status":
         hookenv.status_set("blocked", "Missing relations: nrpe")
 
@@ -278,6 +285,7 @@ def missing_nrpe():
 @when("openstack-service-checks.installed")
 @when("nrpe-external-master.available")
 def parse_hooks():
+    """Catch upgrade-charm, update kv stores, and trigger reconfig."""
     if hookenv.hook_name() == "upgrade-charm":
         # Check if creds storage needs to be migrated
         # Old key: keystone-relation-creds
diff --git a/src/tests/functional/conftest.py b/src/tests/functional/conftest.py
index 948059b..5d19e09 100644
--- a/src/tests/functional/conftest.py
+++ b/src/tests/functional/conftest.py
@@ -1,5 +1,4 @@
-"""
-Reusable pytest fixtures for functional testing
+"""Initialize reusable pytest fixtures for functional testing.
 
 Environment variables
 ---------------------
@@ -11,14 +10,16 @@ if set, the testing model won't be torn down at the end of the testing session
 import asyncio
 import json
 import os
+import subprocess
 import uuid
 
-import pytest
-import subprocess
 import juju
 from juju.controller import Controller
 from juju.errors import JujuError
 
+import pytest
+
+
 STAT_CMD = """python3 - <<EOF
 import json
 import os
@@ -39,8 +40,7 @@ EOF
 
 @pytest.fixture(scope="module")
 def event_loop():
-    """Override the default pytest event loop to allow for fixtures using a
-    broader scope"""
+    """Override default pytest event loop to allow for fixtures using broader scope."""
     loop = asyncio.get_event_loop_policy().new_event_loop()
     asyncio.set_event_loop(loop)
     loop.set_debug(True)
@@ -51,7 +51,7 @@ def event_loop():
 
 @pytest.fixture(scope="module")
 async def controller():
-    """Connect to the current controller"""
+    """Connect to the current controller."""
     _controller = Controller()
     await _controller.connect_current()
     yield _controller
@@ -60,7 +60,7 @@ async def controller():
 
 @pytest.fixture(scope="module")
 async def model(controller):
-    """This model lives only for the duration of the test"""
+    """Create model which lives only for the duration of the test."""
     model_name = os.getenv("PYTEST_MODEL")
     if model_name is None:
         model_name = "functest-{}".format(str(uuid.uuid4())[-12:])
@@ -88,7 +88,7 @@ async def model(controller):
 
 @pytest.fixture
 async def get_app(model):
-    """Returns the application requested"""
+    """Return the application requested."""
 
     async def _get_app(name):
         try:
@@ -101,7 +101,7 @@ async def get_app(model):
 
 @pytest.fixture
 async def get_unit(model):
-    """Returns the requested <app_name>/<unit_number> unit"""
+    """Return the requested <app_name>/<unit_number> unit."""
 
     async def _get_unit(name):
         try:
@@ -115,7 +115,7 @@ async def get_unit(model):
 
 @pytest.fixture
 async def get_entity(get_unit, get_app):
-    """Returns a unit or an application"""
+    """Return a unit or an application."""
 
     async def _get_entity(name):
         try:
@@ -131,8 +131,7 @@ async def get_entity(get_unit, get_app):
 
 @pytest.fixture
 async def run_command(get_unit):
-    """
-    Runs a command on a unit.
+    """Run a command on a unit.
 
     :param cmd: Command to be run
     :param target: Unit object or unit name string
@@ -148,8 +147,7 @@ async def run_command(get_unit):
 
 @pytest.fixture
 async def file_stat(run_command):
-    """
-    Runs stat on a file
+    """Run stat on a file.
 
     :param path: File path
     :param target: Unit object or unit name string
@@ -168,8 +166,7 @@ async def file_stat(run_command):
 
 @pytest.fixture
 async def file_contents(run_command):
-    """
-    Returns the contents of a file
+    """Return the contents of a file.
 
     :param path: File path
     :param target: Unit object or unit name string
@@ -185,7 +182,7 @@ async def file_contents(run_command):
 
 @pytest.fixture
 async def reconfigure_app(get_app, model):
-    """Applies a different config to the requested app"""
+    """Apply a different config to the requested app."""
 
     async def _reconfigure_app(cfg, target):
         application = (
diff --git a/src/tests/functional/test_deploy.py b/src/tests/functional/test_deploy.py
index e7d5782..4656232 100644
--- a/src/tests/functional/test_deploy.py
+++ b/src/tests/functional/test_deploy.py
@@ -1,3 +1,5 @@
+"""Test deployment of openstack-service-checks charm."""
+
 import asyncio
 import os
 
@@ -53,7 +55,10 @@ class ActionFailed(Exception):
 
 
 class Agent:
+    """Define the pytest model management agent."""
+
     def __init__(self, unit):
+        """Initialize the unit the agent should act on."""
         self.unit = unit
 
     async def _act(self, action, **kwargs):
@@ -158,7 +163,7 @@ async def deploy_app(request, deploy_openstack, model):
     # Starts a deploy for each series
     if (
         apps["nrpe"] in model.applications
-        and apps["openstack-service-checks"] in model.applications
+        and apps["openstack-service-checks"] in model.applications  # noqa:W503
     ):
         yield model.applications[apps["openstack-service-checks"]]
         return
@@ -473,8 +478,8 @@ async def test_openstackservicechecks_invalid_keystone_workload_status(
 
     # Wait for osc app to block with expected workload-status
     await agent.block_until(lambda: agent.status("blocked"))
-    assert (
-        agent.unit.workload_status_message
-        == "Keystone server error was encountered trying to list keystone "
+    expected_msg = (
+        "Keystone server error was encountered trying to list keystone "
         "resources. Check keystone server health. View juju logs for more info."
     )
+    assert agent.unit.workload_status_message == expected_msg
diff --git a/src/tests/nagios_plugin/setup.py b/src/tests/nagios_plugin/setup.py
index 90f14d5..83d86e9 100644
--- a/src/tests/nagios_plugin/setup.py
+++ b/src/tests/nagios_plugin/setup.py
@@ -1,7 +1,9 @@
-from setuptools import setup
-from tempfile import TemporaryDirectory
+"""Setup nagios_plugin3 module for unittesting."""
 import os
 import urllib.request
+from tempfile import TemporaryDirectory
+
+from setuptools import setup
 
 
 with TemporaryDirectory() as tempd:
diff --git a/src/tests/unit/conftest.py b/src/tests/unit/conftest.py
index e379711..f144c2b 100644
--- a/src/tests/unit/conftest.py
+++ b/src/tests/unit/conftest.py
@@ -1,5 +1,7 @@
-import unittest.mock as mock
+"""Initialize the pytest unittesting environment."""
+
 import sys
+import unittest.mock as mock
 from os.path import abspath, dirname, join
 
 import pytest
@@ -15,6 +17,7 @@ sys.path.append(CHECKS_DIR)
 # and import layer in lib_openstack_service_checks
 @pytest.fixture
 def mock_layers(monkeypatch):
+    """Mock charms.layer."""
     import sys
 
     sys.modules["charms.layer"] = mock.Mock()
@@ -34,6 +37,7 @@ def mock_layers(monkeypatch):
 
 @pytest.fixture
 def mock_hookenv_config(monkeypatch):
+    """Mock hookenv.config."""
     import yaml
 
     def mock_config():
@@ -53,6 +57,7 @@ def mock_hookenv_config(monkeypatch):
 
 @pytest.fixture
 def mock_remote_unit(monkeypatch):
+    """Mock remote_unit as unit-mock/0."""
     monkeypatch.setattr(
         "lib_openstack_service_checks.hookenv.remote_unit", lambda: "unit-mock/0"
     )
@@ -60,6 +65,7 @@ def mock_remote_unit(monkeypatch):
 
 @pytest.fixture
 def mock_charm_dir(monkeypatch):
+    """Mock hookenv.charm_dir."""
     monkeypatch.setattr(
         "lib_openstack_service_checks.hookenv.charm_dir", lambda: "/mock/charm/dir"
     )
@@ -67,6 +73,7 @@ def mock_charm_dir(monkeypatch):
 
 @pytest.fixture
 def mock_unitdata_keystonecreds(monkeypatch):
+    """Mock keystone credentials from unitdata.kv."""
     creds = {
         "keystonecreds": {
             "username": "nagios",
@@ -82,6 +89,7 @@ def mock_unitdata_keystonecreds(monkeypatch):
 
 @pytest.fixture
 def openstackservicechecks(tmpdir, mock_hookenv_config, mock_charm_dir, monkeypatch):
+    """Mock the OSCHelper library bits."""
     from lib_openstack_service_checks import OSCHelper
 
     helper = OSCHelper()
diff --git a/src/tests/unit/test_check_cinder_services.py b/src/tests/unit/test_check_cinder_services.py
index 3428dd6..588e58d 100644
--- a/src/tests/unit/test_check_cinder_services.py
+++ b/src/tests/unit/test_check_cinder_services.py
@@ -1,8 +1,11 @@
-import pytest
-import nagios_plugin3
+"""Test Cinder Service Checks."""
 
 import check_cinder_services
 
+import nagios_plugin3
+
+import pytest
+
 
 @pytest.mark.parametrize(
     "state,status,result",
@@ -14,6 +17,7 @@ import check_cinder_services
     ],
 )
 def test_check_status(state, status, result):
+    """Ensure status of cinder check is green."""
     cinder_service = {
         "binary": "cinder-scheduler",
         "disabled_reason": None,
@@ -58,15 +62,17 @@ def test_check_status(state, status, result):
     ],
 )
 def test_check_cinder_services(state1, is_skip_disabled, status2, status3, result):
+    """Test several cinder status inputs and expected nagios returns."""
+
     class _TestArgs(object):
         skip_disabled = is_skip_disabled
 
     class _TestCinder(object):
-        def get(cls, name):
+        def get(self, name):
             return _TestCinderJson()
 
     class _TestCinderJson(object):
-        def json(cls):
+        def json(self):
             return {
                 "services": [
                     {
@@ -131,12 +137,14 @@ def test_check_cinder_services(state1, is_skip_disabled, status2, status3, resul
 
 
 def test_check_cinder_services_unknown():
+    """Test that cinder checks return UNKNOWN when no services found."""
+
     class _TestCinder(object):
-        def get(cls, name):
+        def get(self, name):
             return _TestCinderJson()
 
     class _TestCinderJson(object):
-        def json(cls):
+        def json(self):
             return {"services": []}
 
     args, cinder = None, _TestCinder()
diff --git a/src/tests/unit/test_check_contrail_analytics_alarms.py b/src/tests/unit/test_check_contrail_analytics_alarms.py
index 663814a..a057c60 100644
--- a/src/tests/unit/test_check_contrail_analytics_alarms.py
+++ b/src/tests/unit/test_check_contrail_analytics_alarms.py
@@ -1,3 +1,5 @@
+"""Test contrail analytics nagios check script."""
+
 import json
 from os.path import abspath, dirname, join
 
@@ -7,6 +9,7 @@ TEST_DIR = dirname(abspath(__file__))
 
 
 def test_parse_contrail_alarms():
+    """Test defined failure input provides expected alerts."""
     with open(join(TEST_DIR, "contrail_alert_data.json")) as f:
         data = json.load(f)
     parsed = check_contrail_analytics_alarms.parse_contrail_alarms(data)
@@ -30,6 +33,7 @@ CRITICAL: vrouter{compute-7.maas, sev=1, ts[2020-07-03 18:30:32.481386]} Vrouter
 
 
 def test_parse_contrail_alarms_filter_vrouter_control_9():
+    """Test that alerts are ignorable with proper configs."""
     with open(join(TEST_DIR, "contrail_alert_data.json")) as f:
         data = json.load(f)
     ignored_re = r"(?:vrouter)|(?:control-9)"
@@ -48,6 +52,7 @@ CRITICAL: control-node{control-7-contrail-rmq, sev=1, ts[2020-06-25 18:29:24.377
 
 
 def test_parse_contrail_alarms_filter_critical():
+    """Test that we can ignore critical alerts by pattern."""
     with open(join(TEST_DIR, "contrail_alert_data.json")) as f:
         data = json.load(f)
     ignored_re = r"(?:CRITICAL)"
@@ -64,6 +69,7 @@ WARNING: control-node{control-8-contrail-rmq, sev=0, ts[2020-06-25 18:29:23.6848
 
 
 def test_parse_contrail_alarms_all_ignored():
+    """Test that we get okay response if ignoring all crit/warn."""
     with open(join(TEST_DIR, "contrail_alert_data.json")) as f:
         data = json.load(f)
     ignored_re = r"(?:CRITICAL)|(?:WARNING)"
@@ -79,6 +85,7 @@ OK: total_alarms[11], unacked_or_sev_gt_0[10], total_ignored[11], ignoring r'(?:
 
 
 def test_parse_contrail_alarms_no_alarms():
+    """Test that no alarms results in green check response."""
     ignored_re = r""
     parsed = check_contrail_analytics_alarms.parse_contrail_alarms(
         {}, ignored=ignored_re
diff --git a/src/tests/unit/test_check_nova_services.py b/src/tests/unit/test_check_nova_services.py
index 816ec73..5358376 100644
--- a/src/tests/unit/test_check_nova_services.py
+++ b/src/tests/unit/test_check_nova_services.py
@@ -1,8 +1,11 @@
-import pytest
-import nagios_plugin3
+"""Test nova-services-check nagios plugin."""
 
 import check_nova_services
 
+import nagios_plugin3
+
+import pytest
+
 
 @pytest.mark.parametrize(
     "is_skip_disabled,num_nodes",
@@ -14,6 +17,8 @@ import check_nova_services
     ],
 )
 def test_check_hosts_up(is_skip_disabled, num_nodes):
+    """Test aggregate host availability alerts."""
+
     class _TestArgs(object):
         warn = 2
         crit = 1
@@ -61,17 +66,19 @@ def test_check_hosts_up(is_skip_disabled, num_nodes):
 
 @pytest.mark.parametrize("is_skip_disabled", [True, False])
 def test_check_nova_services(is_skip_disabled, monkeypatch):
+    """Check expected results of nova service checks."""
+
     class _TestArgs(object):
         warn = 2
         crit = 1
         skip_disabled = is_skip_disabled
 
     class _TestNova(object):
-        def get(cls, name):
+        def get(self, name):
             return _TestNovaJson()
 
     class _TestNovaJson(object):
-        def json(cls):
+        def json(self):
             return {
                 "aggregates": [],
                 "services": [
diff --git a/src/tests/unit/test_check_octavia.py b/src/tests/unit/test_check_octavia.py
index 5f871e8..2001efb 100644
--- a/src/tests/unit/test_check_octavia.py
+++ b/src/tests/unit/test_check_octavia.py
@@ -1,15 +1,19 @@
-from datetime import datetime, timedelta
+"""Test octavia nagios check script."""
+
 import json
 import unittest.mock as mock
+from datetime import datetime, timedelta
 from uuid import uuid4
 
 import check_octavia
+
 import pytest
 
 
 @mock.patch("check_octavia.openstack.connect")
 @pytest.mark.parametrize("check", ["loadbalancers", "pools", "amphorae", "image"])
 def test_stable_alarms(connect, check):
+    """Test for expected green status."""
     args = mock.MagicMock()
     args.ignored = r""
     args.check = check
@@ -39,6 +43,7 @@ OK: total_alarms[0], total_crit[0], total_ignored[0], ignoring r''
 
 @mock.patch("check_octavia.openstack.connect")
 def test_no_images_is_ignorable(connect):
+    """Test that we can ignore when no images exist."""
     args = mock.MagicMock()
     args.ignored = "none exist"
     args.check = "image"
@@ -59,6 +64,7 @@ OK: total_alarms[1], total_crit[1], total_ignored[1], ignoring r'(?:none exist)'
 
 @mock.patch("check_octavia.openstack.connect")
 def test_no_images(connect):
+    """Test alerting status when no images exist."""
     args = mock.MagicMock()
     args.ignored = r""
     args.check = "image"
@@ -80,6 +86,7 @@ Octavia requires image with tag octavia to create amphora, but none exist
 
 @mock.patch("check_octavia.openstack.connect")
 def test_no_active_images(connect):
+    """Test alerting status when the octavia amphora image is not active."""
     args = mock.MagicMock()
     args.ignored = r""
     args.check = "image"
@@ -108,6 +115,7 @@ Octavia requires image with tag octavia to create amphora, but none are active: 
 
 @mock.patch("check_octavia.openstack.connect")
 def test_no_fresh_images(connect):
+    """Test alerting for stale octavia amphora images."""
     args = mock.MagicMock()
     args.ignored = r""
     args.check = "image"
diff --git a/src/tests/unit/test_lib.py b/src/tests/unit/test_lib.py
index 674285d..1fcfca6 100644
--- a/src/tests/unit/test_lib.py
+++ b/src/tests/unit/test_lib.py
@@ -1,17 +1,20 @@
+"""Test helper library functions."""
+
 from unittest.mock import MagicMock
 
-import pytest
 import keystoneauth1
 
 from lib_openstack_service_checks import (
-    OSCKeystoneServerError,
     OSCKeystoneClientError,
+    OSCKeystoneServerError,
     OSCSslError,
 )
 
+import pytest
+
 
 def test_openstackservicechecks_common_properties(openstackservicechecks):
-    """Verify the most common properties from the class or default config.yaml"""
+    """Verify the most common properties from the class or default config.yaml."""
     assert isinstance(openstackservicechecks.charm_config, dict)
     assert openstackservicechecks.check_dns == ""
     assert openstackservicechecks.contrail_analytics_vip == ""
@@ -93,6 +96,7 @@ def test_openstackservicechecks_get_keystone_credentials_oscredentials(
     ],
 )
 def test_get_rally_checks_context(skip_rally, result, openstackservicechecks):
+    """Check that rally config context configuration works as expected."""
     openstackservicechecks.charm_config["skip-rally"] = skip_rally
     expected = {
         comp: result[num]
@@ -114,6 +118,7 @@ def test_get_rally_checks_context(skip_rally, result, openstackservicechecks):
 def test_keystone_client_exceptions(
     keystone_auth_exception, expected_raised_exception, openstackservicechecks, source
 ):
+    """Test OSC exceptions."""
     mock_keystone_client = MagicMock()
     getattr(mock_keystone_client, source).list.side_effect = keystone_auth_exception
     openstackservicechecks._keystone_client = mock_keystone_client
diff --git a/src/tox.ini b/src/tox.ini
index e00666c..75b9a23 100644
--- a/src/tox.ini
+++ b/src/tox.ini
@@ -29,18 +29,15 @@ commands =
 deps =
     black
     flake8
-#TODO    flake8-docstrings
-#TODO    flake8-import-order
+    flake8-docstrings
+    flake8-import-order
     pep8-naming
     flake8-colors
 
 [flake8]
 ignore =
-    N805 #TODO first argument of a method should be named 'self'
-    E123 #TODO closing bracket does not match indentation of opening bracket's line
-    W504 #TODO line break after binary operator
-    W503 #TODO line break before binary operator
-    N806 #TODO variable 'RALLY_CONF' in function should be lowercase
+    D102 #TODO Missing docstring in public method
+    D103 #TODO Missing docstring in public function
 exclude =
     .git,
     __pycache__,
@@ -65,8 +62,6 @@ commands =
            --cov=reactive \
            --cov=actions \
            --cov=files \
-           --cov=hooks \
-           --cov=src \
            --cov-report=term \
            --cov-report=annotate:report/annotated \
            --cov-report=html:report/html