'host' property is missing for Cinder volume connector when boot from volume

[Guang Yee]

Description
===========
Not sure if this is a bug or by design. But in any case, we need to clarify how to the volume connectors are created when boot from volume. According the doc (https://docs.openstack.org/ironic/pike/admin/boot-from-volume.html), we need to create an iSCSI Qualifying Name (IQN) connector that is unique to the SAN. It didn't mention anything else so its gives reader the impression that nothing else is required. But the problem is, depending on the Cinder backend driver, we also need the 'host' property in the connector. Otherwise, BFV will fail and you'll see something similar to this appearing in the Cinder Volume logs.

cinder-volume.log:2018-04-20 21:20:28.543 20455 ERROR cinder.volume.manager hostname = common._safe_hostname(connector['host'])
cinder-volume.log:2018-04-20 21:20:28.543 20455 ERROR cinder.volume.manager KeyError: 'host'
cinder-volume.log:2018-04-20 21:20:28.543 20455 ERROR cinder.volume.manager

Looking at the code where the volume connector is created, seems like the only workaround would be to also create another volume connection of type 'ip'. See

https://github.com/openstack/nova/blob/stable/pike/nova/virt/ironic/driver.py#L1773

So we ended up creating two connectors, iqn and ip, in order to make the error go away.

Steps to reproduce
==================
1. Configure Ironic to Boot From Volume according to the doc (https://docs.openstack.org/ironic/pike/admin/boot-from-volume.html)
2. Configure a Cinder backend (i.e. 3par_iscsi) which requires the 'host' property.
3. Attempt to BFV and it will fail. Check the Cinder Volume logs for the "KeyError: 'host'" entry.

Expected result
===============
BFV should work.

Actual result
=============
BFV failed with the "KeyError: 'host'" entry in Cinder Volume log.

Environment
===========
1. Install DevStack with Ironic BFV enabled. See https://docs.openstack.org/ironic/pike/contributor/ironic-boot-from-volume.html

2. Configure Cinder to use VSA or some backend which request a 'host'.

3. Do BFV

[Walt Boring (walter-boring)]

Also,
  Here is a list of other cinder drivers and libraries that assume that the host entry
is always there for attach and detach.

datacore/driver.py:357
datacore/iscsi.py:136:
datacore/fc.py:135:
dell_emc/vmax/masking.py:1079:
dell_emc/vmax/common.py:481:
dell_emc/vmax/fc.py:306:
dell_emc/xtremio.py:1278:
dell_emc/vnx/adapter.py:1299:
dell_emc/unity/adapter.py:295:
pure.py:1836:
huawei/hypermetro.py:184:
huawei/huawei_driver.py:1993:
kaminario/kaminario_common.py:835:
kaminario/kaminario_fc.py
kaminario/kaminario_iscsi.py
drbdmanagedrv.py
coprhd/scaleio.py
coprhd/iscsi.py
coprhd/fc.py
inspur/instorage/instorage_common.py
inspur/instorage/instorage_iscsi.py
inspur/instorage/instorage_fc.py
hpe/hpe_3par_iscsi.py
hpe/hpe_lefthand_iscsi.py
hpe/hpe_3par_fc.py
storpool.py
ibm/ibm_storage/ds8k_helper.py
ibm/ibm_storage/xiv_proxy.py
ibm/storwize_svc/storwize_svc_iscsi.py
ibm/storwize_svc/storwize_svc_fc.py
ibm/storwize_svc/storwize_svc_common.py
ibm/flashsystem_fc.py
ibm/flashsystem_iscsi.py
ibm/flashsystem_common.py

[Julia Kreger (juliaashleykreger)]

So a few different things, and my apologies if this comes across as a stream of consciousness.

1) Ironic has moved to https://storyboard.openstack.org for bug/work tracking, you'll need to open an issue there, but first read this entire reply.

2) This is partially by design. Ironic expects a "mac" address entry in the volume connector information stored in Ironic to represent available ports to be used. This was intended to allow for deployments with multiple network attachments, and to be able to declare what the address would be that is acceptable for storage interface use.

