José,

Thanks for working on adding ssh to the Owncloud charm!  This is my first experience with Owncloud and I am really excited about this charm!

The command “charm proof” passes with no errors!  That is excellent, but I have come to understand that you pay attention to details like that.  

I see you added charmhelpers to generate the selfsigned certificate.  That all looks good to me.

The README.md file was updated, it looks very good but I found a few small errors:

“If you do not know what it is, execute `juju status` to find out the public DNS.”

The command “juju status” does not return a Domain Name Server (DNS).  What you might have meant “... to find out the public domain name of the deployed unit.”

“This password will be used in case you want to deo a setup of Shared Instances. If not provided, it will be randomly generated when a DB relation is joined.”
The word “do” is misspelled here.

The readme file asserts that the charm will not be set up without setting the domain configuration option, which is fine.  I tested the server address before setting domain and got the default page:

  It works!
  This is the default web page for this server.
  The web server software is running but no content has been added, yet.

I understand this is the default apache page has not been disabled at this point.  What would be really awesome if the default page could provide the user with instructions on what needs to be done.  The hook could write a small index.html to the www root that tells the user what is needed before the charm will set up and configure.  Something like:

  The owncloud service is not fully configured.  Set the domain value in Juju to configure owncloud.

Bugs found unrelated to this merge proposal:

The admin user name and password are immutable, even after adding a mysql database.  I realize this is not a bug introduced by this merge proposal so I created a bug here:
https://bugs.launchpad.net/charms/+source/owncloud/+bug/1315047

We can track that topic separately on that bug report.

Removing the database relation generates the following error:

[1044] SQLSTATE[42000] [1044] Access denied for user 'paethaicieniaju'@'%' to database 'owncloud'

I opened a bug here:
https://bugs.launchpad.net/charms/+source/owncloud/+bug/1315091

I am really excited to see that the charm included tests!  That is outstanding not many merge propoals have tests.  However the test that I ran did not work.

$ ./tests/100_deploy.test 
Traceback (most recent call last):
  File "./tests/100_deploy.test", line 25, in <module>
    domain = d.sentry.unit['haproxy/0'].info['public-address']
AttributeError: 'NoneType' object has no attribute 'unit'

I took a look at the test and it looks like an error in the amulet test code.  The sentry objects are not created until after d.setup(...).  You should move that domain and d.configure() to after the d.setup(...) call.

Thank you so much for your submission! Based on this test failure I have moved this bug to Incomplete.  Once you have addressed the issues above either with code changes or comments, and wish to have another review, simply move the bug back to either "New" or "Fix Committed" to have it added back to the review queue.