Juju Charms Collection

Bug #1478783
Comment #1

Comment 1 for bug 1478783

Revision history for this message

Matt Bruzek (mbruzek) wrote on 2015-09-04:

# IBM MobileFirstServer review

Thank you for the submission of the IBM MobileFirstServer. I am sorry it took so long for someone to look at this charm. As always I am using the Charm Store Policy and Best Practices documents for reference when reviewing charms: https://jujucharms.com/docs/stable/authors-charm-policy https://jujucharms.com/docs/stable/authors-charm-best-practice

Here is my review.

## charm-proof

$ charm proof
I: all charms should provide at least one thing
I: missing recommended hook start

I would have expected this charm to provide something because it is a “server” in the end the deployment succeeded but I was not able to view the web pages, so it is hard for me to tell. Please consider the peer relation, if this software can be set up in a cluster mode.

## README.md
The readme contains many misspelled words and formatting errors that do not convert to HTML correctly. Please refer to the Markdown cheat for formatting tips: https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet If you want to see how the HTML looks please view how the readme renders on the personal namespace section of the charm store: https://jujucharms.com/u/ibmcharmers/mobilefirst-server/trusty Once the code is in bzr you can see the new revision in the charm store after a 30 minute update delay. Please note that code blocks must be indented in multiples of 4 for code blocks. In some cases 7 spaces are used, in other cases 2 spaces are used. Alternately you can use three back-ticks for code blocks (``` code here ```). Markdown has special handling for anything between the less than (<) and greater than (>) signs so “juju deploy --to <machine number> db2” converts to the following HTML “juju deploy --to db2” which is an invalid command. Revisit everything with the less than (<) and greater than (>) symbols to ensure they convert to HTML correctly.

The readme needs more specific steps in the setup of the apache2 server, and how to host the file, with examples of paths and file names so the Juju use has a chance to get this correct. As someone reading this charm for the first time I would really want more information about the MobileFirst product, what it does and what I can do with it after deployment.

The deploy instructions are incomplete if I followed those instructions db2 and websphere-liberty would not install because the licenses were not accpted. To install websphere-liberty and db2 users have to accept licenses. The readme doesn't have to cover all the details, but the readme should mention that other steps are needed to deploy the other software and the users should refer to their readme files.

## config.yaml
The configuration file looks good, I would expect to see better descriptions including file name examples in the description. For example if the default for a value is empty string, and the config-changed hook looking for predefined package names, the configuration description should provide those names so the users know what is a valid value.

## install
It looks like you are missing the -y on the first two apt-get install lines. Those could cause a problem in the future if they require other packages the user will be prompted and that command will not complete without a -y.

## config-changed
There are a mixture of spaces and tabs in the remove_unaccepted_im_software function.

# Performance suggestion
Juju makes it easy for users to request different instance sizes from the cloud providers. The readme should include what kind of constraints work best for this deployment. The default AWS instance has the following constraints root-disk=8192M, and mem=1740M. Since you put all three services on the same instance this deployment used over 10GB of root-disk (according to df -h). That would not deploy on a default instance! Also with three services running on the same server I suspect that 1.7GB of RAM is not enough.

One would also have to question why the readme co-locates all three services on the same node (--to <machine number>). The power of the cloud is that each service can be deployed to a different machine for fault tolerance and no single point of failure. The cloud makes it easy to deploy as many Vms as are needed. The charms connect using relations already. Why do the services have to be on the same server? I would want to see that these charms work on their own servers smaller servers and they can link up via relations rather than being deployed to one really big server.

# Deployment failure
I deployed the three services and related them without any errors in Juju. When I tried to connect to https://mobilefirst-server-ip:9443/worklightconsole I was prompted for the username and password. I guessed admin/admin and that worked. Two problems with that, the Charm Store Policy does not allow default passwords, and the readme did not contain instructions on how to find the username password information. I would expect to see the username and password configuration options and if the password is empty generate a random password and store it in a file somewhere a user could find it on the server. I was not able to connect to http://mobilefirst-server-ip:9081/appcenterconsole with admin/admin and the readme did not tell me how to log in.

Once I got in to the worklightconsole, I saw the following message: “No runtime environment deployed in this server.” I checked the Juju logs and found no obvious error messages. I will attach the logs to this bug so you can possibly diagnose this problem.

Thanks again for this submission. I am going to put this charm back into “In Progress” while you work on these issues. Please feel free to join us in #juju on chat.freenode.net or email <email address hidden> with questions.

