This is a new charm for IBM Platform LSF Master for review

[shilpa]

Hello Team,

Here is a new charm for IBM Platform LSF Master for review.

To test the IBM Platform LSF Master, we need the IBM Platform LSF Storage charm as well. As per the design changes, the Platform LSF charm has been split into three charms:
IBM Platform LSF Storage
IBM Platform LSF Master
IBM Platform LSF Server.

We need all three charms to be tested to have a functional LSF Cluster.

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

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

The charm has been pushed into charm store as well
branch : cs:~ibmcharmers/trusty/ibm-platform-lsf-master-2

Thanks

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/4014/

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/4299/

[shilpa (shilkaul)]

Hi Team,

Modified the LSF Master charm README as it depends on IBM Platform LSF Storage charm.

The Storage Charm is updated with changes for Resources and Terms.
The IBM Platform LSF Master charm needs to be reviewed and tested along with IBM Platform LSF Storage and IBM Platform LSF Server.

The updated charm has been pushed into charm and below is the url:
branch : cs:~ibmcharmers/trusty/ibm-platform-lsf-master-4

Thanks,
Shilpa

[Pen Gale (pengale)]

This charm fails to build for me because it can't find interface:platformmaster. It looks like the source code for said interface is intended to live at https://code.launchpad.net/charms/+source/interface-ibm-platformmaster, but there is no code there at present.

Where is the best place to get the interface:platformmaster code?

Thank you,
~ petevg

[Pen Gale (pengale)]

Please disregard my previous comment. I found the correct link for the platformmaster code.

[Pen Gale (pengale)]

Thank you for your work on this. I was able to manually deploy this, and the related charms, and follow the steps in the README to check to make sure that they deployed. charm proof passes, and this charm has an icon, maintainer, and other necessary bits.

Note that, if you include the flag "reset: false" in your tests.yaml, I'd be able to run tests against the charms that I manually deployed, and verify that tests pass.

I am +1 on this charm.

[Kevin W Monroe (kwmonroe)]

This review has been migrated to: https://review.jujucharms.com/reviews/17

[Cory Johns (johnsca)]

This review was already +1'd, so I suggest we move finalize the review on this site rather than moving.

Before acting on the +1, however, I wanted to confirm that this charm was not missing a terms acceptance. The IBM Platform LSF Storage charm requires the lsf-platform/1 license to be accepted and it seems like this the Master and Server components should as well but do not. I wanted to confirm whether this is an oversight and should be corrected, or if we should move forward with this charm as-is.

Thank you.

[shilpa (shilkaul)]

The actual LSF installation takes place in LSF Storage charm, that's why the terms are provided for LSF Storage charm only. LSF Storage Charm acts as NSF Server for the LSF Master and LSF Server charm.

When a relation is added between LSF Storage and LSF Master/Server, then the LSF installation and configuration files are shared to LSF Master/Server charm. As the actual installation of product does not take place in LSF Master or Server charm, that's why not provided terms for them.

[Cory Johns (johnsca)]

I was informed that the license is only applicable to the LSF Storage charm.

I was able to deploy and verify these charms according to the README. However, I noticed a few items in the code that I think should be addressed.

First, you are using the ibm-base layer but not any of the features it provides (it seems they've been supplanted by Juju 2.0 features such as terms and resources). When the resources are not attached properly, the layers from ibm-base, apt in particular, hide the status messages that you provide to the user and make it hard to tell why the charm is not working. I recommend replacing 'layer:ibm-base' with 'layer:basic' in layer.yaml. I tested with this and it works as expected and results in a much better user experience.

Finally, I noticed that you use "exit" in your bash reactive handlers. While this seems to work for the moment, it is not a good approach for reactive charms because it can cause handlers to be skipped in an unpredictable fashion, leading to hard to diagnose bugs. I believe your team has hit this before on other charms. From within a reactive handler (a function decorated with @when or the like), you can just replace "exit" with "return". For a function called by a reactive handler, it becomes a bit harder to surface up the fact that you wish to abort, but you can do something like:

    function foo() {
        return 1
    }

    @when 'state'
    function handler() {
        foo || return
    }

[Kevin W Monroe (kwmonroe)]

Moving this back to In Progress awaiting consideration of Cory's suggestions in comment #10.

[shilpa (shilkaul)]

Hi,

Have re-worked on the charm and incorporated the suggestions given by Cory. Putting it to Fix Committed for further review.

Thanks

[Matt Bruzek (mbruzek)]

We are tracking ibm-platform-lsf-server in the new review queue: https://review.jujucharms.com/reviews/18

[Matt Bruzek (mbruzek)]

We are tracking ibm-platform-lsf-master in the new review queue: https://review.jujucharms.com/reviews/17