Merge ~npochet/charm-graylog:graylog-ha into ~graylog-charmers/charm-graylog:master

[Nicolas Pochet]

Support multiple nodes

* Select the inital master based on juju leadership. This information is
then stored for further use.
* The master is only changed if the actual master is departed.
* The elected leader will also send its password to the others.
* Migrate functional tests to zaza
* Fixes LP:1794631

Support multiple nodes

* Select the inital master based on juju leadership. This information is
then stored for further use.
* The master is only changed if the actual master is departed.
* The elected leader will also send its password to the others.
* Migrate functional tests to zaza
* Fixes LP:1794631

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

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

[Stuart Bishop (stub)]

I don't think we can have the graylog master follow the Juju leader, because we will have periods where there are multiple masters (the old master will only learn that it is no longer master when the new leader publishes new leadership settings), and because we will get unnecessary master changes when controller or network issues trigger leadership elections (probably exactly the times when we want logging to be stable).

Instead, the leader should appoint the master, and the master will remain master until the unit departs.

from charms import leadership

@when('leadership.is_leader')
def appoint_master():
    master = leadership.leader_get('master')
    if master and (master == hookenv.local_unit() or master in set(hookenv.expected_peer_units())):
        return # Already have a master
    leadership.leader_set(master, hookenv.local_unit()) # Appoint myself master

def is_master():
    return leadership.leader_get('master') == hookenv.local_unit())

If you are worried about a failed unit, you could also have the leader perform a liveness check to graylog running on the master (which should handle the master being powered off). However, this risks a split brain with two live graylogs thinking they are master (the original proposal has the same flaw too, where a master loses network connectivity to Juju, and a new unit gets leadership and we get a new master and the old master will have no idea until the network connectivity to Juju is repaired).

review: Needs Fixing

[Stuart Bishop (stub)]

(everything else is great, including the zaza tests)

[Nicolas Pochet (npochet)]

I'll apply the changes and update the MP today
Regarding zaza, see my comment inline

[Nicolas Pochet (npochet)]

Regarding zaza, I'm not sure that my previous comment is valid. I can see, in the openstack charms that are using zaza, that they always use master.
For example: https://github.com/openstack/charm-vault/blob/60c35da8b638cf0854bc1ac8039a66499c9259cb/src/test-requirements.txt#L4

[Stuart Bishop (stub)]

This seems good, so +1 from ~canonical-is-reviewers. Should get someone else from graylog-charmers for a second opinion.

review: Approve

[Jeremy Lounder (jldev)]

We're going to hold on this merge until we fully understand the scope of what is required to make Graylog HA. While having multi-node Graylog does have some benefits, such as distributed ingestion. I need to take the time to understand how the Graylog hostnames are communicated out via relation, and how they are consumed on relations by other charms/services.
At the moment, it would appear there is no recovery mechanism if a node fails. It may make sense to put haproxy in front of the ingestion endpoint - in addition to the web endpoint.

review: Needs Information

[Jeremy Lounder (jldev)]

Approving this for merge and release. Additional research reveals that the HA process for our implementation is handled on the filebeats side. Filebeats does client-end loading balancing and delivery guarantee.

For other input types, we'll still need to evaluate improving the haproxy relation, and placing a load balancer in front of the inputs.

review: Approve

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

Change successfully merged at revision f95473e145f97f5a7e6bec9e1b44e2e289f3b2fb

diff --git a/Makefile b/Makefile
index 9e5089e..44e05cc 100644
--- a/Makefile
+++ b/Makefile
@@ -21,11 +21,8 @@ sysdeps:
 
 .PHONY: testdeps
 testdeps:
-	-sudo apt-add-repository -y ppa:juju/stable
-	-sudo apt-add-repository -y ppa:tvansteenburgh/ppa
-	@sudo apt update
-	@sudo apt install -y juju charm amulet python-pip python3-nose
-	@sudo pip2 install bundletester
+	@sudo snap install juju
+	@sudo snap install charm
 
 .PHONY: lint
 lint: sysdeps
@@ -67,11 +64,6 @@ charmrelease:
 	@echo ---------------------------------------------------------------------
 
 .PHONY: test
-test: check-jujumodel testdeps
+test: testdeps charmbuild
 	@echo "Running functional tests (including lint and unit tests)..."
-	bundletester -t $(BUILTCHARMDIR) -Fvl DEBUG -e $(JUJU_MODEL)
-
-check-jujumodel:
-ifndef JUJU_MODEL
-	$(error JUJU_MODEL is undefined)
-endif
+	@tox -e func
diff --git a/layer.yaml b/layer.yaml
index 3d0f751..d4fa469 100644
--- a/layer.yaml
+++ b/layer.yaml
@@ -2,6 +2,7 @@ repo: git+ssh://git.launchpad.net/graylog-charm
 includes:
   - 'layer:basic'
   - 'layer:snap'
