Charm needed: collectd

Bug #853910 reported by Chuck Short
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Juju Charms Collection
Expired
High
Unassigned

Bug Description

Pretty monitor avec graphs

summary: - Formula needed: collectd
+ Charm needed: collectd
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

This is ready for charm store inclusion I think. The earlier charms by chuck short are still here for posterity, but I don't think they were ever completed.

Changed in charms:
assignee: nobody → Clint Byrum (clint-fewbar)
status: New → Confirmed
importance: Undecided → High
James Page (james-page)
Changed in charms:
assignee: Clint Byrum (clint-fewbar) → James Page (james-page)
status: Confirmed → In Progress
Revision history for this message
James Page (james-page) wrote :

Hi Clint

Nice work - a few comments:

1) collectd-server: juju set security-level=None

Changing the security-level config in the collectd-server updates the security level for the server, but not for any related subordinate services - this probably breaks communication as the settings are not the same at each end of the interface.

2) collectd-server: config.yaml

    graph-port:
        type: int
        default: 80
        description: |
            Setting this to "yes" will enable the 'collection3' cgi
            frontend to the RRD Graphs.

Description does not match the type of this config element.

3) collectd-server/manifests/templates/auth_file.erb

This is more of a observation rather than anything that needs changing; I understand why this template calls:

<%
    end
    system("relation-set -r #{relid} username=#{username} password=#{@charm_collectd_password} security-level=#{@config_security_level}")
end %>

After it adds each remote unit to the authentication file but it feels odd that the template itself is mutating the relation to the remote service unit. I'd prefer to see the relation sets elsewhere.... but not quite sure where.

4) Execution of commands in templates

Again more observation because it does work; it feels odd to have .erbs executing juju commands; an alternative approach might be to load this information into facter prior to rendering the template which would align better with the way you retrieve service config before rendering the template.

Other than that it all looked and sniffed out OK for me.

Changed in charms:
status: In Progress → Incomplete
assignee: James Page (james-page) → Clint Byrum (clint-fewbar)
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Hi James, thanks for the quick review.

1) Easy enough to handle. Added a call to hooks/apply to collectd-relation-joined .. now the facts will be updated (and the template relation-sets using @config_security_level).

2) done, this also made me look again and I realized the port was not actually being used, so there are some major changes in revision 9 to make that work.

3,4) There is no way to handle arrays in puppet's language. You'll notice the lack of the word 'loop' here http://docs.puppetlabs.com/guides/language_guide.html.

I think these things are fairly straightforward, though I agree templates seems wrong for this.

In the future I'll be experimenting with using the puppet ruby DSL instead of .pp files, and that will allow for this level of interaction outside of templates. Another option is to write a juju resource provider for puppet, but that will take some time to get right, and at some point will have diminishing returns I think.

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Alright, I've simplified the templates and moved all logic into the hook scripts which then feed into facts. I do think eventually a juju custom provider of some kind would make puppet based charms simpler. C'est la vie, not yet.

Changed in charms:
status: Incomplete → New
James Page (james-page)
Changed in charms:
assignee: Clint Byrum (clint-fewbar) → James Page (james-page)
status: New → In Progress
Revision history for this message
James Page (james-page) wrote :

Hi Clint

Finally got round to reviewing your updates; Although its more verbose I think I prefer the separation of logic and juju calls from the puppet templates.

The collectd-server looks OK - however I was unable to get the collectd sub to work - here are the errors:

1) install error

2012-11-12 10:49:57,326 unit:collectd/2: hook.output INFO: ++ relation-get -r collectd-server:1 security-level collectd-server/0

2012-11-12 10:49:57,519 unit:collectd/2: unit.hook.api DEBUG: Getting relation collectd-server:1
2012-11-12 10:49:57,538 unit:collectd/2: hook.output INFO: + unit_security_level=
+ '[' -n '' ']'
+ fact-add relation_units ''

2012-11-12 10:49:57,539 unit:collectd/2: hook.output INFO: Fact value is needed

2012-11-12 10:49:57,540 unit:collectd/2: hook.output DEBUG: hook install exited, exit code Traceback (most recent call last):
Failure: juju.errors.CharmInvocationError: Error processing '/var/lib/juju/units/collectd-2/units/collectd-2/charm/hooks/install': exit code 1.

2) Hacked in a -n check to drop through the fact-add failing and got:

2012-11-12 10:53:48,818 unit:collectd/0: hook.output INFO: + puppet apply --templatedir=/var/lib/juju/units/collectd-0/units/collectd-0/charm/manifests/templates manifests/collectd.pp

2012-11-12 10:53:53,217 unit:collectd/0: hook.output INFO: Failed to parse template collectd.conf.erb: private method `split' called for nil:NilClass at /var/lib/juju/units/collectd-0/units/collectd-0/charm/manifests/collectd.pp:26

I'm testing this on an openstack cloud for reference.

Changed in charms:
status: In Progress → Incomplete
assignee: James Page (james-page) → Clint Byrum (clint-fewbar)
Changed in charms:
assignee: Clint Byrum (clint-fewbar) → nobody
status: Incomplete → New
Jorge Castro (jorge)
Changed in charms:
status: New → Confirmed
Revision history for this message
Whit Morriss (whitmo) wrote :

Howdy Clint! thanks for your work on this charm. We'd love to get collectd into the store.

Taking a look at collectd and collectd-server, collectd-server deploys fine but the subordinate fails on install (replicating previous comment)

Also charm proof raises some warnings:
  W: Metadata is missing categories.
  W: No icon.svg file.

I know this charm has been hanging out for awhile, but if you are interested in fixing it up, we'd love to have it (and thanks for your work so far).

-w

Changed in charms:
status: Confirmed → Incomplete
Revision history for this message
Jorge Castro (jorge) wrote :

Since this charm doesn't have a maintainer anymore, this is available for anyone who wants to pick it up and fix it.

Revision history for this message
Adam Israel (aisrael) wrote :

I'll pick this up. Forking to my namespace, and I'll submit a new bug when it's ready for review.

Revision history for this message
Launchpad Janitor (janitor) wrote :

[Expired for Juju Charms Collection because there has been no activity for 60 days.]

Changed in charms:
status: Incomplete → Expired
Revision history for this message
José Antonio Rey (jose) wrote :

Hey Adam,

Any updates on this? If you're not taking it right now we could probably move it to new so someone can pick it up in the meanwhile, just so we can get it moving :)

Thanks for your continued work on improving the Charm ecosystem! :)

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.