New charm - Apache Allura

[Cory Johns]

Ready for review. Allura doesn't currently have an icon, but it is being discussed. If an icon is chosen, I will update the charm with it.

https://code.launchpad.net/~johnsca/charms/precise/apache-allura/refactoring-with-tests

[Charles Butler (lazypower)]

Greetings Cory,

Excellent work on a first cut charm! We appreciate the submission. Allura looks like a genuinely interesting project as a project incubator. I've given a cursory review and came up with some notes.

You have tests! This was exciting, as the evaluation process was quickly validated through the amulet tests. Great work on including a bundle test, and unit tests.

Charm proof returns a single warning, but can safely be ignored, as you're working on fetching an icon.

The Readme is a bit light on detail. Ideally we'd like to see where to find additional upstream information, file bugs, and how to contact the maintainer of the charm at the end of the readme. A good litmus would be, if you have charm-tools installed to charm add readme and port what you have into that example template, and fill out the missing sections.

Unfortunately I am unable to ack the charm in its current state as it provides a default administrative user with a password. An accepted method to bypass this, would be to add a password configuration variable and have the charm no-op until a password is provided.

Otherwise I see no blatantly obvious blockers.

Thanks again for the submission. I'm going to set the status of the bug to "incomplete" pending the revision and when you're ready for another review simply change the bug status to "Fix Committed".

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>

[José Antonio Rey (jose)]

Hey, Cory!

Thanks for submitting the Allura charm! Looks like it will be a great addition to the Charm Store!

I have been taking a look at the charm just a couple minutes ago, and here's some things I found:

 * On the config-changed and start hooks log, the following errors/warnings are listed:
    2014-06-14 02:36:33 INFO install Warning: there was a problem reading the certificate file /etc/ssl/certs/T??B??TAK_UEKAE_K??k_Sertifika_Hizmet_Sa??lay??c??s??_-_S??r??m_3.pem. Message:
    2014-06-14 02:36:33 INFO install /etc/ssl/certs/T??B??TAK_UEKAE_K??k_Sertifika_Hizmet_Sa??lay??c??s??_-_S??r??m_3.pem (No such file or directory)
    2014-06-14 02:36:41 INFO config-changed taskd: unrecognized service
    2014-06-14 02:36:41 INFO config-changed allura: unrecognized service
    2014-06-14 02:36:42 INFO start taskd: unrecognized service
    2014-06-14 02:36:42 INFO start allura: unrecognized service
    2014-06-14 02:44:24 INFO mongodb-relation-changed stop: Unknown instance:
    I think this can be solved with sentinel files that tell if a service has been started or not, all the cases with the exception of the cert one.
 * When I first deployed the service, it got stuck in an error state for mongodb-relation-changed, and 'resolved --retry' did the trick. The traceback I got is the following:
    2014-06-14 02:47:41 INFO mongodb-relation-changed AttributeError: 'module' object has no attribute 'MongoClient'
    It would be worth to investigate this.
 * I see that the README is now covering most of the points. Good work!
 * For the password, it does not include a default password anymore. This is good, as it's one step closer. Still, I see that the way of setting the password is by doing `juju run` and running a script. I don't think this is the best way for a user to set the password. They would expect something like `juju set password='myin$ecur3p4ssw0rd'` instead. Maybe what you can do here is to state the values in a hidden file, then compare those with the current config-get values, and if they changed then execute it in config-changed with the values given by the user in `juju set`.

Thanks again for your efforts on the charm, I really look forward to seeing it on the store. If you have any questions or I haven't been too clear in any of the points feel free to ping me directly on IRC (I'm jose), contact the team in #juju on irc.freenode.net, or email the mailing list at <email address hidden>.

[Cory Johns (johnsca)]

José,

I made some changes to suppress the spurious "unrecognized service" messages from the log. Per the discussion on IRC (http://irclogs.ubuntu.com/2014/07/02/%23juju.html#t16:35) I'm leaving the password management as a run script for now. I'll let you and Charles duke it out. :)

Regarding the mongodb-relation-changed error, I was entirely unable to reproduce it, nor even see how it could have occurred, unless the pymongo library was only partially installed, which shouldn't have fixed itself during a retry. If you run into this again, or can provide any more info, I can try to look into it again.

[Cory Johns (johnsca)]

José,

Thanks to your help today, I was able to resolve the mongodb-relation-changed error on precise. As I mentioned, the charm works fine on trusty as well, so if a charmer is willing to promote this for both precise and trusty, I would be entirely alright with that. :)

Thanks again for your help.

[José Antonio Rey (jose)]

Hey Cory!

Thanks again for your submission of the Allura charm for the Charm Store! Now that the fixes for that little error have been pushed, it's time for me to do a review of the charm. Here it goes.

# charm proof

`charm proof` only returns a warning for not having an icon. I was not able to find an icon for Allura, so I believe it is all right to have no icon for now. However, if an icon is created in the future, it would be awesome to update the charm accordingly.

# Deployment

I did a successful deployment by following the instructions found on the README.md file. That's an awesome thing!

# Configuration

The charm currently has three configuration options. I got to play with the three of them.

The revision option did well, as I can tell from the logs.

On the other hand, the port option currently has a problem. I set it to use port 80 a couple seconds after I deployed it to my environment, and still it opened port 8080. I went ahead and tried to connect to port 8080 but nothing was there. Looks like somehow it's changed on the background, but the port is not opened in Juju, meaning the security group does not allow me to connect to the service. When I switched to port 8080 everything was running correctly. Tried this back and forth and port 80 wasn't opened.

Finally, changing the domain name did not make the service error, but I could not test it further because when I tried sending myself a verification email, the email never went through. Probably because port 25 is not opened on the service? Just an assumption. It would be worth to take a look at that.

# Relations

I related the service to mongodb in order for it to function correctly. However, when I destroyed the relation between mongodb and allura, everything seemed to be running normally. Is this expected? Would Allura work without a mongodb relation? Otherwise, it would be nice to take the service down and set up a page saying something like "Please relate mongodb in order for Allura to work".

# Knitpicks

I found the README.md file pretty complete - had most of the things I expected. However, if there could be detailed information about the configuration options inside the README file it would be awesome - I had to go into the config.yaml file in order to check which options were available.

Finally, I have a question. On the README you suggest using juju run against one unit to change the password. If I do this, will the password be changed in all units? Is this something on the database, and will not make it a problem when scaling the service?

I find the charm almost ready for the store, but because the port opened in Juju does not change and emails are not being sent out as expected, I cannot fully give it a +1 at this moment. I am marking this charm as Incomplete, but please mark it as New or Fix Committed as soon as you are ready for another review. As always, if you have any doubts or questions, feel free to send us an email at <email address hidden>, or find us on #juju on irc.freenode.net - my nick is jose in there.