+  - 'layer:leadership'
   - 'interface:elasticsearch'
   - 'interface:elastic-beats'
   - 'interface:http'
diff --git a/reactive/graylog.py b/reactive/graylog.py
index 786df04..e408e0c 100644
--- a/reactive/graylog.py
+++ b/reactive/graylog.py
@@ -10,6 +10,7 @@ from charms.reactive import hook, when, when_not, is_state, remove_state, set_st
 from charms.reactive.helpers import data_changed
 from charmhelpers.core import host, hookenv, unitdata
 from charmhelpers.contrib.charmsupport import nrpe
+import charms.leadership
 
 from charms.layer.graylog import logextract
 from charms.layer.graylog.api import GraylogApi
@@ -102,6 +103,40 @@ def configure_graylog():
     report_status()
 
 
+@when('leadership.is_leader')
+@when_not('leadership.set.admin_password')
+def generate_admin_password():
+    admin_password = host.pwgen(18)
+    charms.leadership.leader_set(admin_password=admin_password)
+
+
+@when('leadership.changed.admin_password')
+def get_leader_admin_password():
+    db = unitdata.kv()
+    admin_password = charms.leadership.leader_get('admin_password')
+    db.set('admin_password', admin_password)
+    remove_state('graylog.configured')
+
+
+@when('leadership.is_leader')
+def appoint_master():
+    master = charms.leadership.leader_get('master')
+    if master and (master == hookenv.local_unit() or master in set(hookenv.expected_peer_units())):
+        # A master already exists
+        return
+    # Appoint myself as master
+    charms.leadership.leader_set(master=hookenv.local_unit())
+
+
+def is_master():
+    return charms.leadership.leader_get('master') == hookenv.local_unit()
+
+
+@when('leadership.changed.master')
+def set_master():
+    set_conf('is_master', is_master())
+
+
 @when('graylog.configured')  # noqa: C901
 def report_status():
     beats_connected = is_state('beats.connected')
