Merge ~canonical-is-bootstack/charm-telegraf/+git/telegraf-charm:bug/1877284-func into charm-telegraf:master

[Alvaro Uria]

[Alvaro Uria (aluria)]

This is the first part of the bug. Functional (integration) tests have been rewritten, using Zaza.

Second part will be to mock unit tests that otherwise would require privileges.

[Zachary Zehring (zzehring)]

Inline comments.

[Alvaro Uria (aluria)]

Thank you. I've replied inline and will come with a code update shortly.

[Alvaro Uria (aluria)]

As we talked, I've also added a try-except for requests.get. If HTTP return code is != 200, test will fail. However, network issues may be unrelated to telegraf, hence we loop through DEFAULT_RETRIES (12 times).

[Zachary Zehring (zzehring)]

LGTM. +1

review: Approve

[Paul Goins (vultaire)]

Overall, this looks like it's headed in the right direction. I've left some comments on some things I'd like to bring attention to.

review: Needs Fixing

[Paul Goins (vultaire)]

One more comment related to requested fixes.

[Alvaro Uria (aluria)]

Thank you for the review, Paul. I've pushed a new update that should fix all the suggestions.

[Paul Goins (vultaire)]

One last thing.

review: Needs Fixing

[Paul Goins (vultaire)]

Per discussion w/ aluria, this is totally fine. Approved.

review: Approve

diff --git a/Makefile b/Makefile
index 39fba04..61df584 100644
--- a/Makefile
+++ b/Makefile
@@ -13,31 +13,57 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+JUJU_REPOSITORY:=$(if $(JUJU_REPOSITORY),$(JUJU_REPOSITORY),/tmp/charm-builds)
+CHARM_BUILD_DIR:=$(if $(CHARM_BUILD_DIR),$(CHARM_BUILD_DIR),$(JUJU_REPOSITORY)/builds)
+BUILTCHARMDIR:=$(CHARM_BUILD_DIR)/telegraf
+
+.PHONY: help
+help:
+	@echo "This project supports the following targets"
+	@echo ""
+	@echo " make help - show this text"
+	@echo " make lint - run flake8"
+	@echo " make test - run the lint, unittests and functional"
+	@echo " make unittest - run the tests defined in the unit_tests directory"
+	@echo " make functional - run the tests defined in the tests directory"
+	@echo " make release - build the charm"
+	@echo " make clean - remove unneeded files"
+	@echo ""
+
 
 .PHONY: lint
 lint:
-	tox -e lint
+	@echo "Checking flake8"
+	@tox -e lint
 
-.PHONY: test
-test:
-	tox -e test
+.PHONY: unittest
+unittest:
+	@echo "Running unit tests..."
+	@tox -e unit
 
-.PHONY: integration_xenial
-integration_xenial:
-	tox -e integration_xenial
+.PHONY: build
+build:
+	CHARM_BUILD_DIR="$(CHARM_BUILD_DIR)" charm build --report --force
 
-.PHONY: integration_trusty
-integration_trusty:
-	tox -e integration_trusty
+.PHONY: functional
+functional: build
+	@echo "Running functional tests..."
+	@CHARM_BUILD_DIR="$(CHARM_BUILD_DIR)" tox -e func
 
-.PHONY: build
-build: realclean
-	@charm build
+.PHONY: test
+test: lint unittest functional
 
 .PHONY: clean
-clean: 
+clean:
 	git clean -fXd
 
 .PHONY: realclean
 realclean:
 	git clean -fxd
+
+.PHONY: release
+release:
+	@echo ---------------------------------------------------------------------
+	@echo charm push $(BUILTCHARMDIR)
+	@echo charm release \<revision\>
+	@echo ---------------------------------------------------------------------
diff --git a/reactive/telegraf.py b/reactive/telegraf.py
index f4ff604..c3dfa8f 100644
--- a/reactive/telegraf.py
+++ b/reactive/telegraf.py
@@ -157,7 +157,7 @@ def render_base_inputs():
 def get_extra_options():
     extra_options = {'inputs': {}, 'outputs': {}}
     extra_options_raw = hookenv.config()['extra_options']
