Merge lp:~lazypower/charms/trusty/kibana/add-dashboard-loader-action into lp:charms/trusty/kibana

[Charles Butler]

Fixes for user already existing contributed by @manjo
Fixes against the amulet test suite contributed by @kjackal
Adds a path to deploy dashboards from configuration (useful for bundles)

[Cory Johns (johnsca)]

The load-dashboards action param and dashboard config option descriptions should mention what dashboards are supported out of the box (i.e., "beats").

If the dashboard config option is left at its default value, the elasticsearch <-> kiabana relation is added, and then any other config option is changed, the config-changed hook fails because the config value is empty and it tries to call action-get from outside of an action. See inline comment below.

If the dashboard config is set before the elasticsearch relation is joined (likely in a bundle), the rest-relation- hooks will not trigger the dashboard load, and it may not be triggered until a manual config change.

If the elasticsearch relation is required for the dashboard to be loaded, why isn't the action enforcing that? Also, shouldn't the dashboard be using the relation values (e.g., $ES_HOST) instead of localhost?

review: Needs Fixing

[Charles Butler (lazypower)]

cory, thanks for the detailed review.

I can address the final comment with a follow up comment - no. There's a virtual host setup to proxy connections over to the elastic search host (this is important, otherwise you have to juju expose elastic search. Kibana works client-side, and directly queries elastic search over a javascript based client side lib.

So good catch, but not a concern in this context.

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/4318/

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/4300/

review: Needs Fixing (automated testing)

=== modified file 'actions/load-dashboard'
--- actions/load-dashboard	2016-03-21 17:39:10 +0000
+++ actions/load-dashboard	2016-05-20 20:00:24 +0000
@@ -2,10 +2,22 @@
 
 set -ex
 
-DASH=$(action-get dashboard)
+# Support being called directly, or via action context
+if [ ! -z $1 ]; then
+  DASH=$1
+else
+  DASH=$(action-get dashboard)
+fi
 
+# Determine if we have a dashboard available
 if [[ -d ${CHARM_DIR}/dashboards/${DASH} ]]; then
  cd  $CHARM_DIR/dashboards/$DASH
  $CHARM_DIR/dashboards/$DASH/load.sh
- exit 0
+ cd $CHARM_DIR
+ else
+   juju-log "Missing Dashboard $DASH"
+   # fail the action if we are in an action context
+   if [ ! -z $JUJU_ACTION_UUID ]; then
+    action-fail "Missing Dashboard $DASH"
+   fi
 fi
 
=== modified file 'config.yaml'
--- config.yaml	2015-11-03 23:06:35 +0000
+++ config.yaml	2016-05-20 20:00:24 +0000
@@ -16,3 +16,9 @@
         default: ''
         description: 'The url path of kibana. This is required if kibana is
         proxied at a non root path. e.g. http://example.com/kibana/'
+    dashboard:
+        type: string
+        default: ''
+        description: |
+          Dashboard to load on deployment. Must exist in 'dashboards' directory
+          and contain a load.sh script to populate into elasticsearch.
 
=== modified file 'hooks/config-changed'
--- hooks/config-changed	2015-11-03 23:06:35 +0000
+++ hooks/config-changed	2016-05-20 20:00:24 +0000
@@ -15,3 +15,10 @@
 open-port $PORT/tcp
 echo $PORT > .PREVOPENPORT
 
+# Determine if we have the relation required to execute dashboard loading
+WE_HAVE_SIGNAL=$(relation-ids rest)
+if [ ! -z $WE_HAVE_SIGNAL ]; then
+  DASHBOARD=$(config-get dashboard)
+  juju-log "Loading Dashboard $DASHBOARD"
+  actions/load-dashboard $DASHBOARD
+fi
 
=== modified file 'hooks/install'
--- hooks/install	2015-11-04 00:47:13 +0000
+++ hooks/install	2016-05-20 20:00:24 +0000
@@ -24,10 +24,15 @@
     cp $KIBANA_TARBALL /tmp/kibana.tgz
 fi
 
-groupadd kibana
-useradd -s /bin/false -g kibana kibana
-mkdir -p /var/log/kibana
-chown kibana:kibana /var/log/kibana
+if ! grep -q kibana /etc/group ; then
+    groupadd kibana
+fi
+
+if ! /usr/bin/id -u kibana &>/dev/null ; then
+    useradd -s /bin/false -g kibana kibana
+fi
+
+[ -d /var/log/kibana ] || { mkdir -p /var/log/kibana ; chown kibana:kibana /var/log/kibana ; } 
 
 install -o root -g root -m 0644 files/charm/nginx.conf /etc/nginx/sites-available/kibana
 install -o root -g root -m 0644 files/charm/nginx_lb.conf /etc/nginx/sites-available/es_cluster
 
