New Charm: Dell OpenManage Server Administrator (OMSA)

Bug #1325700 reported by Kent Baxley
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Juju Charms Collection
Fix Committed
Undecided
Mark W Wenning

Bug Description

Hi All,

I'm in the process of putting together a new subordinate charm for Dell's OpenManage Server Administrator tools (OMSA).

(OMSA) provides a one-to-one systems management solution from either:

1) an integrated, web browser-based GUI

and/or

2) a command line interface (CLI) through the operating system.

OMSA is designed to work on PowerEdge "M", "R", and "T" series systems, and I've tested with the 11G and 12G line, which are the most recent models and used by most of Dell's customers these days.

The charm works most effectively with a Dell server that has been registered with MAAS and a service has been deployed to that system via Juju.

I chose a subordinate charm since it didn't make a lot of sense to have it as a stand-alone one.

The mechanism right now is pretty simple...once the relation is established between the openmanage charm and the machine running a given service, the Dell linux repository is added and the suite of packages is installed. After that, the dataeng services for OMSA are started.

If the user wishes to fire up the web interface, there are instructions in the README.md on how to do that.

The user has an option to configure which set of OMSA packages to install. By default 'srvadmin-all' is used, which installs everything. The user can supply a space-separated list of packages if they want to pare it down a bit.

I currently am only having the charm install the latest version of OMSA for Debian / Ubuntu.

I would welcome all reviews and advice on how to make this charm better.

Thanks!

Revision history for this message
José Antonio Rey (jose) wrote :

Hello, Kent!

Thanks for submitting the OMSA charm to the Charm Store!

I have taken a quick look at the charm and here are some things:

 * `charm proof` returns no errors. Hooray! It would be nice if you could do 80-line wrapping on the README file, but apart from that everything on the structure seems good.
 * I have checked the copyright file and it looks like it is an EULA. According to the Charm Store policy <https://juju.ubuntu.com/docs/authors-charm-policy.html>, all charms in the store need to be licensed under a free license. It is fine if the components the charm is deploying are under an EULA, but the charm itself (hooks, for example) needs to be open-sourced.

With that being a blocker I am marking this review as Incomplete. Please, make sure to mark it as New or Fix Committed once it has been taken a look at. If you need a hand with this, make sure to contact us at #juju on irc.freenode.net, or send us an email to <email address hidden>. Thanks again for your efforts on the OMSA charm!

José Antonio Rey (jose)
Changed in charms:
status: New → Incomplete
Revision history for this message
Kent Baxley (kentb) wrote :

Thanks, Jose.

I made changes to the charm's copyright file, cleaned up the word-wrapping on both the copyright and readme files so they're hopefully a little easier to read.

If the copyright needs more work, please let me know. I added a gplv3 license to all included files and then added an "OMSA" section for the Dell pieces that get installed when the charm is activated.

Please let me know if anything else needs adjusting. Thanks!

Changed in charms:
status: Incomplete → New
Revision history for this message
Charles Butler (lazypower) wrote :

Greetings Kent!

Thank you so much for this excellent charm submission, the Dell Open Manage software charm is sure to be a great utility to have in any admins juju tool belt. I've taken some time to review the charm as it is and I have the following notes.

When charmers review the charm, we use a utility charm-proof from the charm-tools package to get basic linting jobs out of the way first, and during the charm proofing phase, it appears that the 80 column line breaks have broken your metadata.yaml and your config.yaml. When you do multi-line continuations its prudent to start them with a bar character and content with a 4 space indentation (not tab), so the YAML interpreter is aware of a multi-line value that may or may not contain carriage returns.

for example,

description : |
    This deploys my super awesome dell manager service, which assists
    managing a dell power edge (m)(t)and(n) server.

I corrected these items to continue with the review, but a subsequent fix commit will be required prior to a re-review.

Looking at the upgrade-charm hook, there's nothing done here, so its safe to remove this hook file as by default juju skips hooks when no hook file is present.

Otherwise the charm code is very straight forward and easy to digest. Without having dell hardware to test the charm itself, I am not confident in promoting this charm to the store as a recommended charm. THe charm can however live on in a personal namespace by pushing it to a /trunk branch in launchpad. see step 8 of the submitting a new charm in the documentation here: https://juju.ubuntu.com/docs/authors-charm-store.html#submitting-a-new-charm

Thank you again for the continued improvement of the Open Manage charm, and I look forward to a re-review when the time comes that we can test the charm properly.

When you have changes/fixes for the upstream charm in the store, open a MergeProposal against your charm at lp:~charmers/charms/<series>/<charm>/trunk, assign charmers and someone will be along shortly to review your work.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

All the best!

Changed in charms:
status: New → Incomplete
Revision history for this message
Charles Butler (lazypower) wrote :

I apologize for the addtional spam, the second to last paragraph should be ignored - as you simply need to change the status of this bug to "new" or "Fix committed" and someone will be along shortly to review your work. There is no need to open a MP against an existing charm.

Revision history for this message
Kent Baxley (kentb) wrote :

I think I got the line-break requests taken care of. Please review. I also have removed the upgrade-charm hook since it isn't needed.

Changed in charms:
status: Incomplete → Fix Committed
Revision history for this message
Charles Butler (lazypower) wrote :

Kent,

Thanks for the due dilligence in providing a testing lab. The charm functionality is great! I loved the added features it provided. I have a few notes regarding the metadata:

When you placed pipes on every line "|" it actually broke deployment and proof. The requires: and Categories: fields do not require a pipe character, that is only for multi-line strings. To bring the metadata back into passing form, evaluate the following metadata.yaml

http://paste.ubuntu.com/8300884/

Once you get that small detail shored up, the final detail missing for store acceptance is a suite of tests. As this charm targets trusty, the baseline requirement for store acceptance would be unit-tested hooks, however we prefer integration tests written with amulet.

Thanks again for the continued effort of making such a great and useful charm. You're extremely close to store acceptance and barring some slight modifications will be ready for promulgation soon.

All the best

Charles

Changed in charms:
status: Fix Committed → Incomplete
Revision history for this message
Kent Baxley (kentb) wrote :

Thanks, Charles. I've linked a new branch, that contains all recommended changes as well as a first crack at some Amulet tests.

The new branch is the same as my original one. I wanted to move it over since I won't be working with it very much from here on out.

Thanks, again, for the compliments and the thorough review.

Changed in charms:
status: Incomplete → Fix Committed
Revision history for this message
Adam Israel (aisrael) wrote :

Hi Kent,

Thanks for making the recommended changes and adding Amulet tests!

I took a quick look at the new branch and am happy to report that charm proof now passes cleanly. I don't have access to the testing lab that Charles mentioned, which limited my ability to fully run the tests. I'm going to assign this to Charles to re-review.

Thanks again for all of your efforts!

Adam

Revision history for this message
Charles Butler (lazypower) wrote :

Greetings team,

Last word that I had from the charm was the amulet tests are not passing in the lab. Once we have verification that the charm tests have passed feel free to move the bug status from incomplete to fix-submitted and someone will be along shortly to re-review your work.

Thanks for your patience during the review cycle and staying committed to creating a quality charm.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

Changed in charms:
status: Fix Committed → Incomplete
status: Incomplete → In Progress
Changed in charms:
assignee: nobody → Mark W Wenning (mwenning)
Revision history for this message
Mark W Wenning (mwenning) wrote :
Revision history for this message
Mark W Wenning (mwenning) wrote :
Changed in charms:
status: In Progress → Fix Committed
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.