3) Given that the host key is required, the ironic virt driver code will need to be updated. Right now it only does it if it has an IP for iscsi, but it looks like fibrechannel types also require it.

4) There is another bug in that we don't save this information across TO ironic. This is important in that ironic will delete/re-add attachment data to cinder if a node is power cycled. This also allows a node to be turned off and SAN attachments removed, which would be important if a node is powered off for a long period of time. Given the volume connector data is agnostic, I think we could add a "host" key type to the storage interface (this is where opening an item over in ironic land is needed), and then we could just likely wipe those connectors upon tear_down of the instance. Perhaps as a "host" key which is not one we explicitly document and could always change anyways, so it would make sense. Relatively the first part of that in the handling on the ironic side seems reasonable and easy. Saving the data to nova seems like it might be a little more complex, but if we have the host volume connector type being wiped by default on tear_down, then I think it would be okay.

[Walt Boring (walter-boring)]

One of the other items I noticed in the ironic code is that it assumes that the only way to return multipath = True, is if there are more than 1 connectors created in ironic. This isn't a requirement for multipath to work. In fact most multipath deployments rely entirely on the target side to export the volume via multiple ports.

[Walt Boring (walter-boring)]

I have created a storyboard bug for this. here:
https://storyboard.openstack.org/#!/story/2001926

[Matt Riedemann (mriedem)]

Nova tracks bugs in launchpad, not storyboard, so this is OK for the nova bug. SB is fine for tracking ironic-specific work.

> Saving the data to nova seems like it might be a little more complex, but if we have the host volume connector type being wiped by default on tear_down, then I think it would be okay.

With the new style attachments flow for volumes in nova/cinder, the connector dict gets stored with the volume attachment record in Cinder, so you don't need that in nova, although even with the legacy volume attach flows, nova stashes the host connector dict in the BlockDeviceMapping.connection_info, see:

https://github.com/openstack/nova/blob/93f4364a4e7539470edc8bde4e335a9b0d650c57/nova/volume/cinder.py#L568

So we should have the host connector persisted either way. How you access it depends on the flow used to create the volume attachment.

[Matt Riedemann (mriedem)]

https://review.openstack.org/#/c/541420/ is also related for boot from volume to create new-style volume attachments.

[Julia Kreger (juliaashleykreger)]

So the fact the interaction with ironic will likely need some extra work down the road since we still need to store a copy in Ironic..

There seems to be two different things going on here. The fact we just aren't storing host if we don't have an ip address, which seems like an easy fix in the grand scheme of things, and that we are exiting without any fixed_ips when the user is running the test, which just doesn't make sense to me, but I also don't have the strongest knowledge of how nova's networking is all wired together.

There might be a second issue in that we could be beating the vif being recorded on the node. I think we're going to need more logging to figure this out.

A simple fix, which would likely help would just to fallback on the host field and set it if not set to the instance hostname.

There is also the conundrum of host field which I think we discussed in IRC at some point when this bug was filed, in that we could go ahead and post it to ironic. Ideally, if we do get an IP address out of a vif, we need to go ahead and post it to ironic so it can rebuild the network information with everything down. (the case of someone is operating everything on top of ironic, and they are trying to boot up without access to cinder.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/571982

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/571982
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=22ef3465215941097a96f078ae93aa65758d5a64
Submitter: Zuul
Branch: master

commit 22ef3465215941097a96f078ae93aa65758d5a64
Author: Julia Kreger <email address hidden>
Date: Sun Jun 3 11:19:57 2018 -0700

    ironic: bugfix: ensure a host is set for volume connectors

    Somehow, in some not fully understood case,
    we have discovered that we are not storing a
    host field for the volume connector information.

    This information is largely required for logging,
    and in some cases back-end driver relationships,
    but previously we would only set the field in the
    case that we have an IP address. That is not ideal
    in that some users may be using other transport
    mediums like fibre channel.

    Change-Id: I17b1aaa26b7e7aff3ebb73fa10ab576149a00a3f
    Related-Bug: #1766661