discovery does not include /dev/disk/by-path device name

[Ben England]

OpenStack Ironic discovery does not discover, save and report out /dev/disk/by-path names of block devices. This would be easy to correct and would provide sysadmins with a reliable alternative to using traditional device names to define Ceph OSDs, and also for root device hinting (you can use WWN for root device hinting but this is different by definition for every node, whereas /dev/disk/by-path can often be identical across a similar set of nodes). Without this information, you have to deploy Linux first to the node to find out what the /dev/disk/by-path names are, resulting in multiple deployments needed when only one should be needed.

There is another problem implicit here - Ironic database of block devices may become invalid once the node is rebooted, since the block device name can change after reboot. However, if you include /dev/disk/by-path info, then at least you can determine exactly what physical device is being referred to by the block device name at that point in time.

background: Linux does not guarantee stability of low-level device names across reboots. Unfortunately for some hardware configurations, such as those with > 1 storage controller, traditional device names such as /dev/sdc are not stable across reboots, and OpenStack Ironic does multiple reboots during deployment. In such configurations it may be necessary to specify block devices using /dev/disk/by-path softlinks, which have a name that is stable across reboots and is often the same for an entire set of hardware nodes (i.e. if they were all purchased at the same time).

Version-Release number of selected component (if applicable): sorry this is old stuff, if someone suspects that the bug has been fixed in newer versions let me know.

$ rpm -qa | grep -i ironic
python-ironicclient-1.7.1-1.el7ost.noarch
python-ironic-lib-2.1.2-2.el7ost.noarch
openstack-ironic-conductor-6.2.2-5.el7ost.noarch
openstack-ironic-api-6.2.2-5.el7ost.noarch
python-ironic-inspector-client-1.10.0-1.el7ost.noarch
openstack-ironic-inspector-4.2.1-3.el7ost.noarch
openstack-ironic-common-6.2.2-5.el7ost.noarch
puppet-ironic-9.5.0-1.el7ost.noarch

How reproducible:

the lack of /dev/disk/by-path info is reproducible every time. The instability of block device names across reboots is reproducible on at least one kind of system that has multiple storage controllers in it, details available upon request. In this case, one storage controller was for the (operating) system disks, and the other storage controller was a LSI 3108 (MegaRAID descendant) that was for the Ceph OSDs.

Steps to Reproduce:
1. do ironic discover
2. use command "openstack baremetal node list" to find discovered nodes
3. pick a node uuid and use command:

# openstack baremetal introspection data save $uuid | jq '.inventory.disks'

Actual results:

you get a list of block devices along with info about those devices, but /dev/disk/by-path name is not part of this list.

  {
    "size": 800166076416,
    "rotational": false,
    "vendor": null,
    "name": "/dev/nvme0n1",
    "wwn_vendor_extension": null,
    "wwn_with_extension": null,
    "model": "INTEL SSDPEDMD800G4",
    "wwn": null,
    "serial": "PHFT6201004F800CGN"
  },

Expected results:

Would like to see a field added for /dev/disk/by-path name. This info is available at discovery time, though you have to search through the /dev/disk/by-path/ directory to find the softlink that points to a particular block device.

Additional info:

A related bug in OpenStack kept us from using /dev/disk/by-path names for Ceph OSDs, this was reported by John Fulton, but a fix for this in ceph-puppet osd.pp was found by Joe Talerico, see:

https://bugs.launchpad.net/puppet-ceph/+bug/1677605
https://review.openstack.org/#/c/451826

[Jay Faulkner (jason-oldos)]

Thanks for the detailed write-up, I'm going to mark this an RFE as I believe supporting those by-path devices would be considered a feature enhancement rather than a bug fix.

Can you update the bug description to clearly list the action we want to take -- I'm not sure if you're proposing adding the by-path device names, or replacing any non-pathed names with them.

Thanks!

[Ben England (bengland)]

Jay, I'm suggesting that Ironic save and report /dev/disk/by-path value as an attribute of discovered block devices, nothing more.

I'd suggest that this is a bug not an RFE. Does OpenStack support hardware where block device names are not stable across reboots, or not? I think it has to, because Linux in general does not guarantee block device name stability across reboots. Suppose you get 1000 nodes and you have to go deploy OpenStack on them right away. You don't want to do multiple deployments, since this is very time consuming. So you need to be able to get the device information that you need to construct the input YAML for deployment, correctly the first time, using only what is available from introspection.

Paragraph 2 did cloud the issue a bit. I was not suggesting that all device names should be replaced with "pathed" equivalent. Not all configurations require /dev/disk/by-path use, but it should be allowed.

And if we have /dev/disk/by-path field from introspection data, it then becomes possible to write a script that will turn the introspection data into a deployment YAML using rule-based device classification. Ironic metadata-only device cleaning together with this can result in a repeatable deployment process. Nodes in a large-scale config might not have exactly the same number of raw block devices in the same PCI slots and unit numbers. For example, the manufacturer might not have used the same PCI slot for all units, or some of the drives might have failed, or some of the nodes might have been purchased at different times with different hardware in them.

thanks for your interest.

Ben England, Perf & Scale Engr., Red Hat

[Ben England (bengland)]

I think a patch something like this might work. A primitive unit test that I wrote did run successfully. But I'm not a full-fledged openstack developer and am having trouble with getting unit tests to pass, gerrit isn't letting me in, I don't know how to build ramdisk to use the fix in a real-world system, etc. etc. So I was wondering if any ironic developers were interested in using this.

[Giulio Fidente (gfidente)]

Thanks Ben, looks like a sane idea to me!

[Giulio Fidente (gfidente)]

Ben, can you remove from the affected components list all but ironic-python-agent?

[Ben England (bengland)]

done, could not remove ironic-inspector though (no minus sign available)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/498489

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/498489
Committed: https://git.openstack.org/cgit/openstack/ironic-python-agent/commit/?id=d0a53149f82a3587515a4371f0f4cad8570dc715
Submitter: Jenkins
Branch: master

commit d0a53149f82a3587515a4371f0f4cad8570dc715
Author: Ilya Etingof <email address hidden>
Date: Mon Aug 28 16:59:39 2017 +0200

    Report /dev/disk/by-path on inspection

    When inspecting block devices on a node, discover and
    report the /dev/disk/by-path/XXX name along with the
    /dev/XXX block device name.

    The second name does not change between Linux system
    reboots and has greater chances to be the same across
    similarly configured nodes.

    Note: this patch depends on
    https://review.openstack.org/#/c/500524/
    library patch, but this dependency can't be expressed
    with Depends-On clause. Therefore once this patch
    requires a followup patch to enable one currently disabled
    test in this patch.

    Change-Id: I09874f19890500d352521f89573e2aaf50a29022
    Closes-Bug: #1679726

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic-python-agent 3.0.0 release.