Thanks Brian and Ghanshyam for the discussion. I've updated the tempest patch[1] to update one test that I missed earlier and also the cinder patch[2] which now returns a BadRequest stating the reason for the error and how to fix it. $ curl -g -i -X POST http://127.0.0.1/volume/v3/d6634f35c00f409883ecb10361b556c3/volumes -H "Accept: application/json" -H "Content-Type: application/json" -H "User-Agent: python-cinderclient" -H "X-Auth-Token: gAAAAABkAbZkWgdbpXNgObizvGy8jS6LoMGuxzMnMaMOw6wm2j5i5KrG2xIzWCDxrSAiaMJWqneNpKrwn8P852mPOyJB_WmxrhrmKiuafcP0KSljyW44mFwDtGN74VL50NLoVC-srL63L3xduyeF5EIlPEyDsWRqPSRZZwau7wQrngAZ8XBP3M8" -d '{"volume": {"size": 1, "consistencygroup_id": null, "snapshot_id": null, "name": null, "description": null, "volume_type": null, "availability_zone": null, "metadata": {}, "imageRef": null, "source_volid": null, "backup_id": null, "multiattach": "true"}}' HTTP/1.1 400 Bad Request Date: Fri, 03 Mar 2023 09:04:38 GMT Server: Apache/2.4.41 (Ubuntu) OpenStack-API-Version: volume 3.0 Vary: OpenStack-API-Version Content-Length: 261 Content-Type: application/json x-compute-request-id: req-a9f9999e-01e3-4970-9c32-35de193c04c1 x-openstack-request-id: req-a9f9999e-01e3-4970-9c32-35de193c04c1 Connection: close {"badRequest": {"code": 400, "message": "multiattach parameter has been removed. The default behavior is to use multiattach enabled volume types. Contact your administrator to create a multiattach enabled volume type and use it to create multiattach volumes."}} [1] https://review.opendev.org/c/openstack/tempest/+/875372 [2] https://review.opendev.org/c/openstack/cinder/+/874865 On Fri, Mar 3, 2023 at 9:00 AM Ghanshyam Mann <gmann@ghanshyammann.com> wrote:
On 3/1/23 12:19 PM, Ghanshyam Mann wrote:
---- On Wed, 01 Mar 2023 09:02:59 -0800 Brian Rosmaita wrote ---
On 2/28/23 9:02 PM, Ghanshyam Mann wrote: [snip]
I think removing from client is good way to stop exposing this
but API is separate things and removing the API request
the existing users using it this way. Tempest test is one good example of such users use case. To maintain the backward compatibility/interoperability it should be removed by bumping the microversion so that it continue working for older microversions. This way we will not break the existing users and will provide the new way for users to start using.
It's not just that this is not recommended, it can lead to data loss. We should only allow multiattach for volume types that actually support it. So I see this as a case of "I broke your script now, but you'll thank me later".
We could microversion this, but then an end user has to go out of
way and add the correct mv to their request to get the correct behavior. Someone using the default mv + multiattach=true will unknowingly put themselves into a data loss situation. I think it's better to break that person's API request.
Ok, if multiattach=True in the request is always an unsuccessful case (or unknown successful sometimes) then I think changing it without microversion bump makes sense. But if we know there is any success case for xyz configuration/backend then I feel we should not break such success use case.
Thanks, Ghanshyam. An end user is setting themselves up for data loss if they rely on the request parameter rather than on using a volume type that explicitly supports multiattach. They could get lucky and not lose any data, but that's not really a success, so I think the best thing to do here is make this breaking change without a microversion.
I was just thinking from the Tempest test perspective which was
---- On Thu, 02 Mar 2023 19:17:32 -0800 Brian Rosmaita wrote --- old/not-recommended way to users parameter 'multiattach' from it can break the passing but as you corrected me in IRC,
the test does not check the data things so we do not completely test it in Tempest.
It's good that Tempest is there to keep us honest! I think what we can do to help out people whose scripts break is to return a specific error message explaining that the 'multiattach' element is not allowed in a volume-create request and instead the user should select a multiattach-capable volume type.
Thanks, Brian for explaining. This sounds good to me. Explaining the situation in release notes and error message will be really helpful for users.
I am +2 on the tempest change now - https://review.opendev.org/c/openstack/tempest/+/875372
-gmann
-gmann
cheers, brian