Merge ~vultaire/charm-graylog:adding-zaza-tests into ~graylog-charmers/charm-graylog:master

[Paul Goins]

Adding initial zaza tests via "tox -e func"

This patch provides zaza-compatible ports of the charm's original
amulet-based test suite along with the relevant modifications
necessary to enable them to be run. This is intended to provide a
base for future development which includes functional testing.

* lib/charms/layer/graylog/logextract.py: Converted class-level 'url' variables into properties; is_v2() calls juju helpers and thus is not suitable to be called at import-time. If left as-is, tests cannot run because of the missing juju helper binaries.

* tests/README.md: Provided simple instructions about how to build the charm and run the functional tests. Includes important notes about running behind a proxy.

* tests/bundles/**: Temporarily removed the bionic-ha bundle (since we don't yet have HA support), and replaced the bionic bundle with different versions for Graylog 2 and 3. Other small changes were done, including (but not limited to) changes to better allow tests to run via Zaza.

* tests/test_legacy.py: This is a suite of tests based upon the original Amulet-driven tests included with this charm in the past, but ported to work with Zaza. The majority of the original test coverage should be provided by these new tests.

* tests/tests.yaml: This file appears to have been more-or-less a placeholder. Now it provides a real test and has been updated to allow clean deploy and settlement of the bundles.

* tox.ini: A build command has been added, and minimal changes have been made to the func target to allow it to run zaza tests correctly.

[🤖 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)]

Overall, everything looks good, thank you. I added minor comments (ie. make tests support future HA), and a strong suggestion to use Makefile (and document it in tests/README) to build the charm and run func tests. The Makefile is already there, so the MP probably needs a minor tweak and the tests/README to be updated.

review: Needs Fixing

[Paul Goins (vultaire)]

Understood. I will make the updates regarding get_graylog_client() and the Makefile. I will also update this to go more in line with the path Peter has taken in his MR regarding building charms (https://code.launchpad.net/~peter-sabaini/graylog-charm/+git/graylog-charm/+merge/377651).

[Paul Goins (vultaire)]

OK; I've re-worked my code upon the new code in trunk. This is ready for review once more.

[Alvaro Uria (aluria)]

MP looks great, thank you. I haven't run "make test" but skimmed the changes below.

review: Approve

[Xav Paice (xavpaice)]

Can we please rebase this against master? Currently won't merge.

review: Needs Fixing

[Xav Paice (xavpaice)]

Although the actual tests are still failing for me, I'm aware there's other changes queued up that fix this and this change makes a lot of progress towards good testing. Manual testing the charm as is works fine, including looking at the resulting zaza model. Approving.

review: Approve

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

Change successfully merged at revision 9e01c58565939a645ab53d617cddaebbdbcca467

diff --git a/lib/charms/layer/graylog/logextract.py b/lib/charms/layer/graylog/logextract.py
index 5d1ae27..42c034b 100644
--- a/lib/charms/layer/graylog/logextract.py
+++ b/lib/charms/layer/graylog/logextract.py
@@ -48,13 +48,21 @@ class GraylogTitleSourceItems:
 
 
 class GraylogPipelines(GraylogTitleSourceItems):
-    url = '{}/system/pipelines/pipeline'.format(
-        '/plugins/org.graylog.plugins.pipelineprocessor' if is_v2() else '')
+    @property
+    def url(self):
+        if is_v2():
+            return '/plugins/org.graylog.plugins.pipelineprocessor/system/pipelines/pipeline'
+        else:
+            return '/system/pipelines/pipeline'
 
 
 class GraylogRules(GraylogTitleSourceItems):
-    url = '{}/system/pipelines/rule'.format(
-        '/plugins/org.graylog.plugins.pipelineprocessor' if is_v2() else '')
+    @property
+    def url(self):
+        if is_v2():
+            return '/plugins/org.graylog.plugins.pipelineprocessor/system/pipelines/rule'
+        else:
+            return '/system/pipelines/rule'
 
 
 class GraylogStreams:
diff --git a/tests/README.md b/tests/README.md
new file mode 100644
index 0000000..50a965f
--- /dev/null
+++ b/tests/README.md
@@ -0,0 +1,12 @@
+# How to run functional tests
+
+1. export JUJU_REPOSITORY=<directory for built charms>
+2. Optional: export MODEL_SETTINGS=<semicolon-separated list of "juju model-config" settings>
+3. From the repository root, run: make test
+
+NOTE: If you are behind a proxy, be sure to export a MODEL_SETTINGS variable as
+described above.  Note that you will need to use the http-proxy, https-proxy and
+similar settings rather than the app-specific settings as the elasticsearch
+charm (at the time of writing) requires the environment setting for running
+Ansible apt-key actions, but the charm doesn't presently provide a way of
+setting this.
diff --git a/tests/bundles/bionic.yaml b/tests/bundles/bionic-graylog2.yaml
similarity index 91%
rename from tests/bundles/bionic.yaml
rename to tests/bundles/bionic-graylog2.yaml
index b646c7a..bd256db 100644
--- a/tests/bundles/bionic.yaml
+++ b/tests/bundles/bionic-graylog2.yaml
@@ -12,11 +12,13 @@ applications:
   graylog:
     num_units: 1
     series: bionic
-  
+    options:
+      channel: 2/stable
+
   elastic:
     charm: cs:elasticsearch
     num_units: 1
-  
+
   mongo:
     charm: cs:mongodb
     num_units: 1
@@ -25,6 +27,9 @@ applications:
     charm: cs:haproxy
     num_units: 1
 
+  nrpe:
+    charm: cs:nrpe
+
 relations:
   - - ubuntu
     - filebeat
@@ -36,3 +41,5 @@ relations:
     - elastic
   - - graylog
     - haproxy
+  - - graylog
+    - nrpe
diff --git a/tests/bundles/bionic-graylog3.yaml b/tests/bundles/bionic-graylog3.yaml
new file mode 100644
index 0000000..596cf7d
--- /dev/null
+++ b/tests/bundles/bionic-graylog3.yaml
@@ -0,0 +1,45 @@
+series: bionic
+
+applications:
+  ubuntu:
+    charm: cs:ubuntu
+    num_units: 1
+  
+  filebeat:
+    charm: cs:filebeat
+    num_units: 0
+  
+  graylog:
+    num_units: 1
+    series: bionic
+    options:
+      channel: 3/stable
+  
+  elastic:
+    charm: cs:elasticsearch
+    num_units: 1
+  
+  mongo:
+    charm: cs:mongodb
+    num_units: 1
+
+  haproxy:
+    charm: cs:haproxy
+    num_units: 1
+
+  nrpe:
+    charm: cs:nrpe
+
+relations:
+  - - ubuntu
+    - filebeat
+  - - graylog:beats
+    - filebeat:logstash
+  - - graylog
+    - mongo
+  - - graylog
+    - elastic
+  - - graylog
+    - haproxy
+  - - graylog
+    - nrpe
diff --git a/tests/bundles/bionic-ha.yaml b/tests/bundles/bionic-ha.yaml
index 516cff6..70cc1e9 100644
--- a/tests/bundles/bionic-ha.yaml
+++ b/tests/bundles/bionic-ha.yaml
@@ -1,3 +1,4 @@
+# NOTE: Not in use yet.
 series: bionic
 
 applications:
@@ -12,6 +13,8 @@ applications:
   graylog:
     num_units: 3
     series: bionic
+    options:
+      channel: 3/stable
   
   elastic:
     charm: cs:elasticsearch
@@ -25,6 +28,9 @@ applications:
     charm: cs:haproxy
     num_units: 1
 
+  nrpe:
+    charm: cs:nrpe
+
 relations:
   - - ubuntu
     - filebeat
@@ -36,3 +42,5 @@ relations:
     - elastic
   - - graylog
     - haproxy
+  - - graylog
+    - nrpe
diff --git a/tests/test_legacy.py b/tests/test_legacy.py
new file mode 100644
index 0000000..a92d927
--- /dev/null
+++ b/tests/test_legacy.py
@@ -0,0 +1,110 @@
+import re
+import unittest
+import yaml
+from zaza.utilities import juju
+from zaza import model
+
+from api import GraylogApi
+
+
+DEFAULT_API_PORT = "9001"  # Graylog 2 only
+DEFAULT_WEB_PORT = "9000"
+
+
+class LegacyTests(unittest.TestCase):
+
+    # These tests were ported from tests/test_10_basic.py in changeset a3b54c2.
+    # They were temporarily deleted during the conversion from Amulet to Zaza.
+    # Aside from porting to work with libjuju and some refactoring, these tests
+    # are largely as they were before.
+
+    def test_api_ready(self):
+        """Curl the api endpoint on the graylog unit."""
+        port = self.get_api_port()
+        curl_command = "curl http://localhost:{} --connect-timeout 30".format(port)
+        self.run_command("graylog/0", curl_command)
+
+    def test_elasticsearch_active(self):
+        self._assert_remote_address_in_server_conf("elastic/0", "client")
+        # This was also stuck in the same test...
+        api = self.get_graylog_clients()[0]
+        resp = api.indexer_cluster_health()
+        self.assertTrue(resp["status"] == "green")
+
+    def test_mongodb_active(self):
+        self._assert_remote_address_in_server_conf("mongo/0", "database")
+        # This was also stuck in the same test...
+        api = self.get_graylog_clients()[0]
+        resp = api.cluster_get()
+        self.assertTrue(resp[list(resp)[0]]["is_processing"])
+
+    def _assert_remote_address_in_server_conf(self, remote_unit, remote_relation_name):
+        # This is a very simplistic, naive check, but it's equivalent to
+        # that from tests/test_10_basic.py in a3b54c2.
+        source_unit = "graylog/0"
+        remote_addr = juju.get_relation_from_unit(
+            source_unit, remote_unit, remote_relation_name
+        )["private-address"]
+        assert remote_addr in self.get_file_contents(
+            source_unit, "/var/snap/graylog/common/server.conf"
+        )
+
+    def get_graylog_clients(self):
+        graylog_units = juju.get_full_juju_status().applications["graylog"]["units"]
+        graylog_addrs = [unit["public-address"] for unit in graylog_units.values()]
+        port = self.get_api_port()
+        password = self.get_graylog_admin_password()
+        return [
+            GraylogApi("http://{}:{}/api".format(graylog_addr, port), "admin", password)
+            for graylog_addr in graylog_addrs
+        ]
+
+    def get_api_port(self):
+        port = DEFAULT_API_PORT if self.is_v2() else DEFAULT_WEB_PORT
+        return port
+
+    def get_graylog_admin_password(self):
+        result = model.run_action("graylog/0", "show-admin-password")
+        password = result.data["results"]["admin-password"]
+        return password
+
+    def test_website_active(self):
+        data = juju.get_relation_from_unit("haproxy/0", "graylog/0", "website")
+        self.assertEqual(data["port"], DEFAULT_WEB_PORT)
+
+        service_ports = {}
+        for service in yaml.safe_load(data["all_services"]):
+            service_ports[service["service_name"]] = service["service_port"]
+
+        self.assertEqual(str(service_ports["web"]), DEFAULT_WEB_PORT)
+        if self.is_v2():
+            self.assertEqual(str(service_ports["api"]), DEFAULT_API_PORT)
+
+    def run_command(self, target, command, timeout=30):
+        result = model.run_on_unit(target, command, timeout=timeout)
+        assert result["Code"] == "0"
+        return result
+
+    def is_v2(self):
+        return (
+            juju.get_full_juju_status()
+            .applications["graylog"]["workload-version"]
+            .startswith("2.")
+        )
+
+    def test_nrpe_config(self):
+        cfg = self.get_file_contents(
+            "nrpe/0", "/etc/nagios/nrpe.d/check_graylog_health.cfg"
+        )
+        self.assertTrue(re.search(r"command.*check_graylog_health", cfg))
+
+    def test_nrpe_health_check(self):
+        cfg = self.get_file_contents(
+            "nrpe/0", "/usr/local/lib/nagios/plugins/check_graylog_health.py"
+        )
+        self.assertTrue(re.search(r"CRITICAL", cfg))
+
+    def get_file_contents(self, unit, filename):
+        result = self.run_command(unit, "cat '{}'".format(filename))
+        assert result["Code"] == "0"
+        return result["Stdout"]
diff --git a/tests/tests.yaml b/tests/tests.yaml
index 3a0510d..658fc3e 100644
--- a/tests/tests.yaml
+++ b/tests/tests.yaml
@@ -1,14 +1,20 @@
 charm_name: graylog
 gate_bundles:
-  - bionic
-  - bionic-ha
+  - bionic-graylog2
+  - bionic-graylog3
+  #- bionic-ha   # Currently bionic-ha is identical to bionic-graylog3; enable
+                 # once we make changes.
 smoke_bundles:
-  - bionic
+  - bionic-graylog3
 dev_bundles:
-  - bionic
+  - bionic-graylog3
 tests:
+  - tests.test_legacy.LegacyTests
   - tests.test_graylog_charm.CharmOperationTest
 target_deploy_status:
   filebeat:
     workload-status: active
     workload-status-message: 'Filebeat ready.'
+  haproxy:
+    workload-status: unknown
+    workload-status-message: ""