Ironic virt driver node cache may be missing required fields

[Mark Goddard]

Per the discussion in [1], the ironic nodes added to the node cache in the ironic virt driver may be missing the required field resource_class, as this field is not in _NODE_FIELDS. In practice, this is typically not an issue (possibly never), as the normal code path uses a detailed list to sync all ironic nodes, which contain all fields (including resource_class). However, some code paths use a single node query with the fields limited to _NODE_FIELDS, so this should be changed to include the required resource_class.

There are a number of other minor related issues picked up in that discussion, which don't really deserve their own bugs:

* Filter the node list in _refresh_cache using _NODE_FIELDS.
* Improve unit tests to use representative filtered node objects.
* Remove _parse_node_instance_info and associated tests.

[1] https://review.openstack.org/#/c/532288/9/nova/virt/ironic/driver.py@79

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/539506
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=5895566a428be4c30c31ae94070282566a6cc568
Submitter: Zuul
Branch: master

commit 5895566a428be4c30c31ae94070282566a6cc568
Author: Mark Goddard <email address hidden>
Date: Wed Jan 31 11:11:32 2018 +0000

    Add resource_class to fields in ironic node cache

    Per the discussion in [1], the ironic nodes added to the node cache in
    the ironic virt driver may be missing the required field resource_class,
    as this field is not in _NODE_FIELDS. In practice, this is typically
    not an issue (possibly never), as the normal code path uses a
    detailed list to sync all ironic nodes, which contain all fields
    (including resource_class). However, some code paths use a single
    node query with the fields limited to _NODE_FIELDS, so could result in a
    node in the cache without a resource_class.

    This change adds resource_class to _NODE_FIELDS.

    [1]
    https://review.openstack.org/#/c/532288/9/nova/virt/ironic/driver.py@79

    Change-Id: Id84b4a47d05532d341a9b6ca2de7e9e66e1930da
    Closes-Bug: #1746209

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/queens
Review: https://review.openstack.org/546085

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/546085
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=6b11eb794f5881c9c8ed284b2edfa4c31ed62b2b
Submitter: Zuul
Branch: stable/queens

commit 6b11eb794f5881c9c8ed284b2edfa4c31ed62b2b
Author: Mark Goddard <email address hidden>
Date: Wed Jan 31 11:11:32 2018 +0000

    Add resource_class to fields in ironic node cache

    Per the discussion in [1], the ironic nodes added to the node cache in
    the ironic virt driver may be missing the required field resource_class,
    as this field is not in _NODE_FIELDS. In practice, this is typically
    not an issue (possibly never), as the normal code path uses a
    detailed list to sync all ironic nodes, which contain all fields
    (including resource_class). However, some code paths use a single
    node query with the fields limited to _NODE_FIELDS, so could result in a
    node in the cache without a resource_class.

    This change adds resource_class to _NODE_FIELDS.

    [1]
    https://review.openstack.org/#/c/532288/9/nova/virt/ironic/driver.py@79

    Change-Id: Id84b4a47d05532d341a9b6ca2de7e9e66e1930da
    Closes-Bug: #1746209
    (cherry picked from commit 5895566a428be4c30c31ae94070282566a6cc568)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 17.0.1 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 18.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/539508
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=8bbad196a7f6a6e2ea093aeee87dfde2154c9358
Submitter: Zuul
Branch: master

commit 8bbad196a7f6a6e2ea093aeee87dfde2154c9358
Author: Mark Goddard <email address hidden>
Date: Fri Jan 26 14:55:34 2018 +0000

    Include only required fields in ironic node cache

    The ironic virt driver maintains a cache of ironic nodes to avoid
    continually polling the ironic API. Code paths requiring a specific
    node use a limited set of fields, _NODE_FIELDS, when querying the
    ironic API for the node. This reduces the memory footprint required by
    the cache, and the network traffic required to populate it. However,
    in most cases the cache is populated using a detailed node list
    operation in _refresh_cache(), which includes all node fields.

    This change specifies _NODE_FIELDS in the node list operation in
    _refresh_cache().

    We also modify the unit tests to use fake node objects that are
    representative of the nodes in the cache.

    Change-Id: Id96e7e513f469b87992ddae1431cce714e91ed16
    Related-Bug: #1746209

[OpenStack Infra (hudson-openstack)]

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

commit e91fc7da82755358ae6523842d19968f3ac269de
Author: Mark Goddard <email address hidden>
Date: Fri Jan 26 14:54:10 2018 +0000

    Request only instance_uuid in ironic node list

    For list_instances and list_instance_uuids in the ironic virt driver, we
    only need the instance_uuid field of ironic nodes, so restrict the
    node list request to include only that field.

    Change-Id: If9d6747f8e894c1a78a6d89961a3fa2bc6459465
    Related-Bug: #1746209

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/queens
Review: https://review.opendev.org/724862

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/724862
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=f5b6dc603c5f6f72ab35ef21ab9e35ec982e3219
Submitter: Zuul
Branch: stable/queens

commit f5b6dc603c5f6f72ab35ef21ab9e35ec982e3219
Author: Mark Goddard <email address hidden>
Date: Fri Jan 26 14:55:34 2018 +0000

    Include only required fields in ironic node cache

    The ironic virt driver maintains a cache of ironic nodes to avoid
    continually polling the ironic API. Code paths requiring a specific
    node use a limited set of fields, _NODE_FIELDS, when querying the
    ironic API for the node. This reduces the memory footprint required by
    the cache, and the network traffic required to populate it. However,
    in most cases the cache is populated using a detailed node list
    operation in _refresh_cache(), which includes all node fields.

    This change specifies _NODE_FIELDS in the node list operation in
    _refresh_cache().

    We also modify the unit tests to use fake node objects that are
    representative of the nodes in the cache.

    Change-Id: Id96e7e513f469b87992ddae1431cce714e91ed16
    Related-Bug: #1746209
    (cherry picked from commit 8bbad196a7f6a6e2ea093aeee87dfde2154c9358)

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/pike
Review: https://review.opendev.org/754444

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.opendev.org/755653

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova pike-eol release.