-    extra_opts = yaml.load(extra_options_raw) or {}
+    extra_opts = yaml.safe_load(extra_options_raw) or {}
     extra_options.update(extra_opts)
     # jsonify value, required as the telegraf config values format is similar
     # to raw json
@@ -260,7 +260,7 @@ def render_socket_listener_config(context):
 
 @when('apt.installed.telegraf')
 @when_not('telegraf.configured')
-def configure_telegraf():
+def configure_telegraf():  # noqa: C901 (compexity 15)
     config_path = get_main_config_path()
     if get_remote_unit_name() is None:
         hookenv.status_set('waiting', 'Waiting for juju-info relation')
diff --git a/requirements.txt b/requirements.txt
new file mode 100644
index 0000000..32c193e
--- /dev/null
+++ b/requirements.txt
@@ -0,0 +1,2 @@
+charms.reactive>=1.3.0
+requests
diff --git a/test_requirements.txt b/test_requirements.txt
deleted file mode 100644
index 64be687..0000000
--- a/test_requirements.txt
+++ /dev/null
@@ -1,14 +0,0 @@
-coverage
-flake8
-ipython
-ipdb
-pytest 
-pytest-cov 
-pytest-mock
-charms.reactive
-amulet
-requests
-juju-wait
-netifaces
-#--find-links https://launchpad.net/python-apt/+download/
-#python-apt
diff --git a/tests/bundles/bionic-mysql.yaml b/tests/bundles/bionic-mysql.yaml
new file mode 100644
index 0000000..0731f4d
--- /dev/null
+++ b/tests/bundles/bionic-mysql.yaml
@@ -0,0 +1,14 @@
+series: bionic
+
+applications:
+  percona-cluster:
+    charm: cs:percona-cluster
+    num_units: 1
+  telegraf:
+    num_units: 0
+
+relations:
+  - - percona-cluster:juju-info
+    - telegraf:juju-info
+  - - percona-cluster:db-admin
+    - telegraf:mysql
diff --git a/tests/bundles/bionic-postgres.yaml b/tests/bundles/bionic-postgres.yaml
new file mode 100644
index 0000000..a92b5c6
--- /dev/null
+++ b/tests/bundles/bionic-postgres.yaml
@@ -0,0 +1,14 @@
+series: bionic
+
+applications:
+  postgresql:
+    charm: cs:postgresql
+    num_units: 1
+  telegraf:
+    num_units: 0
+
+relations:
+  - - postgresql:juju-info
+    - telegraf:juju-info
+  - - postgresql:db-admin
+    - telegraf:postgresql
diff --git a/tests/bundles/overlays/local-charm-overlay.yaml.j2 b/tests/bundles/overlays/local-charm-overlay.yaml.j2
new file mode 100644
index 0000000..e78574f
--- /dev/null
+++ b/tests/bundles/overlays/local-charm-overlay.yaml.j2
@@ -0,0 +1,3 @@
+applications:
+  {{ charm_name }}:
+    charm: "{{ CHARM_BUILD_DIR }}/{{ charm_name }}"
diff --git a/tests/bundles/xenial-mysql.yaml b/tests/bundles/xenial-mysql.yaml
new file mode 100644
index 0000000..8e60469
--- /dev/null
+++ b/tests/bundles/xenial-mysql.yaml
@@ -0,0 +1,14 @@
+series: xenial
+
+applications:
+  percona-cluster:
+    charm: cs:percona-cluster
+    num_units: 1
+  telegraf:
+    num_units: 0
+
+relations:
+  - - percona-cluster:juju-info
+    - telegraf:juju-info
+  - - percona-cluster:db-admin
+    - telegraf:mysql
diff --git a/tests/bundles/xenial-postgres.yaml b/tests/bundles/xenial-postgres.yaml
new file mode 100644
index 0000000..9f597b6
--- /dev/null
+++ b/tests/bundles/xenial-postgres.yaml
@@ -0,0 +1,14 @@
+series: xenial
+
+applications:
+  postgresql:
+    charm: cs:postgresql
+    num_units: 1
+  telegraf:
+    num_units: 0
+
+relations:
+  - - postgresql:juju-info
+    - telegraf:juju-info
+  - - postgresql:db-admin
+    - telegraf:postgresql
diff --git a/tests/requirements.txt b/tests/requirements.txt
new file mode 100644
index 0000000..b26080d
--- /dev/null
+++ b/tests/requirements.txt
@@ -0,0 +1,2 @@
+git+https://github.com/openstack-charmers/zaza.git#egg=zaza
+charmhelpers>=0.20.8
diff --git a/tests/test_mysql.py b/tests/test_mysql.py
deleted file mode 100644
index 1fbdd38..0000000
--- a/tests/test_mysql.py
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/usr/bin/python3
-#
-# Copyright 2015-2016 Canonical Ltd.
-#
-# This file is part of the Telegraf Charm for Juju.
-#
-# 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 warranties of
-# MERCHANTABILITY, SATISFACTORY QUALITY, 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 <http://www.gnu.org/licenses/>.
-
-import os
-import subprocess
-import unittest
-
-import amulet
-import requests
-
-
-class TestMySQL(unittest.TestCase):
-    @classmethod
-    def setUpClass(cls):
-        # amulet only detects the charm name correctly if run from
-        # the root directory. Work around this.
-        os.environ['JUJU_TEST_CHARM'] = 'telegraf'
-        charm_dir = os.path.join(os.path.dirname(__file__), os.pardir)
-
-        cls.d = amulet.Deployment(series=os.environ.get('SERIES', 'trusty'))
-
-        cls.d.add('mysql', 'cs:mysql', units=1)
-        cls.d.add('telegraf', charm_dir)
-        cls.d.expose('telegraf')
-        cls.d.relate('mysql:juju-info', 'telegraf:juju-info')
-        cls.d.relate('mysql:db-admin', 'telegraf:mysql')
-
-        cls.d.setup(timeout=900)
-        cls.d.sentry.wait()
-
-        subprocess.check_call(['juju', 'wait'], universal_newlines=True)
-
-    def test_service(self):
-        for sentry in self.d.sentry['telegraf']:
-            with self.subTest(unit=sentry.info['unit_name']):
-                # test we can access over http
-                addr = sentry.info['public-address']
-                url = 'http://{}:9103/metrics'.format(addr)
-                page = requests.get(url)
-                self.assertEqual(page.status_code, 200)
-                # Confirm Mysql specific metrics exist.
-                # self.assertIn('mysql_blks_hit', page.text)
-                for i in page.text:
-                    print("DEBUG: {}".format(i))
-                # Confirm general metrics exist.
-                self.assertIn('cpu_usage_idle', page.text)
-
-
-if __name__ == '__main__':
-    unittest.main()
diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py
deleted file mode 100644
index 6497e1a..0000000
--- a/tests/test_postgresql.py
+++ /dev/null
@@ -1,63 +0,0 @@
-#!/usr/bin/python3
-#
-# Copyright 2015-2016 Canonical Ltd.
-#
-# This file is part of the Telegraf Charm for Juju.
-#
-# 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 warranties of
-# MERCHANTABILITY, SATISFACTORY QUALITY, 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 <http://www.gnu.org/licenses/>.
-
-import os
-import subprocess
-import unittest
-
-import amulet
-import requests
-
-
-class TestPostgreSQL(unittest.TestCase):
-    @classmethod
-    def setUpClass(cls):
-        # amulet only detects the charm name correctly if run from
-        # the root directory. Work around this.
-        os.environ['JUJU_TEST_CHARM'] = 'telegraf'
-        charm_dir = os.path.join(os.path.dirname(__file__), os.pardir)
-
-        cls.d = amulet.Deployment(series=os.environ.get('SERIES', 'xenial'))
-
-        cls.d.add('postgresql', 'cs:postgresql', units=2)
-        cls.d.add('telegraf', charm_dir)
-        cls.d.expose('telegraf')
-        cls.d.relate('postgresql:juju-info', 'telegraf:juju-info')
-        cls.d.relate('postgresql:db', 'telegraf:postgresql')
-
-        cls.d.setup(timeout=900)
-        cls.d.sentry.wait()
-
-        subprocess.check_call(['juju', 'wait'], universal_newlines=True)
-
-    def test_service(self):
-        for sentry in self.d.sentry['telegraf']:
-            with self.subTest(unit=sentry.info['unit_name']):
-                # test we can access over http
-                addr = sentry.info['public-address']
-                url = 'http://{}:9103/metrics'.format(addr)
-                page = requests.get(url)
-                self.assertEqual(page.status_code, 200)
-                # Confirm PostgreSQL specific metrics exist.
-                self.assertIn('postgresql_blks_hit', page.text)
-                # Confirm general metrics exist.
-                self.assertIn('cpu_usage_idle', page.text)
-
-
-if __name__ == '__main__':
-    unittest.main()
diff --git a/tests/test_telegraf.py b/tests/test_telegraf.py
new file mode 100644
index 0000000..a6e9201
--- /dev/null
+++ b/tests/test_telegraf.py
@@ -0,0 +1,56 @@
+# Copyright 2015-2020 Canonical Ltd.
+#
+# This file is part of the Telegraf Charm for Juju.
+#
+# 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 warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, 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 <http://www.gnu.org/licenses/>.
+import itertools as it
+import logging
+import time
+import unittest
+
+import requests
+
+from zaza.utilities import juju
+from zaza import model
+
+
+DEFAULT_HTTPGET_TIMEOUT = 10
+DEFAULT_RETRIES = 12
+DEFAULT_RETRIES_TIMEOUT = 5
+DEFAULT_TELEGRAF_EXPORTER_PORT = "9103"
+
+
+class TestTelegraf(unittest.TestCase):
+    def test_service(self):
+        principal_units = (model.get_units(app)
+                           for app in juju.get_principle_applications("telegraf"))
+        for unit in it.chain.from_iterable(principal_units):
+            if not unit.public_address:
+                continue
+            url = "http://{}:{}/metrics".format(unit.public_address, DEFAULT_TELEGRAF_EXPORTER_PORT)
+            for retry in range(DEFAULT_RETRIES):
+                resp = requests.get(url, timeout=DEFAULT_HTTPGET_TIMEOUT)
+                self.assertEqual(resp.status_code, 200)
+
+                if ((unit.name.startswith("postgresql/") and
+                     "postgresql_blks_hit" not in resp.text) or
+                        "cpu_usage_idle" not in resp.text):
+                    time.sleep(DEFAULT_RETRIES_TIMEOUT)
+                    continue
+
+                logging.info("test_service: Unit {} is reachable".format(unit.name))
+                break
+            else:
+                msg = "test_service: Unit {} is NOT reachable".format(unit.name)
+                logging.error(msg)
+                self.fail(msg)
diff --git a/tests/tests.yaml b/tests/tests.yaml
index 409122d..2f77959 100644
--- a/tests/tests.yaml
+++ b/tests/tests.yaml
@@ -1,13 +1,19 @@
-# This file tells bundletester how to run the integration tests.
-# 
-sources:
-  - ppa:juju/stable
-  - ppa:stub/juju
-packages:
-  - juju-wait
-reset: true
-makefile:
-  - lint
-  - test
-  - integration_xenial
-  - integration_trusty
+charm_name: telegraf
+gate_bundles:
+  - xenial-mysql
+  - xenial-postgres
+  - bionic-mysql
+  - bionic-postgres
+smoke_bundles:
+  - bionic-mysql
+  - bionic-postgres
+dev_bundles:
+  - bionic-mysql
+  - bionic-postgres
+target_deploy_status:
+  telegraf:
+    workload-status-message: Monitoring
+  postgresql:
+    workload-status-message: Live master
+tests:
+  - tests.test_telegraf.TestTelegraf
diff --git a/tox.ini b/tox.ini
index ee773e7..3a24f7b 100644
--- a/tox.ini
+++ b/tox.ini
@@ -15,35 +15,36 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 [tox]
-envlist = lint,test
+envlist = lint,unit,func
 skipsdist = True
