Comment 6 for bug 1291783

Marga,

Thank you for the revisions to the VSM charm. I've taken the opportunity to re-review the submission and I'm happy to report the issues I had with deploying are fully resolved. Thank you for your patience while I resolved that bit, I have a few notes about my findings...

I like how there has been shared functionality extracted into a common library. This is a practice used in many charms and adds a nice bit of abstraction to the charm.

The hooks are missing the set -e flag at the top. This is a requirement for inclusion to the charm store to prevent charms from continuing after error. I ran into a condition while deploying where I had an incorrect n1kv-source configuration option, and the unit continued configuring which took over the networking and rendered the server unable to communicate as it changed the networking configuration. With a set -e flag, it would halt upon not being able to find the n1kv-iso package.

I also noticed from the output from charm proof that the stoop hook is not executeable, but in the grand scheme that is a knitpick.

I am very pleased with this charm, the overall formatting, and layout. If you can add the set -e flag to the top of the hooks I would have no problem ack'ing this charm for promulgation.

Thanks again for the hard work and submission of the N1KV VSM charm. I'm going to move the status of the bug to "in progress" pending the requested modification. After you have added the -e flag, if you are ready for another review, simply change the status of the bug to "new" or "fix committed" and someone will be along shortly to review your work.

If you have any additional questions concerns, post back to this bug, or join us on irc.freenode.net in #juju, or email the mailing list at <email address hidden>

Thanks again, and all the best!