Page MenuHomePhabricator

Add a fact holding the type of a disk (spinning/ssd)
Closed, ResolvedPublic

Description

For T288345 we needed to figure out the type of disk(s) a node is equipped with.

We did so by looking for the string "ssd" in the model name (https://gerrit.wikimedia.org/r/c/operations/puppet/+/710566 ), but that is not really ideal. It would be nice to have a fact that provides /sys/block/*/queue/rotational or maybe even something that tells if the disk is hdd, ssd or nvme.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 714572 had a related patch set uploaded (by JMeybohm; author: JMeybohm):

[operations/puppet@production] wmflib: Add disk_type fact

https://gerrit.wikimedia.org/r/714572

It seems to me that looking only at /sys/block/*/queue/rotational would create quite large fact for hosts with a lot of virtual disks like Ganeti and Kubernetes ones (more than 100 for most k8s hosts). And we have already issues with puppetdb, some due to the size of the facts for some hosts, inlcuding k8s ones.

I'm wondering if we should instead either iterate or even expand the built-in disks fact that seems to already have only the real disks on those hosts. (cc-ing @jbond )

I'm wondering if we should instead either iterate or even expand the built-in disks

The implmentation JMeybohm went with iterated over the current disks fact. extending built in structured facts has proven difficult in the past, however i need to look into that a bit more

Change 714572 merged by JMeybohm:

[operations/puppet@production] wmflib: Add disk_type fact (hdd|ssd|virtual)

https://gerrit.wikimedia.org/r/714572

The merged implementation creates a new fact disk_type based on built-in disks fact, using the same keys. So for example thumbor1006.eqiad.wmnet now has:

disks => {
  'sdb': {'model': 'MTFDDAK960TDS', 'vendor': 'ATA', 'size_bytes': 960197124096, 'size': '894.25 GiB'},
  'sda': {'model': 'MTFDDAK960TDS', 'vendor': 'ATA', 'size_bytes': 960197124096, 'size': '894.25 GiB'}
}
disk_type => {
 'sdb': {'type': 'ssd'},
 'sda': {'type': 'ssd'}
}

Oh and FTR: With Puppet 7 this is part of the disks fact already: https://puppet.com/docs/puppet/7/core_facts.html#disks

Change 714969 had a related patch set uploaded (by JMeybohm; author: JMeybohm):

[operations/puppet@production] wmflib: Simplify the structure of disk_type fact

https://gerrit.wikimedia.org/r/714969

Change 714969 merged by JMeybohm:

[operations/puppet@production] wmflib: Simplify the structure of disk_type fact

https://gerrit.wikimedia.org/r/714969