-minversion = 2.3
 
-[flake8]
-ignore = E402,E501
+[testenv]
+basepython = python3
+whitelist_externals = juju
 
 [testenv:lint]
-basepython = python3
 deps = flake8
 commands = flake8 {posargs:reactive/ unit_tests/ tests/}
 
-[testenv:test]
-basepython=python3
-deps = -r{toxinidir}/test_requirements.txt
-commands = py.test {posargs:-v unit_tests/}
+[testenv:unit]
+deps = -r{toxinidir}/unit_tests/requirements.txt
+       -r{toxinidir}/requirements.txt
+commands = pytest -v --ignore {toxinidir}/tests
 
-[testenv:integration_xenial]
-basepython=python3
-deps = -r{toxinidir}/test_requirements.txt
-passenv = JUJU_* HOME
-setenv =
-    SERIES=xenial
-commands = py.test {posargs:-v tests/}
+[testenv:func]
+commands = functest-run-suite --keep-model
+deps = -r{toxinidir}/tests/requirements.txt
+       -r{toxinidir}/requirements.txt
+passenv =
+  HOME
+  CHARM_BUILD_DIR
+  MODEL_SETTINGS
 
-[testenv:integration_trusty]
-basepython=python3
-deps = -r{toxinidir}/test_requirements.txt
-passenv = JUJU_* HOME
-setenv =
-    SERIES=trusty
-commands = py.test {posargs:-v tests/}
+[flake8]
+ignore = E402,E501,W504
+exclude =
+  .git,
+  __pycache__,
+  .tox,
+max-line-length = 120
+max-complexity = 10
diff --git a/unit_tests/requirements.txt b/unit_tests/requirements.txt
new file mode 100644
index 0000000..6d56da6
--- /dev/null
+++ b/unit_tests/requirements.txt
@@ -0,0 +1,6 @@
+coverage
+flake8
+pytest
+pytest-cov
+pytest-mock
+netifaces
diff --git a/unit_tests/test_telegraf.py b/unit_tests/test_telegraf.py
index 085e593..f3a0b1a 100644
--- a/unit_tests/test_telegraf.py
+++ b/unit_tests/test_telegraf.py
@@ -47,6 +47,9 @@ def setup(monkeypatch, tmpdir):
     monkeypatch.setitem(os.environ, 'JUJU_UNIT_NAME', 'telegraf/0')
     monkeypatch.setattr(telegraf, 'get_remote_unit_name', lambda: 'remote-unit-0')
     monkeypatch.setattr(telegraf, 'exec_timeout_supported', lambda: True)
+    monkeypatch.setattr(telegraf.hookenv, 'status_set', lambda status, msg: None)
+    monkeypatch.setattr(telegraf.hookenv, 'log', lambda msg, **kw: None)
+    monkeypatch.setattr(telegraf.host, 'service_resume', lambda svc: None)
 
     # patch host.write for non-root
     user = getpass.getuser()
@@ -105,7 +108,7 @@ def temp_config_dir(monkeypatch, tmpdir):
 
 @pytest.fixture(autouse=True)
 def config(monkeypatch, temp_charm_dir):
-    raw_config = yaml.load(open('config.yaml', 'r'))
+    raw_config = yaml.safe_load(open('config.yaml', 'r'))
     data = dict((k, v['default']) for k, v in raw_config['options'].items())
     config = Config(data)
     monkeypatch.setattr(telegraf.hookenv, 'config', lambda: config)