diff --git a/tests/bundles/bionic-ha.yaml b/tests/bundles/bionic-ha.yaml
new file mode 100644
index 0000000..e266cf5
--- /dev/null
+++ b/tests/bundles/bionic-ha.yaml
@@ -0,0 +1,39 @@
+series: bionic
+
+applications:
+  ubuntu:
+    charm: cs:ubuntu
+    num_units: 1
+  
+  filebeat:
+    charm: cs:filebeat
+    num_units: 0
+  
+  graylog:
+    charm: ../../../graylog-built/builds/graylog
+    num_units: 3
+    series: bionic
+  
+  elastic:
+    charm: cs:elasticsearch
+    num_units: 1
+  
+  mongo:
+    charm: cs:mongodb
+    num_units: 1
+
+  haproxy:
+    charm: cs:haproxy
+    num_units: 1
+
+relations:
+  - - ubuntu
+    - filebeat
+  - - graylog:beats
+    - filebeat:logstash
+  - - graylog
+    - mongo
+  - - graylog
+    - elastic
+  - - graylog
+    - haproxy
\ No newline at end of file
diff --git a/tests/bundles/bionic.yaml b/tests/bundles/bionic.yaml
new file mode 100644
index 0000000..e196355
--- /dev/null
+++ b/tests/bundles/bionic.yaml
@@ -0,0 +1,39 @@
+series: bionic
+
+applications:
+  ubuntu:
+    charm: cs:ubuntu
+    num_units: 1
+  
+  filebeat:
+    charm: cs:filebeat
+    num_units: 0
+  
+  graylog:
+    charm: ../../../graylog-built/builds/graylog
+    num_units: 1
+    series: bionic
+  
+  elastic:
+    charm: cs:elasticsearch
+    num_units: 1
+  
+  mongo:
+    charm: cs:mongodb
+    num_units: 1
+
+  haproxy:
+    charm: cs:haproxy
+    num_units: 1
+
+relations:
+  - - ubuntu
+    - filebeat
+  - - graylog:beats
+    - filebeat:logstash
+  - - graylog
+    - mongo
+  - - graylog
+    - elastic
+  - - graylog
+    - haproxy
\ No newline at end of file
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..b459ab1
--- /dev/null
+++ b/tests/bundles/overlays/local-charm-overlay.yaml.j2
@@ -0,0 +1,3 @@
+applications:
+  graylog:
+    charm: ../../../graylog-built/builds/graylog
\ No newline at end of file
diff --git a/tests/requirements.txt b/tests/requirements.txt
new file mode 100644
index 0000000..3da5fb4
--- /dev/null
+++ b/tests/requirements.txt
@@ -0,0 +1 @@
+git+https://github.com/openstack-charmers/zaza.git#egg=zaza
\ No newline at end of file
diff --git a/tests/test_10_basic.py b/tests/test_10_basic.py
deleted file mode 100755
index 54133c2..0000000
--- a/tests/test_10_basic.py
+++ /dev/null
@@ -1,152 +0,0 @@
-#!/usr/bin/python3
-
-import amulet
-import os
-import re
-import sys
-import time
-import unittest
-import yaml
-
-libs_dir = os.path.abspath(os.path.join(os.path.dirname(__file__),
-                                        '..', 'lib'))
-if libs_dir not in sys.path:
-    sys.path.append(libs_dir)
-
-from graylogapi import GraylogAPI  # noqa: E402
-
-
-ADMIN_PASSWORD = 'admin'
-CLUSTER_NAME = 'amulet'
-CURL_TIMEOUT = 180
-DEFAULT_API_PORT = '9001'
-DEFAULT_WEB_PORT = '9000'
-
-
-class TestCharm(unittest.TestCase):
-    @classmethod
-    def setUpClass(cls):
-        cls.d = amulet.Deployment(series='xenial')
-
-        cls.d.add('graylog')
-        cls.d.configure('graylog', {
-            'elasticsearch_cluster_name': CLUSTER_NAME,
-        })
-        cls.d.expose('graylog')
-
-        cls.d.add('nrpe', 'cs:nrpe')
-        cls.d.relate('graylog:nrpe-external-master',
-                     'nrpe:nrpe-external-master')
-
-        cls.d.add('mongodb', 'cs:mongodb')
-        cls.d.configure('mongodb', {
-            'replicaset': CLUSTER_NAME,
-        })
-        cls.d.relate('graylog:mongodb', 'mongodb:database')
-
-        cls.d.add('elasticsearch', 'cs:elasticsearch')
-        cls.d.configure('elasticsearch', {
-            'cluster-name': CLUSTER_NAME,
-        })
-        cls.d.relate('graylog:elasticsearch', 'elasticsearch:client')
-
-        cls.d.add('apache2', 'cs:apache2')
-        cls.d.relate('graylog:website', 'apache2:reverseproxy')
-        cls.d.expose('apache2')
-
-        try:
-            timeout = int(os.environ.get('AMULET_SETUP_TIMEOUT', 900))
-            cls.d.setup(timeout=timeout)
-            cls.d.sentry.wait(timeout=timeout)
-            cls.d.sentry.wait_for_messages({'graylog': re.compile('Ready with'),
-                                            'elasticsearch': 'Ready',
-                                            'nrpe': 'ready'},
-                                           timeout=timeout)
-        except amulet.helpers.TimeoutError:
-            amulet.raise_status(
-                amulet.FAIL,
-                msg="Deployment timed out ({}s)".format(timeout)
-            )
-        except Exception:
-            raise
-
-        cls.elasticsearch = cls.d.sentry['elasticsearch'][0]
-        cls.mongodb = cls.d.sentry['mongodb'][0]
-        cls.graylog = cls.d.sentry['graylog'][0]
-        cls.nrpe = cls.d.sentry['nrpe'][0]
-        cls.graylog.run_action('set-admin-password', {'password': ADMIN_PASSWORD})
-
-        cls.config = cls.graylog.file_contents('/var/snap/graylog/common/server.conf')
-        api_url = "http://{}:{}/api".format(cls.graylog.info['public-address'], DEFAULT_API_PORT)
-        cls.api = GraylogAPI(api_url, 'admin', ADMIN_PASSWORD)
-
-        # https://bugs.launchpad.net/graylog-charm/+bug/1748040
-        cls.graylog.run('open-port {}'.format(DEFAULT_API_PORT))
-
-    def test_api_ready(self):
-        ''' Curl the api endpoint on the sentried graylog unit. We'll retry
-            until the CURL_TIMEOUT because it may take a few seconds for the
-            graylog systemd service to start.'''
-        curl_command = 'curl http://localhost:{}'.format(DEFAULT_API_PORT)
-        timeout = time.time() + CURL_TIMEOUT
-        while time.time() < timeout:
-            response = self.graylog.run(curl_command)
-            # run returns a msg,retcode tuple
-            if response[1] == 0:
-                return
-            else:
-                print("Unexpected curl response: {}. "
-                      "Retrying in 30s.".format(response[0]))
-                time.sleep(30)
-
-        # we didn't get rc=0 in the alloted time; raise amulet failure
-        msg = (
-            "Graylog didn't respond to the command \n"
-            "'{curl_command}' as expected.\n"
-            "Return code: {return_code}\n"
-            "Result: {result}".format(
-                curl_command=curl_command,
-                return_code=response[1],
-                result=response[0])
-        )
-        amulet.raise_status(amulet.FAIL, msg=msg)
-
-    def test_elasticsearch_active(self):
-        elasticsearch_ip = self.elasticsearch.relation('client',
-                                                       'graylog:elasticsearch')['private-address']
-        self.assertTrue(elasticsearch_ip in self.config)
-        resp = self.api.indexer_cluster_health()
-        self.assertTrue(resp['status'] == 'green')
-
-    def test_mongodb_active(self):
-        mongodb_ip = self.mongodb.relation('database', 'graylog:mongodb')['private-address']
-        self.assertTrue(mongodb_ip in self.config)
-        resp = self.api.cluster_get()
-        self.assertTrue(resp[list(resp)[0]]['is_processing'])
-
-    def test_website_active(self):
-        relation = self.graylog.relation('website', 'apache2:reverseproxy')
-        self.assertEqual(relation['port'], DEFAULT_WEB_PORT)
-
-        service_ports = {}
-        for service in yaml.safe_load(relation['all_services']):
-            service_ports[service['service_name']] = service['service_port']
-
-        self.assertEqual(str(service_ports['web']), DEFAULT_WEB_PORT)
-        self.assertEqual(str(service_ports['api']), DEFAULT_API_PORT)
-
-    def test_nrpe_config(self):
-        cfg = self.nrpe.file_contents(
-            '/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.nrpe.file_contents(
-            '/usr/local/lib/nagios/plugins/check_graylog_health.py')
-        self.assertTrue(
-            re.search(r'CRITICAL', cfg))
-
-
-if __name__ == "__main__":
-    unittest.main()
diff --git a/tests/test_20_clustered.py b/tests/test_20_clustered.py
deleted file mode 100755
index 96284f7..0000000
--- a/tests/test_20_clustered.py
+++ /dev/null
@@ -1,112 +0,0 @@
-#!/usr/bin/python3
-
-import amulet
-import os
-import re
-import sys
-import time
-import unittest
-
-libs_dir = os.path.abspath(os.path.join(os.path.dirname(__file__),
-                                        '..', 'lib'))
-if libs_dir not in sys.path:
-    sys.path.append(libs_dir)
-
-from graylogapi import GraylogAPI  # noqa: E402
-
-
-ADMIN_PASSWORD = 'admin'
-CLUSTER_NAME = 'amulet'
-CURL_TIMEOUT = 180
-DEFAULT_API_PORT = '9001'
-DEFAULT_WEB_PORT = '9000'
-
-
-class TestCharm(unittest.TestCase):
-    @classmethod
-    def setUpClass(cls):
-        cls.d = amulet.Deployment(series='xenial')
-
-        cls.d.add('graylog')
-        # TODO add in clustered graylog when support for it exists
-        cls.d.configure('graylog', {
-            'elasticsearch_cluster_name': CLUSTER_NAME,
-        })
-        cls.d.expose('graylog')
-
-        cls.d.add('mongodb', 'cs:mongodb', units=3)
-        cls.d.configure('mongodb', {
-            'replicaset': CLUSTER_NAME,
-        })
-        cls.d.relate('graylog:mongodb', 'mongodb:database')
-
-        cls.d.add('elasticsearch', 'cs:elasticsearch', units=3)
-        cls.d.configure('elasticsearch', {
-            'cluster-name': CLUSTER_NAME,
-        })
-        cls.d.relate('graylog:elasticsearch', 'elasticsearch:client')
-
-        try:
-            cls.d.setup(timeout=900)
-            cls.d.sentry.wait_for_messages({'graylog': re.compile('Ready with'),
-                                            'elasticsearch': 'Ready'},
-                                           timeout=1800)
-        except amulet.helpers.TimeoutError:
-            # Setup didn't complete before timeout
-            pass
-
-        cls.graylog = cls.d.sentry['graylog'][0]
-        cls.graylog.run_action('set-admin-password', {'password': ADMIN_PASSWORD})
-
-        cls.config = cls.graylog.file_contents('/var/snap/graylog/common/server.conf')
-        api_url = "http://{}:{}/api".format(cls.graylog.info['public-address'], DEFAULT_API_PORT)
-        cls.api = GraylogAPI(api_url, 'admin', ADMIN_PASSWORD)
-
-        # https://bugs.launchpad.net/graylog-charm/+bug/1748040
-        cls.graylog.run('open-port {}'.format(DEFAULT_API_PORT))
-
-    def test_api_ready(self):
-        ''' Curl the api endpoint on the sentried graylog unit. We'll retry
-            until the CURL_TIMEOUT because it may take a few seconds for the
-            graylog systemd service to start.'''
-        curl_command = 'curl http://localhost:{}'.format(DEFAULT_API_PORT)
-        timeout = time.time() + CURL_TIMEOUT
-        while time.time() < timeout:
-            response = self.graylog.run(curl_command)
-            # run returns a msg,retcode tuple
-            if response[1] == 0:
-                return
-            else:
-                print("Unexpected curl response: {}. "
-                      "Retrying in 30s.".format(response[0]))
-                time.sleep(30)
-
-        # we didn't get rc=0 in the alloted time; raise amulet failure
-        msg = (
-            "Graylog didn't respond to the command \n"
-            "'{curl_command}' as expected.\n"
-            "Return code: {return_code}\n"
-            "Result: {result}".format(
-                curl_command=curl_command,
-                return_code=response[1],
-                result=response[0])
-        )
-        amulet.raise_status(amulet.FAIL, msg=msg)
-
-    def test_elasticsearch_active(self):
-        resp = self.api.indexer_cluster_health()
-        self.assertTrue(resp['status'] == 'green')
-        for elasticsearch in self.d.sentry['elasticsearch']:
-            ip = elasticsearch.relation('client', 'graylog:elasticsearch')['private-address']
-            self.assertTrue(ip in self.config)
-
-    def test_mongodb_active(self):
-        resp = self.api.cluster_get()
-        self.assertTrue(resp[list(resp)[0]]['is_processing'])
-        for mongodb in self.d.sentry['mongodb']:
-            ip = mongodb.relation('database', 'graylog:mongodb')['private-address']
-            self.assertTrue(ip in self.config)
-
-
-if __name__ == "__main__":
-    unittest.main()
diff --git a/tests/tests.yaml b/tests/tests.yaml
index 5c4cb8e..1bb1e58 100644
--- a/tests/tests.yaml
+++ b/tests/tests.yaml
@@ -1,5 +1,12 @@
-makefile:
-    - lint
-    - unittest
-packages:
-  - amulet
+charm_name: graylog
+gate_bundles:
+  - bionic
+  - bionic-ha
+smoke_bundles:
+  - bionic
+dev_bundles:
+  - bionic
+configure:
+  - zaza.charm_tests.noop.setup.basic_setup
+tests:
+  - zaza.charm_tests.noop.tests.NoopTest
diff --git a/tox.ini b/tox.ini
index d92a339..a1f299d 100644
--- a/tox.ini
+++ b/tox.ini
@@ -6,12 +6,18 @@ skip_missing_interpreters = True
 [testenv]
 basepython = python3
 setenv = PYTHONPATH={toxinidir}/reactive:{toxinidir}/lib/