# IBM MobileFirstServer review

Thank you for the submission of the IBM MobileFirstServer.  I am sorry it took so long for someone to look at this charm.  As always I am using the Charm Store Policy and Best Practices documents for reference when reviewing charms: https://jujucharms.com/docs/stable/authors-charm-policy  https://jujucharms.com/docs/stable/authors-charm-best-practice

Here is my review.

## charm-proof

$ charm proof
I: all charms should provide at least one thing
I: missing recommended hook start

I would have expected this charm to provide something because it is a “server” in the end the deployment succeeded but I was not able to view the web pages, so it is hard for me to tell.  Please consider the peer relation, if this software can be set up in a cluster mode.

## README.md
The readme contains many misspelled words and formatting errors that do not convert to HTML correctly.  Please refer to the Markdown cheat for formatting tips:  https://github.com/adam-p/markdown-here/wiki/Markdown-Cheatsheet  If you want to see how the HTML looks please view how the readme renders on the personal namespace section of the charm store:  https://jujucharms.com/u/ibmcharmers/mobilefirst-server/trusty  Once the code is in bzr you can see the new revision in the charm store after a 30 minute update delay.  Please note that code blocks must be indented in multiples of 4 for code blocks.  In some cases 7 spaces are used, in other cases 2 spaces are used.  Alternately you can use three back-ticks for code blocks (``` code here ```).  Markdown has special handling for anything between the less than (<) and greater than (>) signs so “juju deploy --to <machine number> db2” converts to the following HTML “juju deploy --to db2” which is an invalid command.  Revisit everything with the less than (<) and greater than (>) symbols to ensure they convert to HTML correctly.

The readme needs more specific steps in the setup of the apache2 server, and how to host the file, with examples of paths and file names so the Juju use has a chance to get this correct.  As someone reading this charm for the first time I would really want more information about the MobileFirst product, what it does and what I can do with it after deployment.

The deploy instructions are incomplete if I followed those instructions db2 and websphere-liberty would not install because the licenses were not accpted.  To install websphere-liberty and db2 users have to accept licenses.  The readme doesn't have to cover all the details, but the readme should mention that other steps are needed to deploy the other software and the users should refer to their readme files.

## config.yaml
The configuration file looks good, I would expect to see better descriptions including file name examples in the description.  For example if the default for a value is empty string, and the config-changed hook looking for predefined package names, the configuration description should provide those names so the users know what is a valid value.

## install 
It looks like you are missing the -y on the first two apt-get install lines.  Those could cause a problem in the future if they require other packages the user will be prompted and that command will not complete without a -y.

## config-changed
There are a mixture of spaces and tabs in the remove_unaccepted_im_software function.

# Performance suggestion
Juju makes it easy for users to request different instance sizes from the cloud providers. The readme should include what kind of constraints work best for this deployment. The default AWS instance has the following constraints root-disk=8192M, and mem=1740M. Since you put all three services on the same instance this deployment used over 10GB of root-disk (according to df -h). That would not deploy on a default instance!  Also with three services running on the same server I suspect that 1.7GB of RAM is not enough.

One would also have to question why the readme co-locates  all three services on the same node (--to <machine number>).  The power of the cloud is that each service can be deployed to a different machine for fault tolerance and no single point of failure.  The cloud makes it easy to deploy as many Vms as are needed.  The charms connect using relations already.  Why do the services have to be on the same server?  I would want to see that these charms work on their own servers smaller servers and they can link up via relations rather than being deployed to one really big server.

# Deployment failure
I deployed the three services and related them without any errors in Juju.  When I tried to connect to https://mobilefirst-server-ip:9443/worklightconsole I was prompted for the username and password.  I guessed admin/admin and that worked.  Two problems with that, the Charm Store Policy does not allow default passwords, and the readme did not contain instructions on how to find the username password information.  I would expect to see the username and password configuration options and if the password is empty generate a random password and store it in a file somewhere a user could find it on the server.  I was not able to connect to http://mobilefirst-server-ip:9081/appcenterconsole with admin/admin and the readme did not tell me how to log in.

Once I got in to the worklightconsole, I saw the following message: “No runtime environment deployed in this server.”  I checked the Juju logs and found no obvious error messages. I will attach the logs to this bug so you can possibly diagnose this problem.

Thanks again for this submission.  I am going to put this charm back into “In Progress” while you work on these issues.  Please feel free to join us in #juju on chat.freenode.net or email juju@lists.ubuntu.com with questions.