Puppet » Facter

This is actually a pretty cool fact, and I can see the utility. However, the concern that I have is that all the actual evaluation of the block devices happens outside of the setcode blocks, which means that this will only be evaluated at load time. If someone were to hot swap a hard drive and facter was running in a long running ruby process, it would not pick up the change. I’ll run this by a dev and see what they think, but again, this would definitely be good to have inside facter.

Adrien, thanks for the feedback! I hadn’t considered the situation you describe (although I can see where that would happen) – if you guys have any suggestions on how to improve the code I am happy to listen. This was my first real crack at Ruby so I’m sure it needs a lot of work.

Thanks again and let me know what else I can do if anything to try to help!

I’ve just pushed a minor update to the github repo for this fact which prevents a small bug in the rare case that a device has a /sys/block/*/device/ directory but no vendor and model files.

Additionally I’ve reviewed some of the other core facts and the situation you describe appears in many of them (for example network interfaces – these could change on demand especially in a virtualized environment, or in cases where removal (USB) network interfaces were used). If there is a recommended workaround I can try to update any fact that might be impacted by changes during a long running facter process but otherwise I think it may just be an occupational hazard of having a long running process :)

So I love the idea of tracking this. By I wanted to add my 2p just for good luck.

Do we expect other metadata to be gleaned about a block device? Here we are assuming that ‘vendor model’ is the main piece of information a user wants about a device. What about other properties such as size, type?

An approach might be to break this out somehow. For example:

blockdevice_sda_vendor
blockdevice_sda_model
blockdevice_sda_size
blockdevice_sda_type

(Which would look much nicer if we had structured data support).

Ken I totally agree, having the size and other details (like SCSI LUN, etc) would be really great. However in this initial version I just went with what was most important to me – the names of the devices (sda sdb sdc) and the make/model (which has been really handy to find which machines have what type of drives, locate old drives, etc, with puppet inventory service).

That being said, adding even more facts probably isn’t the solution – on some of my boxes that have 500+ IP’s up on them, it takes facter about 20 seconds to run and generate all ~1600 facts that come “out of the box” with 1.6.2.

Would it be better to just add more in to the existing blockdevice_sda fact (like a comma separated list of values or some other ugliness)? Or is it possible that a future version of facter could support this sort of “advanced” fact in a hash style or similar?

Jason Gill wrote:

Ken I totally agree, having the size and other details (like SCSI LUN, etc) would be really great. However in this initial version I just went with what was most important to me – the names of the devices (sda sdb sdc) and the make/model (which has been really handy to find which machines have what type of drives, locate old drives, etc, with puppet inventory service).

Lol. The driving force behind most facter submissions :–). We have the problem of trying to please everyone though :–).

That being said, adding even more facts probably isn’t the solution – on some of my boxes that have 500+ IP’s up on them, it takes facter about 20 seconds to run and generate all ~1600 facts that come “out of the box” with 1.6.2.

Sounds like you need caching really :–). This particular fact you’ve written to read files is very light.

Would it be better to just add more in to the existing blockdevice_sda fact (like a comma separated list of values or some other ugliness)?

Possible – do you have a proposed format?

Or is it possible that a future version of facter could support this sort of “advanced” fact in a hash style or similar?

Yes – but in the future. There is a ticket about it somewhere.

I think the only other information that can be reliably retrieved is size (it’s the value of /sys/block/*/size multiplied by 512 to get bytes). Other details like SCSI LUN, if the device is removable, etc, may be harder to detect and are more edge cases than it may be worth finding.

That being said, it’s easy to get the size = I could just update the fact to report:

blockdevice_sda => WDC WD5000AAKS-0,465.76 GB
blockdevice_sdb => DELL PERC H700,4.09 TB
blockdevices => sda,sdb

The memory fact already has a nice util class which can shift numbers from bytes to the appropriate scale (ie, GB, TB) which could be stolen and used here.

Thoughts? This should be real easy to add and add tests for

Jason Gill wrote:

I think the only other information that can be reliably retrieved is size (it’s the value of /sys/block/*/size multiplied by 512 to get bytes). Other details like SCSI LUN, if the device is removable, etc, may be harder to detect and are more edge cases than it may be worth finding.

That being said, it’s easy to get the size = I could just update the fact to report:

blockdevice_sda => WDC WD5000AAKS-0,465.76 GB
blockdevice_sdb => DELL PERC H700,4.09 TB
blockdevices => sda,sdb

The memory fact already has a nice util class which can shift numbers from bytes to the appropriate scale (ie, GB, TB) which could be stolen and used here.

Funny – but most consumers of facts don’t like this shortening … as you can’t just go and use arithmetic with it :–). If I could go back and fix all the other numbers around facter to not use this I probably would :–).

Thoughts? This should be real easy to add and add tests for

I’m not sure I really like comma separated values for this. Its a fixed column index which isn’t very flexible when someone wants to add something later on. Also – what is the behaviour when a column is empty for whatever reason.

Also – wouldn’t model and vendor be comma separated?

Hmm good points. I can easily change to the format you suggested:

blockdevice_sda_model
blockdevice_sda_vendor
blockdevice_sda_size (and just leave this in bytes)

and we can add more facts in the future if needed. Sound acceptable?

Fine by me. Anyone else?

Done; I’ve replaced the commit in the pull request (https://github.com/puppetlabs/facter/pull/98) and I’ve also included updated tests that check for the new output. Tested on my machines without any issues; here is some example output:

blockdevice_sda_model => WDC WD5000AAKS-0
blockdevice_sda_size => 500107862016
blockdevice_sda_vendor => ATA
blockdevice_sdb_model => WDC WD5000AAKS-0
blockdevice_sdb_size => 500107862016
blockdevice_sdb_vendor => ATA
blockdevices => sda,sdb

Thanks guys!