This is a new charm for IBM Websphere ND for review.

Bug #1446999 reported by Uma
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Juju Charms Collection
Triaged
Undecided
Unassigned

Bug Description

Hello Team,

Here is a new charm for IBM WAS ND for review.

To test the IBM WAS ND charm, we need the IBM WAS ND DM and IBM WAS ND Node charms as well. As per the design changes, the WAS ND charm has been split into three charms:
IBM WAS ND
IBM WAS ND DM
IBM WAS ND Node.

We need all three charms to be tested to have a WAS ND dynamic cluster. Here we are using IBM WAS ND charm as a common layer for IBM WAS ND DM and IBM WAS ND Node charms.

Deployable ibm-ibm-was-nd layer charm can be found in the below repository.
Repo : https://code.launchpad.net/~ibmcharmers/charms/trusty/ibm-was-nd/trunk

And, its source code can be found in the below repository
Repo : https://code.launchpad.net/~ibmcharmers/charms/trusty/layer-ibm-was-nd/trunk

The charm has been pushed into charm store as well
branch : cs:~ibmcharmers/trusty/ibm-was-nd-node-3

Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

Hi Uma! Thanks for the submission! Similar to the WAS base comment I made recently, I'm struggling to find the ND product. I see this in README.md:

For WAS 8.5.5.0 Download refer : http://www-01.ibm.com/support/knowledgecenter/SSAW578.5.5/com.ibm.websphere.nd.doc/ae/welcomend.html for WAS 8.5.5.4 fixpack download refer : http://www-01.ibm.com/support/docview.wss?uid=swg24038539 link

That 1st link takes me to a "Topic not found page". Can you point me to some other place where I can download WAS ND for linux?

Revision history for this message
Cory Johns (johnsca) wrote :

My comments from the was-base charm also apply to was-nd:

* 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.

Revision history for this message
Uma (umasv1987) wrote :

Hi Johns,
                     Thanks for reviewing the code and I have taken care of icon.svg template and website relation changed hook comments.

For the first comment regarding was-url and im_file_name ideally we should change these parameters and these are being used only once to download binaries from repository thats why its done in install hook rather than config-changed hook,.

Now i m not giving any default value for was_url and this needs to be entered from the user and added to validation to check the same .

Given option for im_install_path in config.yaml , by this value user can choose his installation path

We have given diff option for was_url and im_file_name because user can have any version of IM with 1.6 and above and hence its file name will be varying. If i append im_file_name with was_url , we will be not able to download other WAS binaries. so given diff option for both.

I have taken care of your comments ,Please review and let me know your comments

Revision history for this message
Uma (umasv1987) wrote :

Hi Kevin,
                  Please find the link for WAS ND Binaries download : http://www-01.ibm.com/support/docview.wss?uid=swg27038624

It redirects to passport advantage site and you need to download following files for WAS ND deployment.

WASND_v8.5.5_1of3.zip
WASND_v8.5.5_2of3.zip
WASND_v8.5.5_3of3.zip

8.5.5-WS-WAS-FP0000004-part1.zip
8.5.5-WS-WAS-FP0000004-part2.zip

You canuse any IM version 1.6 and above and change the im_file_name option accordingly

Revision history for this message
Matt Bruzek (mbruzek) wrote :
Download full text (5.1 KiB)

Hello Uma,

Welcome to the Juju community! WebSphere is a large complex product and I appreciate the effort it took to bring WebSphere Network Deployment into a charm!

When reviewing a charm I generally look for Best Practices and Charm Store Policy
https://jujucharms.com/docs/devel/authors-charm-best-practice
https://jujucharms.com/docs/devel/authors-charm-policy

Here what I found on my review:

# config.yaml
The configuration values look to have defaults and the correct values. The was_nd_url, and im_install_path description could both have better descriptions with examples of valid values or just more description of what needs to be in these configuration values. For example I did not know what path the url was expecting. Looking at the code it appears that names are appended to the end of a $was_repository, so the README or config.yaml could point this out as an example.

# copyright
The copyright doesn't need to be Canonical, it just needs to be an open source license. We will accept “Canonical Ltd” but this could be IBM.

# icon.svg
The icon looks fine thanks for updating that one!

# metadata.yaml
The metadata suffers from other reviews that I have done, the maintainer field must be “Juju Support Team <email address hidden>” or something similar. I see the metadata defines a “website” relation but see no hooks for this relation! The website relation is very easy to implement you only need to set the hostname=$private-address and port=$ in a website-relation-joined hook.

# README.md
The Overview section is very brief and could contain more detail about what WebSphere Network Deployment is and why someone would use it.