+whitelist_externals = juju
+passenv = HOME
 
 [testenv:unit]
 commands = pytest -v --ignore {toxinidir}/tests --cov=lib --cov=reactive --cov=actions --cov-report=term-missing --cov-branch
 deps = -r{toxinidir}/unit_tests/requirements.txt
        -r{toxinidir}/requirements.txt
 
+[testenv:func]
+commands = functest-run-suite --keep-model
+deps = -r{toxinidir}/tests/requirements.txt
+
 [testenv:lint]
 commands = flake8
 deps = flake8
diff --git a/unit_tests/test_graylog.py b/unit_tests/test_graylog.py
index 9a25473..38ac6fa 100644
--- a/unit_tests/test_graylog.py
+++ b/unit_tests/test_graylog.py
@@ -1,14 +1,21 @@
 import os
+import sys
 import tempfile
 import unittest
 from unittest import mock
 
 from charms.layer.graylog.api import GraylogApi
+
+# charms.leadership only exists in the built charm; mock it out before
+# the graylog imports since those depend on charms.leadership
+layer_mock = mock.Mock()
+sys.modules['charms.leadership'] = layer_mock
+
 from reactive.graylog import (
     set_conf,
     set_jvm_heap_size,
-    _check_input_exists)
-from files import check_graylog_health
+    _check_input_exists)  # noqa: E402
+from files import check_graylog_health  # noqa: E402
 
 initial_conf = u"""#key1 = value1
 key2 = value2