Merge ~xavpaice/charm-openstack-service-checks/+git/charm-openstack-service-checks:ssl-nagios-checks into ~canonical-bootstack/charm-openstack-service-checks:master

[Xav Paice]

Add NRPE checks for all API endpoints in the Keystone catalog

This change implements the first stage of spec https://code.launchpad.net/~xavpaice/bootstack-charm-specs/+git/bootstack-charm-specs/+ref/ssl-nagios-checks

The intent is to add an NRPE check to the openstack-services-checks unit, checking each endpoint found in the Keystone catalog using a healthcheck URL if known, otherwise the root URL found in the catalog. If the service is using TLS, then the expiry date of the certificate is also checked.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Alvaro Uria (aluria)]

Please find comments inline. Other than that, I tested it in the charmlab and it looks good to me.

review: Needs Fixing

[Xav Paice (xavpaice)]

Thanks for the review - couple of responses inline. Will update the actual change shortly.

[Joel Sing (jsing)]

Please add an actual commit message to this merge proposal - what is this change doing and why?

Readability/standards review comments inline (mostly consistency related).

[Andrea Ieri (aieri)]

A bunch of inline comments :)

review: Needs Fixing

[Xav Paice (xavpaice)]

Thanks for the detailed review(s). Fresh commit coming.

Couple of inline responses.

[Joel Sing (jsing)]

LGTM for a readability/standards review - couple of minor comments inline.

Please ensure you get a peer approval before merging.

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Xav Paice (xavpaice)]

Review nits have been addressed, would appreciate review.

[Joel Sing (jsing)]

LGTM for development standards - please see a handful of comments inline, mostly around doc strings and internal consistency.

review: Approve (+1)

[Joel Sing (jsing)]

FWIW it also seems that peer review/approval is still needed.

[Xav Paice (xavpaice)]

peer review would be good if someone can please.

Re the formatting nits, thanks for pointing them out. For these particular ones, I'll consider adding another commit - but I do want to avoid growing the scope any further than it already has grown due to catching formatting issues. We should do a separate PEP8 compliance commit for the rest of the code, plus another to add unit test coverage.

[Peter Sabaini (peter-sabaini)]

Comments inline -- one tox.ini issue and some nitpickings

A more general suggestion would be to add some logging to the charm and the service checks to aid in troubleshooting (charm currently has little logging and service checks none). Service checks could maybe just use a syslog handler

review: Needs Fixing

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Xav Paice (xavpaice)]

I've addressed the latest round of review comments, and squashed commits to one to keep the history a bit cleaner.

Thanks Joel for the +1 earlier, all I've done here is address the nits and squash commits.

Re logging, that's a great plan for the next commit.

[Joel Sing (jsing)]

> I've addressed the latest round of review comments, and squashed commits to
> one to keep the history a bit cleaner.
>
> Thanks Joel for the +1 earlier, all I've done here is address the nits and
> squash commits.

Looks like the merge proposal has been resubmitted, so I'll +1 again without further review on this basis. FWIW you can squash and force push without resubmitting, which will avoid this.

> Re logging, that's a great plan for the next commit.

[Joel Sing (jsing)]

Reapproving without further review.

review: Approve (+1)

[Peter Sabaini (peter-sabaini)]

lgtm, cheers

review: Approve

[Xav Paice (xavpaice)]

got +1's from two folks, marking approved. Thanks!

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 7895a97e4ee052c44971576a691e4ce23d6969f2

diff --git a/.gitignore b/.gitignore
 new file mode 100644
index 0000000..dccc0dc
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,10 @@
+*~
+/bin
+debian/files
+/pkg
+*.pyc
+__pycache__
+*.swp
+.tox
+.venv
+.idea
diff --git a/Makefile b/Makefile
index a1ad3a5..27eeff6 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
 #!/usr/bin/make
 
-all: lint unit_test
+all: lint test
 
 
 .PHONY: clean
@@ -10,15 +10,17 @@ clean:
 .PHONY: apt_prereqs
 apt_prereqs:
 	@# Need tox, but don't install the apt version unless we have to (don't want to conflict with pip)
-	@which tox >/dev/null || (sudo apt-get install -y python-pip && sudo pip install tox)
+	@which tox >/dev/null || (sudo apt-get install -y python3-pip && sudo pip3 install tox)
 
 .PHONY: lint
 lint: apt_prereqs
