Merge ~afreiberger/charm-hw-health:makefile-20.08 into charm-hw-health:master

[Drew Freiberger]

[Xav Paice (xavpaice)]

LGTM, intermittent xenial functest fail but I think that's unrelated to this change.

review: Approve

diff --git a/.gitignore b/.gitignore
index 06bf3e8..aa54058 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,25 +1,44 @@
+# Juju files
+.unit-state.db
+.go-cookies
+
+layers/*
+interfaces/*
+
 # Byte-compiled / optimized / DLL files
 __pycache__/
 *.py[cod]
 *$py.class
 
+# Tests files and dir
+.pytest_cache/
+.coverage
+.tox
+report/
+htmlcov/
+
 # Log files
 *.log
 
-.tox/
-.coverage
+# pycharm
+.idea/
 
 # vi
 .*.swp
 
-# pycharm
-.idea/
-
 # version data
-src/repo-info
+repo-info
+version
+
+# Python builds
+deb_dist/
+dist/
+
+# Snaps
+*.snap
 
-# build artifacts
-builds
+# Builds
+.build/
 
-# tools bundle (needed for functional testing)
+# Functest artifacts
 tools.zip
diff --git a/Makefile b/Makefile
index d314eb8..279ba3d 100644
--- a/Makefile
+++ b/Makefile
@@ -1,78 +1,103 @@
-ifndef JUJU_REPOSITORY
-JUJU_REPOSITORY := $(CURDIR)
-export JUJU_REPOSITORY
-$(warning Warning JUJU_REPOSITORY was not set, defaulting to $(JUJU_REPOSITORY))
+PYTHON := /usr/bin/python3
+
+PROJECTPATH=$(dir $(realpath $(MAKEFILE_LIST)))
+ifndef CHARM_BUILD_DIR
+	CHARM_BUILD_DIR=${PROJECTPATH}.build
+endif
+ifndef CHARM_LAYERS_DIR
+	CHARM_LAYERS_DIR=${PROJECTPATH}/layers
 endif
+ifndef CHARM_INTERFACES_DIR
+	CHARM_INTERFACES_DIR=${PROJECTPATH}/interfaces
+endif
+METADATA_FILE="src/metadata.yaml"
+CHARM_NAME=$(shell cat ${PROJECTPATH}/${METADATA_FILE} | grep -E "^name:" | awk '{print $$2}')
 
 help:
 	@echo "This project supports the following targets"
 	@echo ""
 	@echo " make help - show this text"
+	@echo " make clean - remove unneeded files"
 	@echo " make submodules - make sure that the submodules are up-to-date"
-	@echo " make lint - use pre-commit to ensure consistent layout"
-	@echo " make test - run the functional test, unittests and lint"
-	@echo " make unittest - run the tests defined in the unittest subdirectory"
+	@echo " make submodules-update - update submodules to latest changes on remote branch"
+	@echo " make build - build the charm"
+	@echo " make release - run clean, submodules, and build targets"
+	@echo " make lint - run flake8 and black --check"
+	@echo " make black - run black and reformat files"
+	@echo " make proof - run charm proof"
+	@echo " make unittests - run the tests defined in the unittest subdirectory"
 	@echo " make functional - run the tests defined in the functional subdirectory"
-	@echo " make release - build the charm"
-	@echo " make clean - remove unneeded files"
+	@echo " make test - run lint, proof, unittests and functional targets"
 	@echo ""
 
+clean:
+	@echo "Cleaning files"
+	@git clean -ffXd -e '!.idea' -e '!tools.zip'
+	@echo "Cleaning existing build"
+	@rm -rf ${CHARM_BUILD_DIR}/${CHARM_NAME}
+
 submodules:
 	@echo "Cloning submodules"
 	@git submodule update --init --recursive
 
+submodules-update:
+	@echo "Pulling latest updates for submodules"
+	@git submodule update --init --recursive --remote --merge
+
+build: clean
+	@echo "Building charm to directory ${CHARM_BUILD_DIR}/${CHARM_NAME}"
+	@-git rev-parse --abbrev-ref HEAD > ./src/repo-info
+	@CHARM_LAYERS_DIR=${CHARM_LAYERS_DIR} CHARM_INTERFACES_DIR=${CHARM_INTERFACES_DIR} \
+		TERM=linux CHARM_BUILD_DIR=${CHARM_BUILD_DIR} charm build src/
+
+release: clean build
+	@echo "Charm is built at ${CHARM_BUILD_DIR}/${CHARM_NAME}"
+
 lint:
-	@echo "Running flake8"
-	cd src && tox -e pep8
+	@echo "Running lint checks"
+	@cd src && tox -e lint
+
+black:
+	@echo "Reformat files with black"
+	@cd src && tox -e black
 
-test: lint unittest functional
+proof: build
+	@echo "Running charm proof"
+	@charm proof ${CHARM_BUILD_DIR}/${CHARM_NAME}
 
-unittest: build
-	@cd $(JUJU_REPOSITORY)/builds/hw-health && tox -e unit
+unittests: build
+	@echo "Running unit tests"
+	@cd ${CHARM_BUILD_DIR}/${CHARM_NAME} && tox -e unit
 
 functional: resource_check build buildresources
-	@cd $(JUJU_REPOSITORY)/builds/hw-health && tox -e functional
+	@echo "Executing functional tests in ${CHARM_BUILD_DIR}"
+	@cd ${CHARM_BUILD_DIR}/${CHARM_NAME} && tox -e func
+
+test: lint proof unittests functional
+	@echo "Tests completed for charm ${CHARM_NAME}."
 
 resource_check:
-ifeq ("$(wildcard $(JUJU_REPOSITORY)/tools.zip)","")
-	$(error ERROR: You must save the tools resource in $(JUJU_REPOSITORY)/tools.zip \
+ifeq (${PROJECTPATH}tools.zip,)
+	$(error ERROR: You must save the tools resource in ${PROJECTPATH}tools.zip \
 		before running functional tests. \
 		Check the README for instructions on how to do so.)
 endif
 
 buildresources:
 	@echo "Copying tools.zip to the builds directory"
-	@cp $(JUJU_REPOSITORY)/tools.zip $(JUJU_REPOSITORY)/builds/tools.zip
+	@cp ${PROJECTPATH}/tools.zip ${CHARM_BUILD_DIR}/tools.zip
 	@echo "Building tools-missing.zip"
-	@zip $(JUJU_REPOSITORY)/tools.zip --copy \
-		--out $(JUJU_REPOSITORY)/builds/tools-missing.zip \
+	@zip ${PROJECTPATH}/tools.zip --copy \
+		--out ${CHARM_BUILD_DIR}/tools-missing.zip \
 		--exclude megacli
 	@echo "Building tools-checksum.zip"
-	@zip $(JUJU_REPOSITORY)/tools.zip --copy \
-		--out $(JUJU_REPOSITORY)/builds/tools-checksum.zip \
+	@zip ${PROJECTPATH}/tools.zip --copy \
+		--out ${CHARM_BUILD_DIR}/tools-checksum.zip \
 		--exclude megacli
-	@touch $(JUJU_REPOSITORY)/builds/megacli
-	@zip --move --junk-path $(JUJU_REPOSITORY)/builds/tools-checksum.zip \
-		$(JUJU_REPOSITORY)/builds/megacli
-
-build:
-	@echo "Building charm to base directory $(JUJU_REPOSITORY)"
-	@git show -s --format="commit-sha-1: %H%ncommit-short: %h" > ./src/repo-info
-	@echo -n "info-generated: " >> ./src/repo-info
-	@date --utc >> ./src/repo-info
-	@echo "note: This file should exist only in a built or released charm artifact (not in the charm source code tree)." >> ./src/repo-info
-	@CHARM_LAYERS_DIR=./layers CHARM_INTERFACES_DIR=./interfaces\
-		JUJU_REPOSITORY=$(JUJU_REPOSITORY) charm build ./src --force
-
-release: lint clean build
-	# Maybe add the command to push the build bundle to the store?
-	#
-	charm push ./builds/hw-health cs:~nagios-charmers/hw-health
+	@touch ${CHARM_BUILD_DIR}/megacli
+	@zip --move --junk-path ${CHARM_BUILD_DIR}/tools-checksum.zip \
+		${CHARM_BUILD_DIR}/megacli
 
-clean:
-	@echo "Cleaning files"
-	@for dir in src/.tox src/.pytest_cache builds deps; do \
-		rm -rf $$dir ; done
 
 # The targets below don't depend on a file
-.PHONY: lint test unittest build release clean help submodules
+.PHONY: help submodules submodules-update clean build release lint black proof unittests functional test buildresources resource_check
diff --git a/requirements.txt b/requirements.txt
deleted file mode 100644
index 4235f99..0000000
--- a/requirements.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-charm-tools
-charmhelpers
-charms.reactive
-pre-commit
-tox
diff --git a/src/files/mdadm/cron_mdadm.py b/src/files/mdadm/cron_mdadm.py
index ddf5d5e..3e7e8e9 100755
--- a/src/files/mdadm/cron_mdadm.py
+++ b/src/files/mdadm/cron_mdadm.py
@@ -133,7 +133,7 @@ def get_devices_stats(devices):
     return devices_stats
 
 
-def parse_output():
+def parse_output():  # noqa:C901
     devices = get_devices()
     if len(devices) == 0:
         return generate_output('WARNING: unexpectedly checked no devices')
diff --git a/src/files/megacli/check_megacli.py b/src/files/megacli/check_megacli.py
index ed1e8d3..b587d10 100755
--- a/src/files/megacli/check_megacli.py
+++ b/src/files/megacli/check_megacli.py
@@ -55,7 +55,7 @@ def handle_results(
         print(msg)
 
 
-def parse_output(policy=False):
+def parse_output(policy=False):  # noqa:C901
     noadapter_re = r'^Adapter \d+: No Virtual Drive Configured'
     adapter_re = r'^Adapter (\d+) -- Virtual Drive Information:'
     ldrive_re = r'^Virtual Drive\s*:\s+(\d+)'
diff --git a/src/tests/functional/test_hwhealth.py b/src/tests/functional/test_hwhealth.py
index 6a7966e..cae8fd4 100644
--- a/src/tests/functional/test_hwhealth.py
+++ b/src/tests/functional/test_hwhealth.py
@@ -3,6 +3,7 @@ import pytest
 import sys
 import subprocess
 import asyncio
+from os.path import abspath, dirname
 
 sys.path.append('lib')
 
@@ -17,7 +18,8 @@ SERIES = [
     'bionic',
     'xenial',
 ]
-JUJU_REPOSITORY = os.getenv('JUJU_REPOSITORY', '.')
+CHARM_DIR = dirname(dirname(dirname(abspath(__file__))))
+CHARM_BUILD_DIR = dirname(CHARM_DIR)
 NRPECFG_DIR = '/etc/nagios/nrpe.d'
 DEF_TIMEOUT = 600
 # These go along with the hpe repos for the hp* tools
@@ -30,13 +32,13 @@ DEF_TIMEOUT = 600
 async def deploy_hwhealth_res(model, app_name, res_filename):
     # Attaching resources is not implemented yet in libjuju
     # see https://github.com/juju/python-libjuju/issues/294
-    tools_res_path = os.path.join(JUJU_REPOSITORY, 'builds', res_filename)
+    tools_res_path = os.path.join(CHARM_BUILD_DIR, res_filename)
     subprocess.check_call([
         'juju',
         'deploy',
         '-m',
         model.info.name,
-        os.path.join(JUJU_REPOSITORY, 'builds', 'hw-health'),
+        os.path.join(CHARM_BUILD_DIR, 'hw-health'),
         app_name,
         '--resource',
         'tools={}'.format(tools_res_path),
@@ -44,7 +46,7 @@ async def deploy_hwhealth_res(model, app_name, res_filename):
 
 
 async def update_hwhealth_res(model, app_name, res_filename):
-    tools_res_path = os.path.join(JUJU_REPOSITORY, 'builds', res_filename)
+    tools_res_path = os.path.join(CHARM_BUILD_DIR, res_filename)
     subprocess.check_call([
         'juju',
         'attach-resource',
@@ -165,7 +167,7 @@ async def toolset(monkeypatch):
     # deployed charm
     with monkeypatch.context() as m:
         m.setattr('charmhelpers.core.hookenv.charm_dir',
-                  lambda: JUJU_REPOSITORY)
+                  lambda: CHARM_BUILD_DIR)
         m.setattr('charmhelpers.core.hookenv.config',
                   lambda x=None: dict())
         m.setattr('charmhelpers.contrib.charmsupport.nrpe.get_nagios_hostname',
diff --git a/src/tox.ini b/src/tox.ini
index 803d942..db563e9 100644
--- a/src/tox.ini
+++ b/src/tox.ini
@@ -1,30 +1,72 @@
 [tox]
-envlist = unit, functional, pep8
-skipsdist = true
+skipsdist=True
+skip_missing_interpreters = True
+envlist = lint, unit, func
 
-[testenv:unit]
-basepython=python3
-setenv = PYTHONPATH={toxinidir}/lib:{envdir}/lib/python3.8
+[testenv]
+basepython = python3
+setenv =
+  PYTHONPATH = {toxinidir}:{toxinidir}/lib/:{toxinidir}/hooks/
+passenv =
+  HOME
+  PATH
+  CHARM_BUILD_DIR
+  PYTEST_KEEP_MODEL
+  PYTEST_CLOUD_NAME
+  PYTEST_CLOUD_REGION
+  PYTEST_MODEL
+  MODEL_SETTINGS
+  HTTP_PROXY
+  HTTPS_PROXY
+  NO_PROXY
+  SNAP_HTTP_PROXY
+  SNAP_HTTPS_PROXY
+
+[testenv:lint]
 commands =
-  {toxworkdir}/../tests/download_nagios_plugin3.py
-  pytest -v --ignore {toxinidir}/tests/functional --cov=lib/hwhealth --cov=reactive/hw_health.py --cov=actions --cov=files --cov-report=term
+    flake8
+#TODO    black --check --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod)/" .
 deps =
-  -r{toxinidir}/tests/unit/requirements.txt
-  -r{toxinidir}/requirements.txt
+    black
+    flake8
+#TODO    flake8-docstrings
+#TODO    flake8-import-order
+#TODO    pep8-naming
+    flake8-colors
 
+[flake8]
+exclude =
+    .git,
+    __pycache__,
+    .tox,
+    charmhelpers,
+    mod,
+    .build
 
-[testenv:functional]
-passenv =
-  HOME
-  JUJU_REPOSITORY
-  PATH
-  test_preserve_model
-commands = pytest -v --ignore {toxinidir}/tests/unit
-deps = -r{toxinidir}/tests/functional/requirements.txt
-       -r{toxinidir}/requirements.txt
+#TODO max-line-length = 88
+max-line-length = 120
+max-complexity = 10
 
-[testenv:pep8]
-basepython = python3
+[testenv:black]
+commands =
+    black --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod)/" .
 deps =
-  flake8
-commands = flake8 {posargs} --max-complexity=12 --max-line-length=120 .
+    black
+
+[testenv:unit]
+commands =
+  {toxworkdir}/../tests/download_nagios_plugin3.py
+  pytest -v --ignore {toxinidir}/tests/functional \
+           --cov=lib \
+           --cov=reactive \
+           --cov=actions \
+           --cov=hooks \
+           --cov=src \
+           --cov-report=term \
+           --cov-report=annotate:report/annotated \
+           --cov-report=html:report/html
+deps = -r{toxinidir}/tests/unit/requirements.txt
+
+[testenv:func]
+commands = pytest -v --ignore {toxinidir}/tests/unit
+deps = -r{toxinidir}/tests/functional/requirements.txt