Attach with host and instance_uuid not backwards compatible

[Sean McGinnis]

Patch https://review.openstack.org/#/c/266006/ added the ability for Cinder to accept both host and instance_uuid when doing an attach. This is not backwards compatible to earlier API versions, so when Nova calls attach with versions prior to this change with both arguments it causes a failure.

This information is needed for the multiattach work, but we should revert that change and try to find a cleaner way to do this that will not break backwards compatibility.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by John Griffith (<email address hidden>) on branch: master
Review: https://review.openstack.org/273134
Reason: Thanks Matt

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/273097
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=ed5958d552f6743f4659318a4dac0259868cb552
Submitter: Jenkins
Branch: master

commit ed5958d552f6743f4659318a4dac0259868cb552
Author: Ildiko Vancsa <email address hidden>
Date: Wed Jan 27 15:17:46 2016 +0000

    Revert "Pass host when call attach to Cinder"

    Older Cinder blows up because of the extra parameter in the detach call
    to Cinder. We need to find another way to pass the info.

    This reverts commit d31bb4be8edbce6719258ae1cbbb583a2c3c3a28.

    Closes-Bug: #1538620
    Change-Id: I99335827ee6492d3f5629850be8e7cbe19371830

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/nova 13.0.0.0b3 development milestone.

[Matt Riedemann (mriedem)]

The cinder change needs to be reverted also since it's a non-discoverable API change w/o a microversion. Nova can't rely on this API until it's in a proper microversion.

[Sean McGinnis (sean-mcginnis)]

Is anyone really expecting this call to fail if both are passed in? I wouldn't think so.

[Scott DAngelo (scott-dangelo)]

Sean, I'm not sure if I understand your question, but the call will fail if both are passed in for LIberty Cinder:
https://github.com/openstack/cinder/blob/stable/liberty/cinder/api/contrib/volume_actions.py#L101

I believe the concern was that Mitaka+ Nova with Liberty- Cinder will always fail the attach if Nova passes host+instance_id at the same time. Since the Nova patch was reverted, it does seem unlikely that anyone (Nova or other) would pass both in, and I'm not sure what harm it would do if they did and both were accepted (Mitaka cinder only). I don't see an issue in manager.py:attach_volume()

But...For Nova to use this the Cinder code needs to be microversioned. So either back this patch out or fix with microversion logic. Here's the patch that adds logic for pre-Mitaka (Microversion 3.0 or before) to continue to throw an exception for both host and instance_uuid, and allows new microversion 3.1 to accept both:
https://review.openstack.org/#/c/300684/2

[Scott DAngelo (scott-dangelo)]

Now the patch which fixes this will be microversion 3.2, since we just merged v3.1 for another patch.

[Sean McGinnis (sean-mcginnis)]

Scott, yes, my comment was not about needing to fix it with a microversion or how it fails with M-Nova and L-Cinder. It was as you said where it is unlikely anyone would be calling it (in Mitaka) with both arguments. And if they did, that they would expect a failure.

So my comment was - granted it is a behavior change in an existing API - but do we need to revert it to the old behavior since no one will call it expecting the old behavior.

As long as the v3 API has existed, it has allowed both to be passed in. Let's just keep it that way and Nova can expect that with the base v3. No need to bump up a microversion to remove and then readd a change.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Sean McGinnis (<email address hidden>) on branch: master
Review: https://review.openstack.org/275316
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.