-	@tox --notest
-	@PATH=.tox/py34/bin:.tox/py35/bin flake8 $(wildcard hooks reactive lib unit_tests tests)
+	@tox -e pep8
 	@charm proof
 
-.PHONY: unit_test
+.PHONY: test
 unit_test: apt_prereqs
 	@echo Starting tests...
 	tox
+
+build:
+	charm build
diff --git a/README.md b/README.md
index 24a4b54..b3159af 100644
--- a/README.md
+++ b/README.md
@@ -2,7 +2,7 @@
 
 This charm provides OpenStack services checks for Nagios
 
- # Build
+# Build
 The fully built charm needs the following source branch
 * https://git.launchpad.net/~canonical-bootstack/bootstack-ops/+git/charm-openstack-services-checks
 
@@ -29,16 +29,20 @@ Build the charm, and symlink for juju-1 compatibility
 
     juju deploy local:xenial/openstack-services-checks
 
-This charm supports relating to keystone, but keystone-credentials interface
-seems to be flaky, and hard to remove-relation, so the charm works around this
-by adding 'os-credentials' setting (see setting description hints)
+This charm supports relating to keystone via the keystone-credentials
+interface.  If you do not wish to use this, you can supply your own credential
+set for Openstack by  adding 'os-credentials' setting (see setting description
+hints)
 
     juju set openstack-services-checks os-credentials=" ... "
-
     juju add-relation openstack-services-checks nagios
 
 With Keystone
 
     juju add-relation openstack-services-checks:identity-credentials keystone:identity-credentials
 
+If your OpenStack API endpoints have a common URL for the Admin, Public and
+Internal addresses, you should consider disabling some endpoints which would be
+duplicated otherwise, e.g.
 
+    juju config openstack-service-checks check_internal_urls=False check_admin_urls=False
diff --git a/config.yaml b/config.yaml
index e4e8783..b71208c 100644
--- a/config.yaml
+++ b/config.yaml
@@ -48,3 +48,28 @@ options:
     default: false
     description: |
       An option to specify whether you want Warning alerts in nagios for disabled nova-compute hosts.
+  tls_warn_days:
+    type: int
+    default: 30
+    description: |
+      Number of days left for the TLS certificate to expire before warning.
+  tls_crit_days:
+    type: int
+    default: 14
+    description: |
+      Number of days left for the TLS certificate to expire before alerting Critical.
+  check_public_urls:
+    type: boolean
+    default: True
+    description: |
+      If true, create NRPE checks matching all 'public' URLs in the Keystone catalog.
+  check_internal_urls:
+    type: boolean
+    default: True
+    description: |
+      If true, create NRPE checks matching all 'internal' URLs in the Keystone catalog.
+  check_admin_urls:
+    type: boolean
+    default: True
+    description: |
+      If true, create NRPE checks matching all 'admin' URLs in the Keystone catalog.
diff --git a/files/plugins/check_nova_services.py b/files/plugins/check_nova_services.py
index 1279c6a..6498542 100755
--- a/files/plugins/check_nova_services.py
+++ b/files/plugins/check_nova_services.py
@@ -43,11 +43,11 @@ def check_hosts_up(args, aggregate, hosts, services_compute):
         local_msg.append("Host Aggregate {} has {} hosts alive".format(
             aggregate, counts['ok']))
     nova_status = {
-            'agg_name': aggregate,
-            'msg_text': ", ".join(local_msg),
-            'critical': status_crit,
-            'warning': status_warn,
-            }
+        'agg_name': aggregate,
+        'msg_text': ", ".join(local_msg),
+        'critical': status_crit,
+        'warning': status_warn,
+    }
     return nova_status
 
 
@@ -103,8 +103,8 @@ if __name__ == '__main__':
     command = ['/bin/bash', '-c', "source {} && env".format(args.env)]
     proc = subprocess.Popen(command, stdout=subprocess.PIPE)
     for line in proc.stdout:
-        (key, _, value) = line.partition("=")
-        os.environ[key] = value.rstrip()
+        (key, _, value) = line.partition(b'=')
+        os.environ[key.decode('utf-8')] = value.rstrip().decode('utf-8')
     proc.communicate()
     nova = os_client_config.session_client('compute', cloud='envvars')
     nagios_plugin.try_check(check_nova_services, args, nova)
