Hi! Here is a new charm for IBM Websphere Base for review.

Hi Uma! Thanks for the submission, but I'm struggling to find the Base product. I see this in README.md:

For WAS 8.5.5.0 Download refer : http://www-01.ibm.com/support/docview.wss?uid=swg27038625 for WAS 8.5.5.4 fixpack download refer : http://www-01.ibm.com/support/docview.wss?uid=swg24038539 link

I see 19 zip files related to the linux product, but needs a passport advantage membership to access. Would it be possible for you to assemble the 19 files and host that somewhere for me to grab and test?

In addition to Kevin's comment, I wanted to note a few things I found on the charm:

* The was_url and im_file_name config options are only used in the install hook, and thus are immutable (i.e., they don't respond to post-install changes to the values using `juju set`). This goes against our best practice recommendations (https://jujucharms.com/docs/1.20/authors-charm-best-practice) and can create a confusing user experience.

* There is a default value provided for was_url but it does not seem to be publicly accessible. The charm should work with the default config values, if provided. Perhaps this can be addressed while addressing Kevin's request to make the files available for testing. If not, however, the default should probably be removed (changed to blank) and a check be added to require the user to set it before attempting the download.

* The charm uses the template (grey) icon.svg. It will need an actual icon.

* The charm defines a website (http) relation but does not implement it. A small website-relation-changed hook should be added to set the host and port for the relation.

* Is it useful to have im_install_path as a config option, rather than being hard-coded in the charm? If so, perhaps an example of when / why you would want to change it could be added.

* Is it useful to have separate options for was_url and im_file_name? It seems like the user experience might be cleaner if there was just a URL option, with the file name included.

Taken care of these comments and its same as was-base code change.

Hi Kevin,

For WAS 8.5.5.0 Download refer : http://www-01.ibm.com/support/docview.wss?uid=swg27038625 for WAS 8.5.5.4 fixpack download refer : http://www-01.ibm.com/support/docview.wss?uid=swg24038539 link

In this you need to download only below files.

WAS_V8.5.5_1_OF_3.zip
WAS_V8.5.5_2_OF_3.zip
WAS_V8.5.5_3_OF_3.zip
8.5.5-WS-WAS-FP0000004-part1.zip
8.5.5-WS-WAS-FP0000004-part2.zip

Greetings Uma,

Thank you for your contribution to the Juju charm store. We appreciate your efforts to bring the IBM Websphere Liberty base to our charm ecosystem, and welcome the contributions. I've taken some time to review your work on this charm, and I have a few notes:

# Charm proof, Readme, and Metadata

By running `charm proof` I noticed that the hook is not implemented. While this is only an I: level message, the relationship should at bare minimum have a hook that sends the port that websphere is going to be running on. This is a necessary component to the Juju relationship model, as the interface requires 2 datapoints: a private address and a port. When deploying websphere, if I were to add a relationship to HAPROXY as a reverse proxy - it would fail due to this missing data point.

When I opened the README to inspect and investigate the deployment routine, I see a significant block of instructions that calls out to "setting up an apache repository". This leads me to believe that there is an expected manual component to the setup and If i'm understanding it would look like this:

- Setup another server which will warehouse the downloaded binaries
- Preload the server with said binaries
- point this charm at that server, along with a path to find the binaries hosted over an HTTP connection

This breaks another core tenant of the Juju Charm store - charms should have sane defaults and be deployable out of the box with minimal effort from the user. I feel that there are other options available here that have not been fully investigated. I'm going to encourage you to investigate other charms that we have in the charm store that are behind EULA/LoginWalls and evaluate their practices encapsulated in charms - such as the MariaDB Charm, which installs a Community edition by default. When the user decides to upgrade to the Enterprise edition, you set the configuration to accept the EULA, and provide an apt string that contains a unique login/password embedded.

https://jujucharms.com/mariadb/

Cory mentioned a UX with regard to combining the config values into a single string, and parsing that code side in the charm if required - I am +1 to that advice and re-state it here. Reducing the overall complexity of the charm config by being opinionated where you can be is a +1 to our core tenants.

# Charm Code

I see you have included some basic standup tests for the websphere_base charm, which is great! I'd like to see the deployment formation flexed bit more including a reverse proxy to exercise the website relationship, and additionally the config options should be altered and validated as well.

Looking through the code, the installation hook raises a few concerns. None of the downloads are being cryptographically signed. This poses a problem as it's subject to a few potential problems here. The charm could be compromised by a man in the middle attack by not delivering over HTTPS. In addition, the lack of CRC validation - we have no idea if the download completed successfully receiving the intended package.

For each file being downloaded, I highly recommend setting it as a configuration variable for 2 things. The source URL of the downl...

We are taken care of all the review comments apart from testing with reverse proxy for website relationship. Please review the same. While testing WAS with reverse proxy, it's giving connection error to access WAS Console using haproxy's ip address.
We are still investigating on this issue. Once this issue will get fix, we will upload the code.

We have done testing with reverse proxy and uploaded latest code.

Hello Geetha,

Thanks for work done in this charm. We appreciate your efforts in to bring WebSphere to the Juju community!

There are many things are done very well in this charm. It passes `charm proof` without warnings or informational messages!

# README.md
This readme contains multiple spelling errors and markdown formatting problems. Please go to the charm store to see how the markdown renders to HTML https://jujucharms.com/u/ibmcharmers/was-base/trusty This is the “ibmcharmers” personal namespace that is updated when you push code to launchpad. Use this page to see if the markdown renders correctly.

I was unable to follow the readme to successfully deploy this charm on amazon because the virtual machine ran out of disk space. The Juju creates default amazon instances of 1.7GB of RAM and 8GB of root-disk. An installed instance takes over 15GB of root-disk! The readme should include some information about how to use “constraints” to request a larger root disk. Also you might want to add something about the suggested RAM memory to run WebSphere correctly.

The “Creating the repositories in Apache” section could contain more detail and perhaps some examples of a url and what the file structure looks like when the apache is set up correctly. Please consider someone new trying to read these instructions. Can they follow along and set up a correct repository? Would they be able to deploy?

The steps to deploy the charm are missing the `sha_im` configuration option. Again examples of valid commands would be useful here, I do realize that IP addresses change, but some of these values can be given as example:

juju set was-base accept-ibm-im-license=True accept-ibm-websphere-license=True was_url=http://apache-file-server/was-base im_file_name=Install_Mgr_v1.6.2_Lnx_WASv8.5.5.zip sha_im=93e6532d2318d8268313ca62a406a23160306f81

On public cloud deployments (Amazon and Google) the charms are behind security groups or firewalls until the service is exposed. Local deployments have no such restrictions. Please add instructions on how to expose the service to the public at the end of the deployment steps: `juju expose was-base`

The readme instructed the user to browse to http://was-base unit's ip-address:9060/ibm/cinsole (should be console) but that is where the guide ends. The console page asks for a user id, and it seems that any user id will work. This was not clear to me when I first did it and should have some explanation in the readme if there is a way to prevent anyone and everyone from logging in.

# metadata.yaml
This charm only exposes a website relation. The real value of Juju is the ability for charms to relate to each other. The WebSphere products are know for their ability to cluster. I would have expected to see a peers relationship here so if 2 of these servers are deployed they can set up a cluster. Please consider adding a peer relation that sets up WAS clustering, and any other relations that are appropriate.

# config-changed and start hook
When I increased the instance size I was able to get the charm to install but noticed a bug. WAS deployed without opening any ports on Juju. Since I deployed following...

Hi,
Thanks for the review comments. We will be working on the same soon.

Hello Team,

Thank you for your review on this charm.

We have taken care of all the review comments apart from adding peer relation in metadata.yaml. As confirmed by the product team WAS Base product doesn't support clustering.

Thanks!

We have removed reverse proxy testcase as it's giving error message as "503 Service Unavailable" when tried with SSL termination using haproxy charm. We are investigating on this issue. Once this issue will get fix, we will upload the code.

Hello Geetha,

Thanks for updating the charm. Here is my review:

## charm proof

We always run charm proof on charm reviews. Here is the output of the current branch:

I: website-relation-changed not executable
I: install not executable
I: start not executable
I: stop not executable
I: config-changed not executable

These are only informational messages which we are NOT required to fix. Juju makes the hooks executable when copying them to the machine, luckily this is an easy fix: `chmod +x install start stop config-changed website-relation-changed`

## README.md

This README.md is much better, the examples are very helpful to figure out what values to provide. The switch to sftp rather than using curl took me by surprise but was properly documented in the readme. The WAS-BASE binary files were not expanded on brickftp and therfore did not download correctly. Please make sure the sftp site we share has both the power and intel binaries uploaded and expanded.

The first command in the readme (juju set-constraints --service ibm-was-base root-disk=15G) did not work for me. We can not specify a constraint on a service because the service had not been deployed yet. Rather we should issue the constraints on the deploy command. `juju deploy ibm-was-base --constraints root-disk=15G` or change the constraints for all machines 'juju set-constraints root-disk=15G'

## config-changed

The config-changed uses a default user and password (wasadmin) that violates charm policy. “Must not run any network services using default passwords.” https://jujucharms.com/docs/1.25/authors-charm-policy

By looking at the code the “im_install_path” is immutable. After installing IBM IM and WAS base changing this parameter will not put IBM IM in a different location (as a user would expect). My recommendation is to remove the “im_install_path” configuration option, as it does not provide the user any benefit to change the directory of IBM IM.

You should add configuration options for profile options on line http://bazaar.launchpad.net/~ibmcharmers/charms/trusty/ibm-was-base/trunk/view/head:/hooks/config-changed#L425 things like the profile name, path, and admin username and password. Don't forget to change the start and stop hooks to use the configured admin username.

Thanks for your continued work here, but some of these are blocking issues. I am going to put this state to “In progress” while you work on these issues. Please join us in #juju if you have questions or concerns or email the Juju mailing list.

This revision of the charm changed to using sftp to download the IBM software. The code is only checking the cryptographic signature of the IBM IM software and not the WAS software. I am not sure that sftp satisfies the need to cryptographically verify the downloaded software. Someone could set up a different SFTP site and the software the charm downloads may not be what was intended.

Hi Matt,

Thank you for review comments. I have fixed all the issues.

I have given executable permission to all hooks in the hooks directory as suggested. Also I have done README changes, given configuration options for profile name, path, username and password and also removed "im_install_path" config option in config.yaml. I am changing the status to Fix Committed. Kindly request you to have a look at the charm.

Regards,
Geetha S

This charm is marked as "depricated" in favour of the "ibm-was-base". The review process should continue for the "ibm-was-base" not this one.

This ticket also applies to "ibm-was-base". I missed the link to lp:~ibmcharmers/charms/trusty/ibm-was-base/trunk . Apologies.

Hi Geetha,

Thank you for your work on this charm. Here are some issues that may require your attention:

- Building the layer failed for me because (I think) some of the interfaces were not registered in http://interfaces.juju.solutions/ . Can you please make sure all the interfaces and layers used are registered and submitted for review?

- I've seen a number of code style errors. You can spot them by running "make lint" inside the build output directory.

- In the README you are using "juju set" however this has been replaced by "juju config" in Juju 2.0. Could you update the instructions?

- Can you make sure that the tests are running & passing without using any non promulgated charms? We cannot establish the correctness of a charm when using items we do not know they work. If you need to test multiple charms you can submit a bundle for review.

Thank you for your time,
Konstantinos

Moving back to In Progress based on the previous review from Kostas.

Hi,

Thank you for your review comments, We will be working and fix it soon.

Hi Konstantinos,

Thank you for your review comments! I have fixed most of all the above issues.

- I have registered ibm-mq interface in http://interfaces.juju.solutions/, It has missed. Apologies.

- Taken care of all code style errors and fixed it.

- Modified README with "juju config"

- I have tested WAS Base with non promulgated charm like IBM Http Server, It worked fine. But when we test using amulet framework with terms, we are getting error:
"urllib2.HTTPError: HTTP Error 407: Proxy Authentication Required". We are working on this, once this issue will get fix, we will upload the code. I am changing the status to Fix Committed. Kindly have a look at the charm.

Regards,
Geetha S

Thanks for the updates to this charm -- the review has been migrated to the new queue:

https://review.jujucharms.com/reviews/34

The latest ibm-was-base charm has been imported and will be reviewed shortly.

This charm has been released: https://jujucharms.com/ibm-was-base/trusty. Thank you for the contribution!