=== removed file 'tests/00-setup'
--- tests/00-setup	2014-10-29 15:52:18 +0000
+++ tests/00-setup	1970-01-01 00:00:00 +0000
@@ -1,16 +0,0 @@
-#!/bin/bash
-
-# This script sets up the requirements for amulet tests.
-
-set -x 
-
-# Check if amulet is installed before adding the stable repository and updating apt-get.
-dpkg -s amulet
-if [ $? -ne 0 ]; then
-    sudo add-apt-repository -y ppa:juju/stable
-    sudo apt-get update -qq
-    sudo apt-get install -y amulet
-fi
-
-# Install any additional python packages and other required software.
-sudo apt-get install -y python3-requests
 
=== modified file 'tests/10-bundles-test.py'
--- tests/10-bundles-test.py	2014-12-04 18:31:05 +0000
+++ tests/10-bundles-test.py	2016-05-20 20:00:24 +0000
@@ -6,6 +6,7 @@
 import unittest
 import yaml
 import amulet
+import requests
 
 seconds_to_wait = 960
 
@@ -28,6 +29,49 @@
         """ Test to see if the bundle deployed successfully. """
         self.assertTrue(self.d.deployed)
 
+    def test_port_change(self):
+        """ Change kibana port and validate the change happened """
+        self.d.configure('kibana', {'port': 81})
+        self.d.sentry.wait()
+
+        url = 'http://%s:81/' % kibana_unit.info['public-address']
+        r = requests.get(url)
+        assert r.ok
+
+    def test_web_interface():
+
+        elasticsearch_unit.run("""
+            curl -X POST 'http://127.0.0.1:9200/person/1' -d '{
+            "info" : {
+                    "height" : 2,
+                    "width" : 20
+            }
+            }'
+        """)
+
+        time.sleep(3)
+
+        # Ensure the kibana unit can query ES via localhost (it proxies local 9200
+        # queries to ES)
+        url = 'http://127.0.0.1:9200/_all/_search'
+        result = kibana_unit.run(
+            "curl -XGET {url} -d '@11-scale-elastic-query.json'".format(url=url))
+        if not result[1] == 0:
+            amulet.raise_status(
+                amulet.FAIL, msg="Error performing search. Error code: {}. "
+                                 "Result:\n{}".format(result[1], result[0]))
+        try:
+            result = json.loads(result[0])
+            if result['hits']['total'] != 1:
+                amulet.raise_status(amulet.FAIL,
+                                    msg="Error inserting value in elasticsearch.")
+        except ValueError:
+            exc_type, value, traceback = sys.exc_info()
+            exc_text = "Exception: (%s) %s. %s" % (value, exc_type, traceback)
+            amulet.raise_status(
+                amulet.FAIL, msg="Value returned not a valid json.\n{}\n{}".format(
+                    result[0], exc_text))
+
 
 if __name__ == '__main__':
     unittest.main()
 
=== modified file 'tests/11-scale-elastic.py'
--- tests/11-scale-elastic.py	2015-11-09 03:08:32 +0000
+++ tests/11-scale-elastic.py	2016-05-20 20:00:24 +0000
@@ -38,6 +38,7 @@
 
 
 def test_web_interface():
+    hits = get_total_hits()
 
     elasticsearch_unit.run("""
         curl -X POST 'http://127.0.0.1:9200/person/1' -d '{
@@ -48,8 +49,19 @@
         }'
     """)
 
-    time.sleep(3)
-
+    iteration = 0
+    newhits = get_total_hits()
+    while iteration < 10 and newhits-hits != 1:
+        time.sleep(3)
+        newhits = get_total_hits()
+        iteration += 1
+
+    if newhits-hits != 1:
+        amulet.raise_status(amulet.FAIL,
+                            msg="Error inserting value in elasticsearch.")
+
+
+def get_total_hits():
     # Ensure the kibana unit can query ES via localhost (it proxies local 9200
     # queries to ES)
     url = 'http://127.0.0.1:9200/_all/_search'
@@ -59,11 +71,10 @@
         amulet.raise_status(
             amulet.FAIL, msg="Error performing search. Error code: {}. "
                              "Result:\n{}".format(result[1], result[0]))
+
     try:
         result = json.loads(result[0])
-        if result['hits']['total'] != 1:
-            amulet.raise_status(amulet.FAIL,
-                                msg="Error inserting value in elasticsearch.")
+        return result['hits']['total']
     except ValueError:
         exc_type, value, traceback = sys.exc_info()
         exc_text = "Exception: (%s) %s. %s" % (value, exc_type, traceback)
 
=== modified file 'tests/bundles.yaml'
--- tests/bundles.yaml	2014-10-29 15:52:18 +0000
+++ tests/bundles.yaml	2016-05-20 20:00:24 +0000
@@ -5,3 +5,6 @@
     kibana:
       charm: kibana
       num_units: 1
+  elasticsearch:
+    charm: cs:trusty/elasticsearch
+    num_units: 1
 
=== added file 'tests/tests.yaml'
--- tests/tests.yaml	1970-01-01 00:00:00 +0000
+++ tests/tests.yaml	2016-05-20 20:00:24 +0000
@@ -0,0 +1,3 @@
+packages:
+  - python3-amulet
+  - python3-requests