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

Proposed by Charles Butler
Status: Merged
Merged at revision: 23
Proposed branch: lp:~lazypower/charms/trusty/kibana/add-dashboard-loader-action
Merge into: lp:charms/trusty/kibana
Diff against target: 231 lines (+102/-27)
9 files modified
actions/load-dashboard (+14/-2)
config.yaml (+6/-0)
hooks/config-changed (+7/-0)
hooks/install (+9/-4)
tests/00-setup (+0/-16)
tests/10-bundles-test.py (+44/-0)
tests/11-scale-elastic.py (+16/-5)
tests/bundles.yaml (+3/-0)
tests/tests.yaml (+3/-0)
To merge this branch: bzr merge lp:~lazypower/charms/trusty/kibana/add-dashboard-loader-action
Reviewer Review Type Date Requested Status
Review Queue (community) automated testing Needs Fixing
Cory Johns (community) Needs Fixing
Review via email: mp+295359@code.launchpad.net

Description of the change

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)

To post a comment you must log in.
Revision history for this message
Cory Johns (johnsca) wrote :

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
Revision history for this message
Charles Butler (lazypower) wrote :

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.

Revision history for this message
Review Queue (review-queue) wrote :

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)
Revision history for this message
Review Queue (review-queue) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'actions/load-dashboard'
2--- actions/load-dashboard 2016-03-21 17:39:10 +0000
3+++ actions/load-dashboard 2016-05-20 20:00:24 +0000
4@@ -2,10 +2,22 @@
5
6 set -ex
7
8-DASH=$(action-get dashboard)
9+# Support being called directly, or via action context
10+if [ ! -z $1 ]; then
11+ DASH=$1
12+else
13+ DASH=$(action-get dashboard)
14+fi
15
16+# Determine if we have a dashboard available
17 if [[ -d ${CHARM_DIR}/dashboards/${DASH} ]]; then
18 cd $CHARM_DIR/dashboards/$DASH
19 $CHARM_DIR/dashboards/$DASH/load.sh
20- exit 0
21+ cd $CHARM_DIR
22+ else
23+ juju-log "Missing Dashboard $DASH"
24+ # fail the action if we are in an action context
25+ if [ ! -z $JUJU_ACTION_UUID ]; then
26+ action-fail "Missing Dashboard $DASH"
27+ fi
28 fi
29
30=== modified file 'config.yaml'
31--- config.yaml 2015-11-03 23:06:35 +0000
32+++ config.yaml 2016-05-20 20:00:24 +0000
33@@ -16,3 +16,9 @@
34 default: ''
35 description: 'The url path of kibana. This is required if kibana is
36 proxied at a non root path. e.g. http://example.com/kibana/'
37+ dashboard:
38+ type: string
39+ default: ''
40+ description: |
41+ Dashboard to load on deployment. Must exist in 'dashboards' directory
42+ and contain a load.sh script to populate into elasticsearch.
43
44=== modified file 'hooks/config-changed'
45--- hooks/config-changed 2015-11-03 23:06:35 +0000
46+++ hooks/config-changed 2016-05-20 20:00:24 +0000
47@@ -15,3 +15,10 @@
48 open-port $PORT/tcp
49 echo $PORT > .PREVOPENPORT
50
51+# Determine if we have the relation required to execute dashboard loading
52+WE_HAVE_SIGNAL=$(relation-ids rest)
53+if [ ! -z $WE_HAVE_SIGNAL ]; then
54+ DASHBOARD=$(config-get dashboard)
55+ juju-log "Loading Dashboard $DASHBOARD"
56+ actions/load-dashboard $DASHBOARD
57+fi
58
59=== modified file 'hooks/install'
60--- hooks/install 2015-11-04 00:47:13 +0000
61+++ hooks/install 2016-05-20 20:00:24 +0000
62@@ -24,10 +24,15 @@
63 cp $KIBANA_TARBALL /tmp/kibana.tgz
64 fi
65
66-groupadd kibana
67-useradd -s /bin/false -g kibana kibana
68-mkdir -p /var/log/kibana
69-chown kibana:kibana /var/log/kibana
70+if ! grep -q kibana /etc/group ; then
71+ groupadd kibana
72+fi
73+
74+if ! /usr/bin/id -u kibana &>/dev/null ; then
75+ useradd -s /bin/false -g kibana kibana
76+fi
77+
78+[ -d /var/log/kibana ] || { mkdir -p /var/log/kibana ; chown kibana:kibana /var/log/kibana ; }
79
80 install -o root -g root -m 0644 files/charm/nginx.conf /etc/nginx/sites-available/kibana
81 install -o root -g root -m 0644 files/charm/nginx_lb.conf /etc/nginx/sites-available/es_cluster
82
83=== removed file 'tests/00-setup'
84--- tests/00-setup 2014-10-29 15:52:18 +0000
85+++ tests/00-setup 1970-01-01 00:00:00 +0000
86@@ -1,16 +0,0 @@
87-#!/bin/bash
88-
89-# This script sets up the requirements for amulet tests.
90-
91-set -x
92-
93-# Check if amulet is installed before adding the stable repository and updating apt-get.
94-dpkg -s amulet
95-if [ $? -ne 0 ]; then
96- sudo add-apt-repository -y ppa:juju/stable
97- sudo apt-get update -qq
98- sudo apt-get install -y amulet
99-fi
100-
101-# Install any additional python packages and other required software.
102-sudo apt-get install -y python3-requests
103
104=== modified file 'tests/10-bundles-test.py'
105--- tests/10-bundles-test.py 2014-12-04 18:31:05 +0000
106+++ tests/10-bundles-test.py 2016-05-20 20:00:24 +0000
107@@ -6,6 +6,7 @@
108 import unittest
109 import yaml
110 import amulet
111+import requests
112
113 seconds_to_wait = 960
114
115@@ -28,6 +29,49 @@
116 """ Test to see if the bundle deployed successfully. """
117 self.assertTrue(self.d.deployed)
118
119+ def test_port_change(self):
120+ """ Change kibana port and validate the change happened """
121+ self.d.configure('kibana', {'port': 81})
122+ self.d.sentry.wait()
123+
124+ url = 'http://%s:81/' % kibana_unit.info['public-address']
125+ r = requests.get(url)
126+ assert r.ok
127+
128+ def test_web_interface():
129+
130+ elasticsearch_unit.run("""
131+ curl -X POST 'http://127.0.0.1:9200/person/1' -d '{
132+ "info" : {
133+ "height" : 2,
134+ "width" : 20
135+ }
136+ }'
137+ """)
138+
139+ time.sleep(3)
140+
141+ # Ensure the kibana unit can query ES via localhost (it proxies local 9200
142+ # queries to ES)
143+ url = 'http://127.0.0.1:9200/_all/_search'
144+ result = kibana_unit.run(
145+ "curl -XGET {url} -d '@11-scale-elastic-query.json'".format(url=url))
146+ if not result[1] == 0:
147+ amulet.raise_status(
148+ amulet.FAIL, msg="Error performing search. Error code: {}. "
149+ "Result:\n{}".format(result[1], result[0]))
150+ try:
151+ result = json.loads(result[0])
152+ if result['hits']['total'] != 1:
153+ amulet.raise_status(amulet.FAIL,
154+ msg="Error inserting value in elasticsearch.")
155+ except ValueError:
156+ exc_type, value, traceback = sys.exc_info()
157+ exc_text = "Exception: (%s) %s. %s" % (value, exc_type, traceback)
158+ amulet.raise_status(
159+ amulet.FAIL, msg="Value returned not a valid json.\n{}\n{}".format(
160+ result[0], exc_text))
161+
162
163 if __name__ == '__main__':
164 unittest.main()
165
166=== modified file 'tests/11-scale-elastic.py'
167--- tests/11-scale-elastic.py 2015-11-09 03:08:32 +0000
168+++ tests/11-scale-elastic.py 2016-05-20 20:00:24 +0000
169@@ -38,6 +38,7 @@
170
171
172 def test_web_interface():
173+ hits = get_total_hits()
174
175 elasticsearch_unit.run("""
176 curl -X POST 'http://127.0.0.1:9200/person/1' -d '{
177@@ -48,8 +49,19 @@
178 }'
179 """)
180
181- time.sleep(3)
182-
183+ iteration = 0
184+ newhits = get_total_hits()
185+ while iteration < 10 and newhits-hits != 1:
186+ time.sleep(3)
187+ newhits = get_total_hits()
188+ iteration += 1
189+
190+ if newhits-hits != 1:
191+ amulet.raise_status(amulet.FAIL,
192+ msg="Error inserting value in elasticsearch.")
193+
194+
195+def get_total_hits():
196 # Ensure the kibana unit can query ES via localhost (it proxies local 9200
197 # queries to ES)
198 url = 'http://127.0.0.1:9200/_all/_search'
199@@ -59,11 +71,10 @@
200 amulet.raise_status(
201 amulet.FAIL, msg="Error performing search. Error code: {}. "
202 "Result:\n{}".format(result[1], result[0]))
203+
204 try:
205 result = json.loads(result[0])
206- if result['hits']['total'] != 1:
207- amulet.raise_status(amulet.FAIL,
208- msg="Error inserting value in elasticsearch.")
209+ return result['hits']['total']
210 except ValueError:
211 exc_type, value, traceback = sys.exc_info()
212 exc_text = "Exception: (%s) %s. %s" % (value, exc_type, traceback)
213
214=== modified file 'tests/bundles.yaml'
215--- tests/bundles.yaml 2014-10-29 15:52:18 +0000
216+++ tests/bundles.yaml 2016-05-20 20:00:24 +0000
217@@ -5,3 +5,6 @@
218 kibana:
219 charm: kibana
220 num_units: 1
221+ elasticsearch:
222+ charm: cs:trusty/elasticsearch
223+ num_units: 1
224
225=== added file 'tests/tests.yaml'
226--- tests/tests.yaml 1970-01-01 00:00:00 +0000
227+++ tests/tests.yaml 2016-05-20 20:00:24 +0000
228@@ -0,0 +1,3 @@
229+packages:
230+ - python3-amulet
231+ - python3-requests

Subscribers

People subscribed via source and target branches