diff --git a/layer.yaml b/layer.yaml
index f364d90..72f0af5 100644
--- a/layer.yaml
+++ b/layer.yaml
@@ -1,6 +1,16 @@
 includes:
+  - 'layer:apt'
   - 'layer:basic'
-  - 'interface:nrpe-external-master'
   - 'interface:keystone-credentials'
+  - 'interface:nrpe-external-master'
 ignore: ['.*.swp' ]
 repo: 'lp:~canonical-bootstack/+git/charm-openstack-service-checks'
+options:
+  basic:
+    use_venv: true
+  apt:
+    packages:
+      - nagios-nrpe-server
+      - python3-keystoneclient
+      - python3-openstackclient
+      - python-openstackclient
diff --git a/reactive/service_checks.py b/reactive/service_checks.py
index 8341324..02a82d3 100644
--- a/reactive/service_checks.py
+++ b/reactive/service_checks.py
@@ -17,112 +17,117 @@ from charmhelpers.core import (
     unitdata,
 )
 
-from charmhelpers.fetch import (
-    apt_install,
-    apt_update,
-)
-
 from charmhelpers.contrib.charmsupport.nrpe import NRPE
+from urllib.parse import urlparse
 
 config = hookenv.config()
-install_packages = ['nagios-nrpe-server', 'python-openstackclient']
+NOVARC = '/var/lib/nagios/nagios.novarc'
+PLUGINS_DIR = '/usr/local/lib/nagios/plugins/'
 
 
 @when_not('os-service-checks.installed')
 def install_service_checks():
-    hookenv.status_set('maintenance', 'Installing software')
-    apt_update()
-    apt_install(install_packages)
+    # Apt package installs are handled by the apt layer
     set_state('os-service-checks.installed')
     remove_state('os-service-checks.configured')
     hookenv.status_set('active', 'Ready')
-# setup openstack user
 
 
 @when('identity-credentials.connected')
-def configure_keystone_username(keystone):
+def configure_ident_username(keystone):
     username = 'nagios'
     keystone.request_credentials(username)
 
 
 @when('identity-credentials.available')
 def save_creds(keystone):
-    creds = get_creds(keystone)
-    unitdata.kv().set('keystone-relation-creds', creds)
-    remove_state('os-service-checks.configured')
-
-
-def get_creds(keystone):
-
+    """
+    Collect and save credentials from Keystone relation.
+
+    Get credentials from the Keystone relation,
+    reformat them into something the Keystone client
+    can use, and save them into the unitdata.
+    """
+    creds = {
+        'username': keystone.credentials_username(),
+        'password': keystone.credentials_password(),
+        'region': keystone.region(),
+    }
     if keystone.api_version() == '3':
         api_url = "v3"
         try:
             domain = keystone.domain()
         except AttributeError:
             domain = 'service_domain'
-        creds = {
-            'credentials_username': keystone.credentials_username(),
-            'credentials_password': keystone.credentials_password(),
-            'credentials_project': keystone.credentials_project(),
+        # keystone relation sends info back with funny names, fix here
+        creds.update({
+            'project_name': keystone.credentials_project(),
             'auth_version': '3',
-            'region': keystone.region(),
-            'credentials_user_domain': domain,
-            'credentials_project_domain': domain
-            }
+            'user_domain_name': domain,
+            'project_domain_name': domain
+        })
     else:
         api_url = "v2.0"
-        creds = {
-            'credentials_username': keystone.credentials_username(),
-            'credentials_password': keystone.credentials_password(),
-            'credentials_project': keystone.credentials_project(),
-            'region': keystone.region(),
-            }
+        creds['tenant_name'] = keystone.credentials_project()
 
     auth_url = "%s://%s:%s/%s" % (keystone.auth_protocol(),
                                   keystone.auth_host(), keystone.auth_port(),
                                   api_url)
     creds['auth_url'] = auth_url
-    return creds
+    unitdata.kv().set('keystonecreds', creds)
+    remove_state('os-service-checks.configured')
 
 
-# allow user to override credentials (and the need to be related to keystone)
+# allow user to override credentials (and the need to be related to Keystone)
 # with 'os-credentials'
 def get_credentials():
