Request to review the wildfly-ha-master charm

[saurabh]

Request to review the wildfly-ha-master charm

[Review Queue (review-queue)]

The results (PASS) are in and available here: http://reports.vapour.ws/charm-tests/charm-bundle-test-10604-results

[Tim Van Steenburgh (tvansteenburgh)]

Hi saurabh, thanks for submitting your charm for review! I've added a Related Branch to this bug, in which I've included some suggested changes, which I'll explain below:

1. Added amulet tests (see tests/ dir). Tests are required for admission to the trusty charm store. I added a basic test -- you are welcome to add more (for example, a test that logs in to the admin console). You can find more info on amulet at https://juju.ubuntu.com/docs/tools-amulet.html

2. Added apt-get update before apt-get install in hooks/install. Ensures package lists are up-to-date before attempting installation.

3. Cleaned up README (formatting/typos).

4. -relation-changed hooks:
   - Use $CHARM_DIR to get path to templates
   - Replace sleep with a conditional that exits until data is available. Read more about this technique here: https://juju.ubuntu.com/docs/authors-relations-in-depth.html

I also notice that the charm seems to support a website relation, but there is no info in the README on how to use it -- that would be good to add.

I did not look closely at the slave charm, but you may wish to see which of these improvements would also apply there.

Great start on this charm! If you have any questions, please feel free to ping me (tvansteenburgh) in #juju on irc.freenode.net, or send an email to the mailing list at <email address hidden>.

[saurabh (jhasaurabh)]

Hi Tim,

I have made the chnages you pointed out. Have updated the slave charm as well.
The website relation was for Jenkins integration which I have removed for now. I am planning to do that in next iteration.

Please have a look at the recent changes and provide your valuable suggestion.

Thanks,
Saurabh

[Cory Johns (johnsca)]

Saurabh,

Thanks for creating this charm and for incorporating the suggestions from Tim. Both charms look good, and the test passes, so I give this my +1 for being promoted to Recommended.

However, there is a bit of a chicken-and-egg problem with it referencing the wilfly-ha-slave charm as a local: charm, since this would fail the automated test runner. Unfortunately, this isn't easily resolvable until both charms are in the recommended namespace in the store, so my recommendation is that, once both charms are promoted, the test is updated to use a cs: reference for the slave charm. The test files also need their permissions to be set to executable so that they will be picked up by the test runner.

[Matt Bruzek (mbruzek)]

Moving this bug to Fix Committed as the wildfly-ha-charm is in the recommended section of the charms store.

You can access this charm at: https://code.launchpad.net/~charmers/charms/trusty/wildfly-ha-slave/trunk

You can deploy this charm by typing:

juju deploy cs:trusty/wildfly-ha-master

Thank you for working with us on the review process and thank you for your contribution!