The “Creating the repositories” section was confusing. Like other IBM charms this one requires a separate download and configuration but the steps were very vague and it was not clear to me that these steps must be done on another server or my own system. Please note that markdown interprets greater than (>) and less than (<) signs so "http://<server-name>/debs/" gets incorrectly rendered to “http:///debs/” in the HTML result. This section needs more detail and description of exactly what the user needs to do, with links to trial versions if possible.

The “Usage” section could again have more detail. Upload to where, and what directory? Could you give examples of directory names or file names. Since this is a manual process this is prone to more errors so the instructions must be as clear as possible. I did not find links on where to find the IBM Installation Manager. Each charm must be self contained and if you need IBM IM for this charm the README should indicate where to get it.

The “Configuration” section should have more description or examples of the configuration options as was suggested in the config.yaml review.

The “Contact Information” section should have links to the WebSphere forums or bug trackers, or Information Centers where users can get more information about WebSphere Network Deployment software.

# hooks
Many of the hooks mix spaces and tabs which is not a best practice for Bash charms. Please choose one and remain consistent throughout all the hooks.

# install
As Cory pointe...

Read more...

Changed in charms:
status: New → In Progress
Geetha S (geethas1)
Changed in charms:
status: In Progress → Fix Committed
Revision history for this message
Matt Bruzek (mbruzek) wrote :
Download full text (3.7 KiB)

Hello Geetha,

Thank you for the new submission of the WebSphere Application Server Network Deployment (WAS ND). It looks like you fixed many of the previous review comments. But some of the config-changed comments were not addressed (see that section).

Juju users still have to download and host the IBM software to deploy this charm. I was able to find the software and download it and host it on an HTTP server so I could deploy. However this is a lot of software 5.4GB and the process of hosting it is error prone so users need all the help they can get.

When I tried to deploy this charm the instance that I used ran out of disk space! The default instances that Juju creates are 8GB of “root-disk” and a fully installed was-nd charm needs over 16GB of root-disk. Juju allows users to specify “root-disk” constraints either on the deploy command or the “juju set-constraints” command itself to request bigger instance. Running out of disk space on the default instance is a big blocker and must be explicitly called out in the README.md as clearly as possible!

# config-changed
The use of “set -e” is a best practice and the code uses it! This means however that if any command returns a non-zero return code the hook will stop in error. So checking the return code from previous commands will not work if they are anything but zero. There are alternate ways to capture the return code such as assigning the return code to a variable and using the command from within an if statement.

# README.md
This document needs a lot more information. There are markdown errors in the file things do not render in HTML as they appear they should by looking at the markdown source. Please check the source against what is rendered on the Charm Store web site: https://jujucharms.com/u/ibmcharmers/was-nd/trusty/4

There are 3 major things that are not clear from reading the current README.md file. New users who don't know WAS ND will need more details on how to get the IBM files (1), how to setup the web server (2), and that they need a more root-disk to deploy this charm (3). These things were not clear to me when I re-read the document. I created a Merge Proposal to update the README with the areas that needed more information.
https://code.launchpad.net/~mbruzek/charms/trusty/was-nd/readme-update/+merge/269560
Please continue to edit the README document and add to more description in areas that may still be unclear.

# start
When I installed this on AWS the ports were not opened so I do not think this hook was correctly run. So I took a look at this hook and I have a few comments. The “set -e” is a best practice that is not used in this hook. All the hooks run as root so you do not need sudo in any of your commands! I don't think checking the return code of the netstat command is a good way to see if the server is running. I would use the “./serverStatus.sh server1” command instead. If that script can return a non-zero result there are ways to capture that return code and not exit the hook in error.

Once I properly configured the HTTP server, and increase the root-disk past 16GB I was able to get was-nd charm to deploy correctly! I am still concern...

Read more...

Changed in charms:
status: Fix Committed → Incomplete
status: Incomplete → In Progress
Revision history for this message
Matt Bruzek (mbruzek) wrote :

I spoke with a colleague that works on WebSphere and he told me that the strengths of the WAS ND product was the clustering aspect. I would highly encourage you to look at the "peer relation" that is often used to cluster services. https://jujucharms.com/docs/stable/authors-relations-in-depth

For an example of how to use the peer charm please look at the tomcat charm or other charms that contain the peer relation.

Revision history for this message
Geetha S (geethas1) wrote :

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

Geetha S (geethas1)
description: updated
Geetha S (geethas1)
Changed in charms:
status: In Progress → Fix Committed
Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

This review has been migrated to the new queue:

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

The latest ibm-was-nd charm has been imported and should be reviewed soon.

Changed in charms:
status: Fix Committed → Triaged
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.