-    keystone_creds = config_flags_parser(config.get('os-credentials'))
-    if keystone_creds:
-        if '/v3' in keystone_creds['auth_url']:
-            creds = {
-                'credentials_username': keystone_creds['username'],
-                'credentials_password': keystone_creds['password'],
-                'credentials_project': keystone_creds['credentials_project'],
-                'region': keystone_creds['region_name'],
-                'auth_url': keystone_creds['auth_url'],
+    """
+    Get credential info from either config or relation data.
+
+    If config 'os-credentials' is set, return that info otherwise look for for a keystonecreds relation data.
+
+    :return: dictionary of credential information for Keystone.
+    """
+    ident_creds = config_flags_parser(config.get('os-credentials'))
+    if ident_creds:
+        creds = {
+            'username': ident_creds['username'],
+            'password': ident_creds['password'],
+            'region': ident_creds['region_name'],
+            'auth_url': ident_creds['auth_url'],
+        }
+        if '/v3' in ident_creds['auth_url']:
+            creds.update({
+                'project_name': ident_creds['credentials_project'],
                 'auth_version': '3',
-                'credentials_user_domain': keystone_creds['domain'],
-                'credentials_project_domain': keystone_creds['domain'],
-            }
+                'user_domain_name': ident_creds['domain'],
+                'project_domain_name': ident_creds['domain'],
+            })
         else:
-            creds = {
-                'credentials_username': keystone_creds['username'],
-                'credentials_password': keystone_creds['password'],
-                'credentials_project': keystone_creds['credentials_project'],
-                'region': keystone_creds['region_name'],
-                'auth_url': keystone_creds['auth_url'],
-            }
+            creds['tenant_name'] = ident_creds['credentials_project']
     else:
         kv = unitdata.kv()
-        creds = kv.get('keystone-relation-creds')
+        creds = kv.get('keystonecreds')
+        old_creds = kv.get('keystone-relation-creds')
+        if old_creds and not creds:
+            # This set of creds needs an update to a newer format
+            creds['username'] = old_creds.pop('credentials_username')
+            creds['password'] = old_creds.pop('credentials_password')
+            creds['project_name'] = old_creds.pop('credentials_project')
+            creds['tenant_name'] = old_creds['project_name']
+            creds['user_domain_name'] = old_creds.pop('credentials_user_domain', None)
+            creds['project_domain_name'] = old_creds.pop('credentials_project_domain', None)
+            kv.set('keystonecreds', creds)
+            kv.update(creds, 'keystonecreds')
     return creds
 
 
 def render_checks():
     nrpe = NRPE()
-    plugins_dir = '/usr/local/lib/nagios/plugins/'
-    if not os.path.exists(plugins_dir):
-        os.makedirs(plugins_dir)
+    if not os.path.exists(PLUGINS_DIR):
+        os.makedirs(PLUGINS_DIR)
     charm_file_dir = os.path.join(hookenv.charm_dir(), 'files')
     charm_plugin_dir = os.path.join(charm_file_dir, 'plugins')
-
     host.rsync(
         charm_plugin_dir,
         '/usr/local/lib/nagios/',
@@ -133,9 +138,8 @@ def render_checks():
     crit = config.get("nova_crit")
     skip_disabled = config.get("skip-disabled")
     check_dns = config.get("check-dns")
-
-    check_command = plugins_dir + 'check_nova_services.py --warn ' \
-                                + str(warn) + ' --crit ' + str(crit)
+    nova_check_command = os.path.join(PLUGINS_DIR, 'check_nova_services.py')
+    check_command = '{} --warn {} --crit {}'.format(nova_check_command, warn, crit)
 
     if skip_disabled:
         check_command = check_command + ' --skip-disabled'
@@ -146,15 +150,18 @@ def render_checks():
 
     nrpe.add_check(shortname='neutron_agents',
                    description='Check that enabled Neutron agents are up',
-                   check_cmd=plugins_dir + 'check_neutron_agents.sh')
+                   check_cmd=os.path.join(PLUGINS_DIR, 'check_neutron_agents.sh'))
 
     if len(check_dns):
         nrpe.add_check(shortname='dns_multi',
                        description='Check DNS names are resolvable',
-                       check_cmd=plugins_dir + 'check_dns_multi.sh ' + ' '.join(check_dns.split()))
+                       check_cmd=PLUGINS_DIR + 'check_dns_multi.sh ' + ' '.join(check_dns.split()))
     else:
         nrpe.remove_check(shortname='dns_multi')
 
