Landscape Charm

Merge lp://qastaging/~stephanpampel/landscape-charm/systemd-monitoring into lp://qastaging/~landscape/landscape-charm/trunk

  1. systemd-monitoring
  2. Merge into trunk
Proposed by Stephan Pampel
Status: Rejected
Rejected by: Simon Poirier
Proposed branch: lp://qastaging/~stephanpampel/landscape-charm/systemd-monitoring
Merge into: lp://qastaging/~landscape/landscape-charm/trunk
Diff against target: 2957 lines (+1912/-189)
26 files modified
charm-helpers.yaml (+1/-0)
charmhelpers/__init__.py (+6/-4)
charmhelpers/contrib/charmsupport/__init__.py (+13/-0)
charmhelpers/contrib/charmsupport/nrpe.py (+522/-0)
charmhelpers/contrib/hahelpers/apache.py (+5/-1)
charmhelpers/contrib/hahelpers/cluster.py (+47/-2)
charmhelpers/core/decorators.py (+38/-0)
charmhelpers/core/hookenv.py (+184/-35)
charmhelpers/core/host.py (+262/-60)
charmhelpers/core/host_factory/ubuntu.py (+13/-5)
charmhelpers/core/services/base.py (+7/-2)
charmhelpers/core/strutils.py (+7/-4)
charmhelpers/core/sysctl.py (+12/-2)
charmhelpers/core/unitdata.py (+3/-3)
charmhelpers/fetch/__init__.py (+7/-2)
charmhelpers/fetch/python/packages.py (+6/-4)
charmhelpers/fetch/snap.py (+3/-3)
charmhelpers/fetch/ubuntu.py (+341/-59)
charmhelpers/fetch/ubuntu_apt_pkg.py (+312/-0)
charmhelpers/osplatform.py (+27/-3)
config.yaml (+20/-0)
hooks/nrpe-external-master-relation-changed (+9/-0)
hooks/nrpe-external-master-relation-joined (+9/-0)
lib/callbacks/nrpe.py (+53/-0)
lib/services.py (+2/-0)
metadata.yaml (+3/-0)
To merge this branch: bzr merge lp://qastaging/~stephanpampel/landscape-charm/systemd-monitoring
Reviewer Review Type Date Requested Status
🤖 Landscape Builder test results Needs Fixing
Jeremy Lounder (community) Needs Information
Simon Poirier Pending
Review via email: mp+411083@code.qastaging.launchpad.net

Commit message

Add charmhelpers.contrib.charmsupport dependency to charm-helpers.yaml to get nrpe.py
Sync charm-helpers
Add relation: nrpe-external-master
Add nrpe checks check_systemd for landscape services

Description of the change

To extend the monitoring for landscape servers I added systemd service checks for the landscape units the work when a 'nrpe-external-master' relation is added.
Example bundle: https://pastebin.canonical.com/p/GtKXbwDfbR/

To post a comment you must log in.
Revision history for this message
Stephan Pampel (stephanpampel) wrote :

Unit tests are currently failing: https://pastebin.canonical.com/p/yZKXH64pPG/
But the charm works well locally.

@simpoir Could this be caused by the charm-helpers sync? Or do maybe you have another idea why they are failing?

Revision history for this message
Simon Poirier (simpoir) wrote :

> @simpoir Could this be caused by the charm-helpers sync? Or do maybe you have
> another idea why they are failing?

The NRPE you imported doesn't use the hookenv stub but calls juju `config-get` directly in tests. So yes, your change breaks the tests.

Revision history for this message
Stephan Pampel (stephanpampel) wrote :

> > @simpoir Could this be caused by the charm-helpers sync? Or do maybe you
> have
> > another idea why they are failing?
>
> The NRPE you imported doesn't use the hookenv stub but calls juju `config-get`
> directly in tests. So yes, your change breaks the tests.

Thank you for looking into it. I am not sure how to fix it - does it mean I can not use the NRPE class from charmhelpers contrib.charmsupport.nrpe?

Revision history for this message
Stephan Pampel (stephanpampel) wrote :

I added the config option 'enable_monitoring' which puts the code behind a config flag.
All ci-tests are passing now: https://pastebin.canonical.com/p/mRRQgKJxcX/

Revision history for this message
Kevin Nasto (silverdrake11) :
Revision history for this message
Stephan Pampel (stephanpampel) wrote :

All changes in the 'charmhelpers/' folder are from running 'make sync' to update the charmhelpers library (upstream: https://github.com/juju/charm-helpers).
I had to run this to get 'contrib.charmsupport.nrpe' to create nrpe checks.

Revision history for this message
Jeremy Lounder (jldev) wrote :

@stephanpampel - Can you explain why you added a configuration value for enabling/disabling monitoring? The existence of the NRPE relationship should be enough to indicate the administrator wants monitoring enabled.

review: Needs Information
Revision history for this message
🤖 Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: make ci-test
Result: Fail
Revno: 408
Branch: lp:~stephanpampel/landscape-charm/systemd-monitoring
Jenkins: https://jenkins.canonical.com/landscape/job/latch-test-xenial/1450/

review: Needs Fixing (test results)
Revision history for this message
Linda Guo (lihuiguo) wrote :

> All changes in the 'charmhelpers/' folder are from running 'make sync' to
> update the charmhelpers library (upstream: https://github.com/juju/charm-
> helpers).
> I had to run this to get 'contrib.charmsupport.nrpe' to create nrpe checks.

I have fixed this issue. The changes are being reviewed by this proposal

https://code.launchpad.net/~lihuiguo/landscape-charm/bug-1934816/+merge/411583

Revision history for this message
Simon Poirier (simpoir) wrote :

Rejected MP: superseeded by lihuiguo's MP

Unmerged revisions

408. By Stephan Pampel

Add config flag 'enable_monitoring' to disable NRPE checks by default

407. By Stephan Pampel

Add charmhelpers.contrib.charmsupport dependency to charm-helpers.yaml to get nrpe.py
Sync charm-helpers
Add relation: nrpe-external-master
Add nrpe checks check_systemd for landscape services

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