Juju Charms Collection
kibana package

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

  1. Trusty Tahr (14.04)
  2. add-dashboard-loader-action
  3. Merge into trunk
Proposed by Charles Butler
Status: Merged
Merged at revision: 23
Proposed branch: lp://staging/~lazypower/charms/trusty/kibana/add-dashboard-loader-action
Merge into: lp://staging/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://staging/~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.staging.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
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches