Comment 2 for bug 853910

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.