+    endpoint_checks = create_endpoint_checks()
+    for check in endpoint_checks:
+        nrpe.add_check(**check)
     nrpe.write()
 
 
@@ -171,8 +178,8 @@ def render_config():
         hookenv.log('render_config: No credentials yet, skipping')
         return
     hookenv.log('render_config: Got credentials for username={}'.format(
-        creds['credentials_username']))
-    render('nagios.novarc', '/var/lib/nagios/nagios.novarc', creds,
+        creds['username']))
+    render('nagios.novarc', NOVARC, creds,
            owner='nagios', group='nagios')
     render_checks()
     if config.get('trusted_ssl_ca', None):
@@ -198,3 +205,104 @@ def fix_ssl():
     with open(cert_file, 'w') as f:
         print(cert_content, file=f)
     subprocess.call(["/usr/sbin/update-ca-certificates"])
+
+
+def create_endpoint_checks():
+    """
+    Create an NRPE check for each Keystone catalog endpoint.
+
+    Read the Keystone catalog, and create a check for each endpoint listed.
+    If there is a healthcheck endpoint for the API, use that URL, otherwise check
+    the url '/'.
+    If SSL, add a check for the cert.
+    """
+    # provide URLs that can be used for healthcheck for some services
+    # This also provides a nasty hack-ish way to add switches if we need
+    # for some services.
+    health_check_params = {
+        'keystone': '/healthcheck',
+        's3': '/healthcheck',
+        'swift': '/healthcheck',
+        'aodh': '/ -e Unauthorized -d x-openstack-request-id',
+        'cinderv3': '/v3 -e Unauthorized -d x-openstack-request-id',
+        'cinderv2': '/v2 -e Unauthorized -d x-openstack-request-id',
+        'cinderv1': '/v1 -e Unauthorized -d x-openstack-request-id',
+        'glance': '/healthcheck',
+        'nova': '/healthcheck',
+    }
+
+    creds = get_credentials()
+    keystone_client = get_keystone_client(creds)
+    endpoints = keystone_client.endpoints.list()
+    services = [x for x in keystone_client.services.list() if x.enabled]
+    nrpe_checks = []
+    for endpoint in endpoints:
+        endpoint.service_names = [x.name for x in services if x.id == endpoint.service_id]
+        service_name = endpoint.service_names[0]
+        endpoint.healthcheck_url = health_check_params.get(service_name, '/')
+        if config.get('check_{}_urls'.format(endpoint.interface)):
+            cmd_params = ['/usr/lib/nagios/plugins/check_http ']
+            check_url = urlparse(endpoint.url)
+            host_port = check_url.netloc.split(':')
+            cmd_params.append('-H {} -p {}'.format(host_port[0], host_port[1]))
+            cmd_params.append('-u {}'.format(endpoint.healthcheck_url))
+            # if this is https, we want to add a check for cert expiry
+            # also need to tell check_http use use TLS
+            if check_url.scheme == 'https':
+                cmd_params.append('-S')
+                # Add an extra check for TLS cert expiry
+                cert_check = ['-C {},{}'.format(
+                    config.get('tls_warn_days'),
+                    config.get('tls_crit_days'))]
+                nrpe_checks.append({
+                    'shortname': "{}_{}_cert".format(
+                        service_name,
+                        endpoint.interface),
+                    'description': 'Certificate expiry check for {} {}'.format(
+                        service_name,
+                        endpoint.interface),
+                    'check_cmd': ' '.join(cmd_params + cert_check)
+                })
+            # Add the actual health check for the URL
+            nrpe_checks.append({
+                'shortname': "{}_{}".format(
+                    service_name,
+                    endpoint.interface),
+                'description': 'Endpoint url check for {} {}'.format(
+                    service_name,
+                    endpoint.interface),
+                'check_cmd': (' '.join(cmd_params))})
+    return nrpe_checks
+
+
+def get_keystone_client(creds):
+    """
+    Import the appropriate Keystone client depending on API version.
+
+    Use credential info to determine the Keystone API version, and make a client session object that is to be
+    used for authenticated communication with Keystone.
+
+    :returns: a keystoneclient Client object
+    """
+    from keystoneclient import session
+    if int(creds['auth_version']) >= 3:
+        from keystoneclient.v3 import client
+        from keystoneclient.auth.identity import v3
+        auth_fields = ['auth_url', 'password', 'username', 'user_domain_name',
+                       'project_domain_name', 'project_name']
+        auth_creds = {}
+        for key in auth_fields:
+            auth_creds[key] = creds[key]
+        auth = v3.Password(**auth_creds)
+
+    else:
+        from keystoneclient.v2_0 import client
+        from keystoneclient.auth.identity import v2
+        auth_fields = ['auth_url', 'password', 'username', 'tenant_name']
+        auth_creds = {}
+        for key in auth_fields:
+            auth_creds[key] = creds[key]
+        auth = v2.Password(**auth_creds)
+
+    sess = session.Session(auth=auth)
+    return client.Client(session=sess)
diff --git a/templates/nagios.novarc b/templates/nagios.novarc
index e4c7e68..27f1c2a 100644
--- a/templates/nagios.novarc
+++ b/templates/nagios.novarc
@@ -1,7 +1,7 @@
-export OS_USERNAME={{ credentials_username }}
-export OS_TENANT_NAME={{ credentials_project }}
-export OS_PROJECT_NAME={{ credentials_project }}
-export OS_PASSWORD={{ credentials_password }}
+export OS_USERNAME={{ username }}
+export OS_TENANT_NAME={{ tenant_name }}
+export OS_PROJECT_NAME={{ project_name }}
+export OS_PASSWORD={{ password }}
 export OS_REGION_NAME={{ region }}
 export OS_AUTH_URL={{ auth_url }}
 {%- if cacert %}
