Cinder volume attachments_list attribute should not be cached

[Gerry Buteau]

The Cinder volume 'attachments_list' attribute is a dynamically built list and must be queried live from the Cinder API on each request. There are several factors that may change the contents of this list that are unknown to Heat. Caching should be prevented for this attribute.

This is causing the following murano bug:

https://bugs.launchpad.net/murano/+bug/1703387

[OpenStack Infra (hudson-openstack)]

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

[Rabi Mishra (rabi)]

I did try to reproduce this with the below template.

[centos@devstack ~]$ cat volume_test.yaml

heat_template_version: pike
resources:
  n1:
    type: OS::Nova::Server
    properties:
      flavor: m1.nano
      image: cirros-0.3.5-x86_64-disk
  b1-vol:
    type: OS::Cinder::Volume
    properties:
      size: 2
# b1-vol-attachment:
# type: OS::Cinder::VolumeAttachment
# properties:
# instance_uuid: {get_resource: n1}
# volume_id: {get_resource: b1-vol}
outputs:
  attachments:
    value: {get_attr: [b1-vol, attachments_list]}

Steps to reproduce:

1. create stack with the above template.
2. Do a stack show, which would store the outputs referenced in outputs.
3. Do a stack update after uncommenting the VolumeAttachment resource.
4. Do a stack show again, it shows an empty list for attachments output which is incorrect.

If I remove step 2, it seems to get the correct value for attachment_list attribute in step 4.

Probably the cache is not refreshed for it to be stored again?

[Gerry Buteau (gerry.buteau)]

Yes, this is what I'm seeing as well. Also, the cinder attachments will be filtered based on 'attach_status' of both the Volume and VolumeAttachment. In both cases the 'attach_status' must be 'attached' and not 'attaching'. So if the 'stack show' is performed during this time it will cache the empty list as well it seems.

In this case is there a option invalidate the cache without specifying CACHE_NONE on the 'attachments_list' attribute?

[Zane Bitter (zaneb)]

> If I remove step 2, it seems to get the correct value for attachment_list attribute in step 4.

That makes sense, but will no longer help once https://review.openstack.org/#/c/475099/ merges.

The correct fix here is the one proposed, marking that attribute as CACHE_NONE.

In the meantime, Murano could work around the problem by using the "openstack stack resource show --with-attr attachments_list" instead of an output to get the attachments_list attribute.

[Rabi Mishra (rabi)]

As mentioned in the review, we should expect similar behaviour when using the below template, but interestingly we don't.

heat_template_version: pike
resources:
  server:
    type: OS::Nova::Server
    properties:
      image: cirros-0.3.5-x86_64-disk
      flavor: m1.nano
  port:
    type: OS::Neutron::Port
    properties:
      network: private
  floating_ip:
    type: OS::Neutron::FloatingIP
    properties:
      floating_network: public
# floating_ip_assc:
# type: OS::Neutron::FloatingIPAssociation
# properties:
# floatingip_id: {get_resource: floating_ip}
# port_id: {get_resource: port}

outputs:
  floating_ports:
    value: {get_attr: [floating_ip, port_id]}

[Zane Bitter (zaneb)]

