added logstash-indexer charm

[Paul Czarkowski]

/charms/precise/logstash-indexer/trunk

basic standalong logstash indexer charm. hopefully will use as a starting point to built elastic-search and kibana charms and connectors/relationships.

[Paul Czarkowski (paulcz)]

updated charm to md5 checksum downloaded file, other cleanups.

[James Page (james-page)]

Hi Paul

Thanks for submitting this charm for inclusion in the charm store.

I've taken a first pass review of it; heres some feedback:

1) Redundant hooks; quite a few of the hooks do nothing - please feel free to drop the scripts as they just clutter the charm.

2) Jar file to download (not directly charm related but stopped me testing):

Resolving logstash.objects.dreamhost.com (logstash.objects.dreamhost.com)... 64.90.32.4, 2607:f298:4:143:acce:55:0:1
Connecting to logstash.objects.dreamhost.com (logstash.objects.dreamhost.com)|64.90.32.4|:443... connected.
HTTP request sent, awaiting response... 404 Not Found
2012-11-16 16:29:03 ERROR 404: Not Found.

Oops....

+1 for checking the checksum once downloaded tho - good thinking and very much best practice.

3) cluster-relation-changed

I'd put in a fast exit with zero return code if cluster-name or host are not yet set by the remote host - the hook will get called again when they are:

   [ -z "$(relation-get cluster-name)" ] && exit 0

or suchlike (each to his own style I say :-))

4) files/charm/logstash-indexer.conf

Contains a reference to BASEPATH which I think should be set to /opt/logstash; as the charm did not install unable to check..

5) HOME=`dirname $0` and associated use.

The charm executes in the root of the charm - so its safe to reference files/charm/logstash-indexer.conf for example. Saves a little clutter in the charm hooks.

6) Port 5140 syslog?

I am right in thinking that logstash is listening on 5140 for syslog data once everything is running? if so it might make sense to provide this as relation data to clients connecting to the input_syslog relation - that way there is not implicit knowledge required in the remote charm about where to send data.

7) Hook re-execution

install hook:

juju-log "create logstash user"
useradd ${USER}
juju-log "create skeleton dir"

Its possible an install hook (or any other hook) might get re-executed during the lifecycle of a charm; the above would fail on second execution as the user already exists; hook id-impotency is really important

Anyway - thats probably enough for a first review; please make amends and I'll take another look.

Cheers

James

[Paul Czarkowski (paulcz)]

thanks for the feedback, I'll work on it over the weekend and try and clear any of your comments/suggestions. some initial comments without having the code in front of me :-

2) the .jar file thing is painful, it's too big for bzr to upload ( I have a copy in my file/ dir but bzr complains ) so have to rely on the url provided on the logstash website, I guess it changed, or I somehow munged it.

4) There should be a sed line in the install hook to rename this. will check.

5) I had some weirdness accessing it files dir from relative files/ ... possibly because I was chdir'ing around the place during the install at first. HOME gave me a way to always get to the right place.

6) correct I added the syslog over port:5140 section just the other day but haven't done appropriate hooks for relations yet.

[Paul Czarkowski (paulcz)]

1) Redundant hooks; quite a few of the hooks do nothing - please feel free to drop the scripts as they just clutter the charm.

Cleaned up.

2) Jar file to download (not directly charm related but stopped me testing):

fixed .. . was a bad line in commons.sh

3) cluster-relation-changed

done.

4) files/charm/logstash-indexer.conf

sed fixes this during install.

5) HOME=`dirname $0` and associated use.

I've had charms ( both mine and others ) bite me with relying on relative paths .. this fixes that and ensures if a chdir occurs the paths remain accurate.

6) Port 5140 syslog?

yeah, that was test stuff. I've pulled it out and set up redis as the transport mechanism for logstash-agent and set up an appropriate relationship hook

7) Hook re-execution

added test for user to skip creation if already exists.

[Juan L. Negron (negronjl)]

Reviewing this now.

-Juan

[Juan L. Negron (negronjl)]

This looks good to me.

Reviewing the other two related charms ( kibana and logstash-indexer ) as well.

-Juan

[Juan L. Negron (negronjl)]

I man kibana and elasticsearch :)

-Juan

[Juan L. Negron (negronjl)]

Promulgated kibana.

-Juan

[Juan L. Negron (negronjl)]

Promulgated.

-Juan

[Clint Byrum (clint-fewbar)]

Excerpts from Paul Czarkowski's message of 2012-11-16 21:56:22 UTC:
> thanks for the feedback, I'll work on it over the weekend and try and
> clear any of your comments/suggestions. some initial comments
> without having the code in front of me :-
>
> 2) the .jar file thing is painful, it's too big for bzr to upload ( I
> have a copy in my file/ dir but bzr complains ) so have to rely on the
> url provided on the logstash website, I guess it changed, or I somehow
> munged it.
>
> 4) There should be a sed line in the install hook to rename this.
> will check.
>
> 5) I had some weirdness accessing it files dir from relative files/ ...
> possibly because I was chdir'ing around the place during the install at
> first. HOME gave me a way to always get to the right place.
>
> 6) correct I added the syslog over port:5140 section just the other day
> but haven't done appropriate hooks for relations yet.
>

Re the jar file, perhaps consider putting that into a package in a
PPA. That is meant specifically for delivering versioned binaries to
machines. It can be a very simple package which just copies the jar file
into /usr/share/something