Merge lp://staging/~sinzui/charms/precise/juju-gui/nagios into lp://staging/charms/juju-gui

Proposed by Curtis Hovey
Status: Merged
Merged at revision: 74
Proposed branch: lp://staging/~sinzui/charms/precise/juju-gui/nagios
Merge into: lp://staging/charms/juju-gui
Diff against target: 446 lines (+378/-3)
8 files modified
config.yaml (+11/-1)
files/nrpe-external-master/check-app-access.sh (+18/-0)
metadata.yaml (+3/-0)
revision (+1/-1)
scripts/charmsupport/hookenv.py (+150/-0)
scripts/charmsupport/nrpe.py (+169/-0)
scripts/update-nrpe.py (+14/-0)
tests/20-functional.test (+12/-1)
To merge this branch: bzr merge lp://staging/~sinzui/charms/precise/juju-gui/nagios
Reviewer Review Type Date Requested Status
Benji York (community) Approve
charmers Pending
Review via email: mp+177588@code.staging.launchpad.net

Description of the change

Add nagios support.

RULES

    Pre-implementation: sidnei, moon127, wedgwood
    * We want to support the nagios subordinate charm to monitor production.
    * Webops prefer to use the nrpe module from charmsupport.
      * The project merged with charmhelpers (not related to the charm-helpers
        ppa that all PPA configured juju deploys get).
      * Webops write a custom script for each charm they deploy to embed the
        library in the charm; they alway deploy a forked charm. The charm
        runs with a modified python path to find that included libs.
      * Webops require the charm to ensure errors installing dependencies are
        are ignored since they have ensured the libs are already in the charm.
        * Or have they? Every change to the charm requires review of a webop
          to ensure the building of a fat charm really works.
    * I decided to include just the nrpe module and its dependencies in
      the charm's tree.
      * No action is needed to build out a fat charm.
      * No future coordination with webops to ensure the charm really
        installs its deps.
      * I chose an older version of nrpe because it has fewer deps than
        the current version. I chose the same version used by charmworld.
        I'd like to keep these two charms synced to simplify the parts used
        to deploy charmworld.com

QA

    DEPLOY JUJU-GUI AND NAGIOS
    juju bootstrap
    juju deploy --repo=~/Work/juju-charms/ local:precise/juju-gui
    juju deploy nagios
    juju add-relation nagios juju-gui
    juju expose nagios
    Verify you can see juju-gui in nagios with a single ssh check

    DEPLOY NRPE-EXTERNAL-MASTER
    juju deploy nrpe-external-master
    juju set nrpe-external-master nagios_master=http://10.0.3.45/<nagio-public-ip>
    juju add-relation nrpe-external-master:nrpe-external-master juju-gui:nrpe-external-master

    Verify you can see the nagios external cfg files in juju-gui's file system.
    ls /var/lib/nagios/export/
    Verify the unit config "host__juju-juju-gui-0.cfg" has the the nagio ip address.

IMPLEMENTATION

    * I added just the parts of the nrpe library needed.
      scripts/charmsupport/__init__.py
      scripts/charmsupport/hookenv.py
      scripts/charmsupport/nrpe.py

    * I added a hook that calls the update-nrpe.py script that in turn
      registers the check-front-page.sh script. nrpe requires the
      check scripts to be in files/nrpe-external-master/.
      hooks/nrpe-external-master-relation-changed
      scripts/update-nrpe.py
      files/nrpe-external-master/check-front-page.sh

    * I updated the metadata and config to enable the subordinate charm.
      config.yaml
      metadata.yaml
      revision

To post a comment you must log in.
78. By Curtis Hovey

Merged tip and resolved conflicts.

79. By Curtis Hovey

Merged tip.

Revision history for this message
Benji York (benji) wrote :

Thanks for wiring this up. Monitoring is essential.

Given that this MP doesn't have Reitveld links, I assume you didn't use
lbox to propose it. Despite it's many annoyances, lbox is the
prescribed way of proposing and landing GUI branches. The most
important thing it does is to run "make check" before proposing a
branch. Please manually run that command before landing. Thanks.

> I decided to include just the nrpe module and its dependencies in the
> charm's tree.

It's a shame that our policies push us toward doing things like copy
chunks of code from outdated dependencies, but I can't proffer anything
better.

In check-front-page.sh (line 31 of the diff) shouldn't it be "sites-enabled"
instead of "sites-available"?

Also, I suggest using "https://127.0.0.1:443/juju-ui/version.js" for ADDRESS
and "jujuGuiVersionInfo" for LIFE_SIGN. Being code, that is less likely
to change because of a UI/UX redesign and we can write a test for it
that ensures if it does change a test named "testMonitoringEndpoint"
will fail. Oh, and we need that test in this branch too.

Once the above are addressed, this will be ready to land.

review: Approve
80. By Curtis Hovey

change script to check-app-access.sh because it now checks that the
GUI can be downloaded.

81. By Curtis Hovey

Added test to correlate check-app-access.sh to the path it checks.

Revision history for this message
Gary Poster (gary) wrote :

Hey Curtis. Thank you! Is it reasonable/easy to merge these revisions into the ~juju-gui charm branch as well, or are there conflicts?

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