Yep, I would have expected the same behaviour there. (As long as you either do the stack show in step 2 or have the patch https://review.openstack.org/#/c/475099/.) We should probably fix that one in the same patch.

[Gerry Buteau (gerry.buteau)]

It appears that attribute values of 'None' will not be cached, see comment in:

heat/engine/attributes.py : Attributes::__getitem__

  # only store if not None, it may resolve to an actual value
  # on subsequent calls

This explains the anomaly with port_id. This remains a live (un-cached)
call until it returns non-'None'.

This also helps explain the original issue with attachments_list.
Since the empty list is being returned (i.e. 'non-None') it will
be cached immediately.

I have verified the original test case by omitting step 2, I do see
the attachments_list correctly in step 4.

However, with patch https://review.openstack.org/#/c/475099/, omittimg step 2
will produces an incorrect attachments_list in step 4.
(port_id is correct however because 'None' is never cached)

It seems like preventing the caching of empty lists might help here,
but I still feel that CACHE_NONE is safer in this case due to the
nature of the state transition of volume attachment. For instance, issuing
a stack show during an UPDATE_IN_PROGRESS could cache an incorrect
representation of the list.

One other note, I was able to produce the original problem with omitting step 2
and quickly issuing a stack show after step 3 (no --wait option,
i.e. stack status was UPDATE_IN_PROGRESS). In that case step 4 produced an
incorrect list as well.

[Rabi Mishra (rabi)]

>attribute values of 'None' will not be cached

Ah, that explains it. I checked and FloatingIP port_id behaves the same way if it's not None (change the port for FloatingIPAssociation with an stack update). Let's mark these attributes as CACHE_NONE to fix these.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/482663
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=9269787d39e052318ec59c58102cca128065b37a
Submitter: Jenkins
Branch: master

commit 9269787d39e052318ec59c58102cca128065b37a
Author: Gerry Buteau <email address hidden>
Date: Tue Jul 11 10:09:15 2017 -0400

    Cinder volume attachment attributes should not be cached

    The Cinder volume 'attachments' and 'attachments_list' attributes are
    dynamically built lists and must be queried from the Cinder API on each
    request. There are several factors that may change the contents of this
    list that are unknown to Heat. Caching should be prevented for these
    attributes.

    Closes-Bug: #1703605
    Related-Bug: #1703387
    Co-Authored-By: Paul Bourke <email address hidden>
    Change-Id: I1a646e567b2d88f2acf09b3077c205c5b061d4bc

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/483812
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=b0b2161ca98d6d177de12cc46a6be24c5b3b39d6
Submitter: Jenkins
Branch: master

commit b0b2161ca98d6d177de12cc46a6be24c5b3b39d6
Author: rabi <email address hidden>
Date: Fri Jul 14 10:36:03 2017 +0530

    Add CACHE_NONE for FloatingIP attributes

    'port_id' and 'fixed_ip_address' attributes for FloatingIP would change
    when FloatingIPAssociation is created/updated. Therefore we should not
    cache/store them in db.

    Change-Id: I8cd580c15289a674888f194e8244041ed64bb172
    Ralated-Bug: #1703605

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/487641

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/487651

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/heat 9.0.0.0b3 development milestone.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/487641
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=1ac09ea1b1b172934d7b8c8395f2d6669ae0dcf3
Submitter: Jenkins
Branch: stable/ocata

commit 1ac09ea1b1b172934d7b8c8395f2d6669ae0dcf3
Author: rabi <email address hidden>
Date: Fri Jul 14 10:36:03 2017 +0530

    Add CACHE_NONE for FloatingIP attributes

    'port_id' and 'fixed_ip_address' attributes for FloatingIP would change
    when FloatingIPAssociation is created/updated. Therefore we should not
    cache/store them in db.

    Change-Id: I8cd580c15289a674888f194e8244041ed64bb172
    Ralated-Bug: #1703605
    (cherry picked from commit b0b2161ca98d6d177de12cc46a6be24c5b3b39d6)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/487651
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=579866aefc0d69a203bf4ce65a6c3d24f8ce079f
Submitter: Jenkins
Branch: stable/ocata

commit 579866aefc0d69a203bf4ce65a6c3d24f8ce079f
Author: Gerry Buteau <email address hidden>
Date: Tue Jul 11 10:09:15 2017 -0400

    Cinder volume attachment attribute should not be cached

    The Cinder volume 'attachments'attribute are
    dynamically built lists and must be queried from the Cinder API on each
    request. There are several factors that may change the contents of this
    list that are unknown to Heat. Caching should be prevented for this
    attribute.

    Closes-Bug: #1703605
    Related-Bug: #1703387
    Co-Authored-By: Paul Bourke <email address hidden>
    Change-Id: I1a646e567b2d88f2acf09b3077c205c5b061d4bc
    (cherry picked from commit 9269787d39e052318ec59c58102cca128065b37a)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/heat 8.0.5 release.