Expose PD sizes, and support older MegaCli binaries.

[negz]

I recently had the need to expose the sizes of the physical disks behind the Dell PERCs in our production machines as facts. We're stuck with a fairly old build of MegaCli due to some other tooling dependencies, so I also hacked around a few of the facts to fall back on the older syntax.

Apologies in advance for my horrible Ruby skills. :)

[jhoblitt]

I have no objection to supporting older versions of megacli but I dislike attempting to run multiple versions of each command. How about using the megacli_version fact to select the correct set of options for each fact?

[jhoblitt]

Another option would be define multiple resolutions for each fact. Eg., https://docs.puppetlabs.com/facter/2.3/custom_facts.html#fact-precedence

[negz]

Makes sense - I'll look into the multiple resolution option tomorrow. Thanks!

[jhoblitt]

Hopefully it's possible to determine the megacli version and have one set of the resolutions bail out before execing. That would minimize the number of times the megacli binary is actually run, which is desirable in my opinion as it can be fairly slow.

[negz]

https://docs.puppetlabs.com/facter/2.3/custom_facts.html#confining-facts

So I would guess we can likely do that using Puppet's confine optional. Particularly if it's possible to check greater/less than via confine. What I need to figure out in that case is at what version of megacli did the command syntax change? :)

[negz]

http://www.lsi.com/downloads/Public/RAID%20Controllers/RAID%20Controllers%20Common%20Files/Linux%20MegaCLI%208.07.10.txt

My best guess it was 8.02.16 based on the following line from the above:

LSIP200100159 (CO) (auto4comp UNIV_VIVA_Cli) CLI version command 

[jhoblitt]

It's not well documented but it is possible to pass a block to confine. https://github.com/puppetlabs/facter/blob/master/lib/facter/util/confine.rb#L17-L19

[negz]

Hah, which was apparently committed by my colleague @dalen. ;)

[jhoblitt]

It's hard to use puppet without touching @dalen code. :)

[jhoblitt]

Out of curiosity, why are you restricted to an old version of megacli? LSI informed me that they are planning to eventually drop it completely for storcli.

[negz]

To be quite honest, mostly due to laziness in this instance.

We have a whole bunch of tooling built around megacli 8.00.11. I need to expose PD sizes via Facter to unblock a thing that in turn unblocks the actual thing that I'm supposed to be working on. Adding legacy support to puppet-smartd is the path of least resistance compared to auditing, updating, and testing the rest of our tooling to work with current build.

[negz]

PTAL

I like this method better, though it does still feel like there's a fair bit of duplication given that a lot of these facts simply run a command, then extract something from the output using a regex. It might be worth factoring all that out into a utility function that takes a command to run and a regex to return on, but I'd like to get this PR merged first if it looks good to you.

[jhoblitt]

What I had envisioned was constraining the resolution directly based on the megacli_version but megacli_legacy seems reasonable at this point. I suspect, new resolutions will eventually need to be made with storcli rather than having to deal with a megacli API change.

[jhoblitt]

@negz This was a huge PR. Thank you for all the effort to get this merged!

[jhoblitt]

@negz This PR is included in the v2.4.0 release that was just pushed to the forge. Thanks again!

[negz]

Awesome, thanks a bunch!

P.S. By way of a feature request, it might be nice to move the megacli facts out into a new package. I for one won't actually be using the smartd stuff - I just want to expose some details about our RAID controllers.

[jhoblitt]

I've been planning to eventually to move the megaraid specific facts into this module https://github.com/jhoblitt/puppet-megaraid but it needs a bit of TLC first.