@@ -13,6 +13,6 @@ export HOME=${SNAP_COMMON}
 {%- if auth_version %}
 export OS_IDENTITY_API_VERSION={{ auth_version }}
 export OS_AUTH_VERSION={{ auth_version }}
-export OS_USER_DOMAIN_NAME={{ credentials_user_domain }}
-export OS_PROJECT_DOMAIN_NAME={{ credentials_project_domain }}
+export OS_USER_DOMAIN_NAME={{ user_domain_name }}
+export OS_PROJECT_DOMAIN_NAME={{ project_domain_name }}
 {%- endif %}
diff --git a/test-requirements.txt b/test-requirements.txt
 new file mode 100644
index 0000000..77ec3f5
--- /dev/null
+++ b/test-requirements.txt
@@ -0,0 +1,9 @@
+# Lint and unit test requirements
+flake8>=2.2.4,<=2.4.1
+os-testr>=0.4.1
+requests>=2.18.4
+charms.reactive
+mock>=1.2
+nose>=1.3.7
+coverage>=3.6
+netifaces
diff --git a/tox.ini b/tox.ini
index 0b8b27a..f4560b5 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,12 +1,40 @@
 [tox]
 skipsdist=True
-envlist = py34, py35
+envlist = pep8
 skip_missing_interpreters = True
 
 [testenv]
-commands = py.test -v
+setenv = VIRTUAL_ENV={envdir}
+         PYTHONHASHSEED=0
+         TERM=linux
+         LAYER_PATH={toxinidir}/layers
+         INTERFACE_PATH={toxinidir}/interfaces
+         JUJU_REPOSITORY={toxinidir}/build
+passenv = http_proxy https_proxy
+install_command =
+  pip install {opts} {packages}
 deps =
     -r{toxinidir}/requirements.txt
 
+[testenv:build]
+basepython = python3
+commands =
+    charm-build --log-level DEBUG -o {toxinidir}/build src {posargs}
+
+[testenv:py3]
+basepython = python3
+deps = -r{toxinidir}/test-requirements.txt
+commands = ostestr {posargs}
+
+[testenv:pep8]
+basepython = python3
+deps = -r{toxinidir}/test-requirements.txt
+commands = flake8 {posargs} --max-complexity=10 --max-line-length=120 reactive files unit_tests
+
+[testenv:venv]
+basepython = python3
+commands = {posargs}
+
 [flake8]
-exclude=docs
+# E402 ignore necessary for path append before sys module import in actions
+ignore = E402