New charm: logstash-forwarder

[Chris Stratford]

This charm is a log-shipper for logstash. It performs a similar role to logstash-agent, but is more lightweight (no Java)

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-1146-results

[Charles Butler (lazypower)]

Greetings Chris,

Thank you for the submission of the light weight logstash forwarder charm! I love the idea of having a log shipping agent that doesn't ship with a java dependency and will be following the development of this charm pretty closely. I've taken a moment to review the charm as is and have the following notes:

# Proof
The charm passes our automated testing that runs proof and any linting targets, which is great

# Missing Tests
For trusty based charms, we've started enforcing a policy update that trusty charms must include tests for our automated CI infrastructure. To learn more about the charm testing story give this post a quick read: http://blog.juju.solutions/cloud/juju/2014/10/02/charm-testing.html

This should help explain the why, and how to get moving quickly with charm testing. Along that veign there will also be a charm school today held at 3pm EDT over getting started with charm testing.

# Readme
The readme is very straight forward, it would be nice to see the addition of upstream contact information for the project should users become interested in following the project, or contributing to the further development of the charm/service based on their interaction with the Juju Charm. Additionally if you were to provide the Readme in markdown format, it renders much nicer in the charm store.

There are quite a few config options exposed in config.yaml that are not represented in the README, and it would be helpful to display proposed deployment workloads. Think brand new user vs a user that fully grokks Juju.

# Deploy Test
When deploying the charm 'as is' - I received an error during install: AttributeError: Config instance has no attribute 'apt_repository' - which appears to be related to not having anything specified in the config. This falls under sane defaults for deployment and warrants updating.

As an initial look this is a good first cut of the charm, and barring some modifications will be ready for the store in short order. We appreciate the submission and look forward to the next cut of the charm. I'm going to move the status of this bug to 'incomplete', and when you are ready for another review simply change the status to 'fix committed' and someone will be along shortly to review your work.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

[Chris Stratford (chris-gondolin)]

- Fixed apt_repository typo
- Improved README (now markdown format)
- Added some basic amulet tests

The amulet tests just check the everything has installed correctly. It does not test the complete logstash-forwarder -> logstash-indexer -> elasticsearch system.

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10352-results

[Chris Stratford (chris-gondolin)]

Fixed the amulet tests.

[Whit Morriss (whitmo)]

Howdy Chris,
I confirmed the test fix, repo fix and readme updates and conversion to markdown. Thanks for your updates!

I'd be concerned about inclusion in the charm store without providing relationshipto which elasticsearch or logstash indexer could connect. Add those relationship, and this could provide a great alternative to logstash-agent.

[Whit Morriss (whitmo)]

Found this back in the queue. No visible change.

[Tom Haddon (mthaddon)]

Hi Whit,

Which charm were you looking for a relation with? There is no logstash-indexer charm in trusty and I don't think you'd want to/be able to relate this directly to elasticsearch. We could mimic the "input" relation from the logstash-agent charm if that makes sense.