[kolla] [train] [cinder] Volume multiattach exposed to non-admin users via API
According to this document [1] multiattach volumes can only be setup if explicitly allowed by creating a “multiattach” volume type.
“Starting from the Queens release the ability to attach a volume to multiple hosts/servers requires that the volume is of a special type that includes an extra-spec capability setting of multiattach=<is> True… Creating a new volume type is an admin-only operation by default.
One of our customers appears to have used TerraForm to create a volume with the multiattach flag set and it worked, and that volume has multiple attachments. When I look here [2] it appears that the default is:
#"volume:multiattach": "rule:xena_system_admin_or_project_member"
So it looks like, by default, any project member can create a multiattach volume. What am I missing?
[1]: https://docs.openstack.org/cinder/latest/admin/volume-multiattach.html [2]: https://docs.openstack.org/cinder/latest/configuration/block-storage/samples...
Creating a volume is not the same as creating a volume type. A tenant can consume a volume type that allows multi-attach with no issue as you see in that policy. ________________________________ From: Albert Braden ozzzo@yahoo.com Sent: 22 February 2023 17:12 To: Openstack-discuss openstack-discuss@lists.openstack.org Subject: [kolla] [train] [cinder] Volume multiattach exposed to non-admin users via API
CAUTION: This email originates from outside THG
According to this document [1] multiattach volumes can only be setup if explicitly allowed by creating a “multiattach” volume type.
“Starting from the Queens release the ability to attach a volume to multiple hosts/servers requires that the volume is of a special type that includes an extra-spec capability setting of multiattach=<is> True… Creating a new volume type is an admin-only operation by default.
One of our customers appears to have used TerraForm to create a volume with the multiattach flag set and it worked, and that volume has multiple attachments. When I look here [2] it appears that the default is:
#"volume:multiattach": "rule:xena_system_admin_or_project_member"
So it looks like, by default, any project member can create a multiattach volume. What am I missing?
[1]: https://docs.openstack.org/cinder/latest/admin/volume-multiattach.htmlhttps://docs.openstack.org/cinder/latest/admin/volume-multiattach.html [2]: https://docs.openstack.org/cinder/latest/configuration/block-storage/samples...https://docs.openstack.org/cinder/latest/configuration/block-storage/samples/policy.yaml.html#policy-file
Danny Webb Principal OpenStack Engineer Danny.Webb@thehutgroup.com [THG Ingenuity Logo] www.thg.comhttps://www.thg.com [https://i.imgur.com/wbpVRW6.png]https://www.linkedin.com/company/thg-ingenuity/?originalSubdomain=uk [https://i.imgur.com/c3040tr.png] https://twitter.com/thgingenuity?lang=en
We didn't create a multi-attach volume type, and when we try to create a multi-attach volume via CLI we aren't able to. It appears that our customer was able to circumvent the restriction by using the API via TF. Is this a bug? On Wednesday, February 22, 2023, 02:32:57 PM EST, Danny Webb danny.webb@thehutgroup.com wrote:
#yiv9135123901 P {margin-top:0;margin-bottom:0;}Creating a volume is not the same as creating a volume type. A tenant can consume a volume type that allows multi-attach with no issue as you see in that policy. From: Albert Braden ozzzo@yahoo.com Sent: 22 February 2023 17:12 To: Openstack-discuss openstack-discuss@lists.openstack.org Subject: [kolla] [train] [cinder] Volume multiattach exposed to non-admin users via API CAUTION: This email originates from outside THG
According to this document [1] multiattach volumes can only be setup if explicitly allowed by creating a “multiattach” volume type.
“Starting from the Queens release the ability to attach a volume to multiple hosts/servers requires that the volume is of a special type that includes an extra-spec capability setting of multiattach=<is> True… Creating a new volume type is an admin-only operation by default.
One of our customers appears to have used TerraForm to create a volume with the multiattach flag set and it worked, and that volume has multiple attachments. When I look here [2] it appears that the default is:
#"volume:multiattach": "rule:xena_system_admin_or_project_member"
So it looks like, by default, any project member can create a multiattach volume. What am I missing?
[1]: https://docs.openstack.org/cinder/latest/admin/volume-multiattach.html [2]: https://docs.openstack.org/cinder/latest/configuration/block-storage/samples...
| | | Danny Webb | | Principal OpenStack Engineer | | Danny.Webb@thehutgroup.com | | | | | | www.thg.com | | |
Hi,
It looks like there is a confusion between 3 things 1) Multiattach volume type 2) multiattach flag on the volume 3) The policy volume:multiattach
I will try to briefly describe all of the 3 so there is clarity on the issue. 1) Multiattach volume type: This is a volume type created with an extra spec *multiattach="<is> True"*. This allows multiattach volumes to be created by using this type. Previously we used to allow a parameter *--allow-multiattach* while creating the volume. This was deprecated in Queens and removed in Train in favor of the volume type way of creating the multiattach volume[1]. 2) Multiattach flag of a volume: This is a parameter of volume that specifies if a volume is multiattach or not. 3) volume:multiattach policy: The policy verifies if the user creating a multiattach volume is *member* or *admin* (and not *reader*).
Coming to the issue, I verified that what you're observing is correct. We removed the support for providing the "multiattach" flag from cinderclient and openstackclient but there still exists code on the API side that allows you to provide "multiattach": "True" in the JSON body of a curl command to create a multiattach volume. I will work on fixing the issue on the API side. In the meantime, can you report an issue on launchpad for the same?
https://bugs.launchpad.net/cinder/+filebug
*Snippet of curl command* $ curl -g -i -X POST http://127.0.0.1/volume/v3/a5df9e29f521464f9158ff7a30b7e51f/volumes -H "Accept: application/json" -H "Content-Type: application/json" -H "User-Agent: python-cinderclient" -H "X-Auth-Token: gAAAAABj9zDtZO1mTld-BC-Yd8FRHDunc4-Xyg1jsgLembA-Ke7cr8aA4kCHHYYB4EPvhq1xL02FBYuXahhYBl_nKWjVbOTpd7R3kS4Libf-Kd9ackaYpWq4Mq4g7-2ORi7FcVg2IOdj3wUkDWegu9lI5PI-brNsAGUh8R1fW_y5bpDYWtfEFdw" -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 202 Accepted Date: Thu, 23 Feb 2023 09:25:23 GMT Server: Apache/2.4.41 (Ubuntu) Content-Type: application/json x-compute-request-id: req-131b4a2d-f9d4-4d9d-b99c-c52012056dec Content-Length: 798 OpenStack-API-Version: volume 3.0 Vary: OpenStack-API-Version x-openstack-request-id: req-131b4a2d-f9d4-4d9d-b99c-c52012056dec Connection: close
[1] https://github.com/openstack/python-cinderclient/commit/3c1b417959689c85a2f5...
Thanks Rajat Dhasmana
On Thu, Feb 23, 2023 at 3:08 AM Albert Braden ozzzo@yahoo.com wrote:
We didn't create a multi-attach volume type, and when we try to create a multi-attach volume via CLI we aren't able to. It appears that our customer was able to circumvent the restriction by using the API via TF. Is this a bug? On Wednesday, February 22, 2023, 02:32:57 PM EST, Danny Webb < danny.webb@thehutgroup.com> wrote:
Creating a volume is not the same as creating a volume type. A tenant can consume a volume type that allows multi-attach with no issue as you see in that policy.
*From:* Albert Braden ozzzo@yahoo.com *Sent:* 22 February 2023 17:12 *To:* Openstack-discuss openstack-discuss@lists.openstack.org *Subject:* [kolla] [train] [cinder] Volume multiattach exposed to non-admin users via API
CAUTION: This email originates from outside THG
According to this document [1] multiattach volumes can only be setup if explicitly allowed by creating a “multiattach” volume type.
“Starting from the Queens release the ability to attach a volume to multiple hosts/servers requires that the volume is of a special type that includes an extra-spec capability setting of multiattach=<is> True… Creating a new volume type is an admin-only operation by default.
One of our customers appears to have used TerraForm to create a volume with the multiattach flag set and it worked, and that volume has multiple attachments. When I look here [2] it appears that the default is:
#"volume:multiattach": "rule:xena_system_admin_or_project_member"
So it looks like, by default, any project member can create a multiattach volume. What am I missing?
*Danny Webb* Principal OpenStack Engineer Danny.Webb@thehutgroup.com [image: THG Ingenuity Logo] www.thg.com https://www.linkedin.com/company/thg-ingenuity/?originalSubdomain=uk https://twitter.com/thgingenuity?lang=en
Opened https://bugs.launchpad.net/cinder/+bug/2008259 On Thursday, February 23, 2023, 04:41:10 AM EST, Rajat Dhasmana rdhasman@redhat.com wrote:
Hi, It looks like there is a confusion between 3 things1) Multiattach volume type2) multiattach flag on the volume3) The policy volume:multiattach I will try to briefly describe all of the 3 so there is clarity on the issue.1) Multiattach volume type: This is a volume type created with an extra spec multiattach="<is> True". This allows multiattach volumes to be created by using this type.Previously we used to allow a parameter --allow-multiattach while creating the volume. This was deprecated in Queens and removed in Train in favor of the volume type way of creating the multiattach volume[1].2) Multiattach flag of a volume: This is a parameter of volume that specifies if a volume is multiattach or not.3) volume:multiattach policy: The policy verifies if the user creating a multiattach volume is member or admin (and not reader). Coming to the issue, I verified that what you're observing is correct. We removed the support for providing the "multiattach" flag from cinderclient and openstackclient but there still exists code on the API side that allows you to provide "multiattach": "True" in the JSON body of a curl command to create a multiattach volume.I will work on fixing the issue on the API side. In the meantime, can you report an issue on launchpad for the same? https://bugs.launchpad.net/cinder/+filebug
Snippet of curl command$ curl -g -i -X POST http://127.0.0.1/volume/v3/a5df9e29f521464f9158ff7a30b7e51f/volumes -H "Accept: application/json" -H "Content-Type: application/json" -H "User-Agent: python-cinderclient" -H "X-Auth-Token: gAAAAABj9zDtZO1mTld-BC-Yd8FRHDunc4-Xyg1jsgLembA-Ke7cr8aA4kCHHYYB4EPvhq1xL02FBYuXahhYBl_nKWjVbOTpd7R3kS4Libf-Kd9ackaYpWq4Mq4g7-2ORi7FcVg2IOdj3wUkDWegu9lI5PI-brNsAGUh8R1fW_y5bpDYWtfEFdw" -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 202 Accepted Date: Thu, 23 Feb 2023 09:25:23 GMT Server: Apache/2.4.41 (Ubuntu) Content-Type: application/json x-compute-request-id: req-131b4a2d-f9d4-4d9d-b99c-c52012056dec Content-Length: 798 OpenStack-API-Version: volume 3.0 Vary: OpenStack-API-Version x-openstack-request-id: req-131b4a2d-f9d4-4d9d-b99c-c52012056dec Connection: close
[1] https://github.com/openstack/python-cinderclient/commit/3c1b417959689c85a2f5... ThanksRajat Dhasmana On Thu, Feb 23, 2023 at 3:08 AM Albert Braden ozzzo@yahoo.com wrote:
We didn't create a multi-attach volume type, and when we try to create a multi-attach volume via CLI we aren't able to. It appears that our customer was able to circumvent the restriction by using the API via TF. Is this a bug? On Wednesday, February 22, 2023, 02:32:57 PM EST, Danny Webb danny.webb@thehutgroup.com wrote:
Creating a volume is not the same as creating a volume type. A tenant can consume a volume type that allows multi-attach with no issue as you see in that policy. From: Albert Braden ozzzo@yahoo.com Sent: 22 February 2023 17:12 To: Openstack-discuss openstack-discuss@lists.openstack.org Subject: [kolla] [train] [cinder] Volume multiattach exposed to non-admin users via API CAUTION: This email originates from outside THG
According to this document [1] multiattach volumes can only be setup if explicitly allowed by creating a “multiattach” volume type.
“Starting from the Queens release the ability to attach a volume to multiple hosts/servers requires that the volume is of a special type that includes an extra-spec capability setting of multiattach=<is> True… Creating a new volume type is an admin-only operation by default.
One of our customers appears to have used TerraForm to create a volume with the multiattach flag set and it worked, and that volume has multiple attachments. When I look here [2] it appears that the default is:
#"volume:multiattach": "rule:xena_system_admin_or_project_member"
So it looks like, by default, any project member can create a multiattach volume. What am I missing?
[1]: https://docs.openstack.org/cinder/latest/admin/volume-multiattach.html [2]: https://docs.openstack.org/cinder/latest/configuration/block-storage/samples...
| | | Danny Webb | | Principal OpenStack Engineer | | Danny.Webb@thehutgroup.com | | | | | | www.thg.com | | |
---- On Thu, 23 Feb 2023 01:33:12 -0800 Rajat Dhasmana wrote ---
Hi, It looks like there is a confusion between 3 things1) Multiattach volume type2) multiattach flag on the volume3) The policy volume:multiattach I will try to briefly describe all of the 3 so there is clarity on the issue.1) Multiattach volume type: This is a volume type created with an extra spec multiattach=" True". This allows multiattach volumes to be created by using this type.Previously we used to allow a parameter --allow-multiattach while creating the volume. This was deprecated in Queens and removed in Train in favor of the volume type way of creating the multiattach volume[1].2) Multiattach flag of a volume: This is a parameter of volume that specifies if a volume is multiattach or not.3) volume:multiattach policy: The policy verifies if the user creating a multiattach volume is member or admin (and not reader). Coming to the issue, I verified that what you're observing is correct. We removed the support for providing the "multiattach" flag from cinderclient and openstackclient but there still exists code on the API side that allows you to provide "multiattach": "True" in the JSON body of a curl command to create a multiattach volume.I will work on fixing the issue on the API side.
I think removing from client is good way to stop exposing this old/not-recommended way to users but API is separate things and removing the API request parameter 'multiattach' from it can break 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.
Similar way in Nova we have lot of deprecated API and we need to keep them for older microversions. -gmann
In the meantime, can you report an issue on launchpad for the same?
https://bugs.launchpad.net/cinder/+filebug
Snippet of curl command$ curl -g -i -X POST http://127.0.0.1/volume/v3/a5df9e29f521464f9158ff7a30b7e51f/volumes -H "Accept: application/json" -H "Content-Type: application/json" -H "User-Agent: python-cinderclient" -H "X-Auth-Token: gAAAAABj9zDtZO1mTld-BC-Yd8FRHDunc4-Xyg1jsgLembA-Ke7cr8aA4kCHHYYB4EPvhq1xL02FBYuXahhYBl_nKWjVbOTpd7R3kS4Libf-Kd9ackaYpWq4Mq4g7-2ORi7FcVg2IOdj3wUkDWegu9lI5PI-brNsAGUh8R1fW_y5bpDYWtfEFdw" -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 202 Accepted Date: Thu, 23 Feb 2023 09:25:23 GMT Server: Apache/2.4.41 (Ubuntu) Content-Type: application/json x-compute-request-id: req-131b4a2d-f9d4-4d9d-b99c-c52012056dec Content-Length: 798 OpenStack-API-Version: volume 3.0 Vary: OpenStack-API-Version x-openstack-request-id: req-131b4a2d-f9d4-4d9d-b99c-c52012056dec Connection: close
[1] https://github.com/openstack/python-cinderclient/commit/3c1b417959689c85a2f5... ThanksRajat Dhasmana On Thu, Feb 23, 2023 at 3:08 AM Albert Braden ozzzo@yahoo.com> wrote: We didn't create a multi-attach volume type, and when we try to create a multi-attach volume via CLI we aren't able to. It appears that our customer was able to circumvent the restriction by using the API via TF. Is this a bug? On Wednesday, February 22, 2023, 02:32:57 PM EST, Danny Webb danny.webb@thehutgroup.com> wrote:
Creating a volume is not the same as creating a volume type. A tenant can consume a volume type that allows multi-attach with no issue as you see in that policy.From: Albert Braden ozzzo@yahoo.com> Sent: 22 February 2023 17:12 To: Openstack-discuss openstack-discuss@lists.openstack.org> Subject: [kolla] [train] [cinder] Volume multiattach exposed to non-admin users via API CAUTION: This email originates from outside THG
According to this document [1] multiattach volumes can only be setup if explicitly allowed by creating a “multiattach” volume type.
“Starting from the Queens release the ability to attach a volume to multiple hosts/servers requires that the volume is of a special type that includes an extra-spec capability setting of multiattach= True… Creating a new volume type is an admin-only operation by default.
One of our customers appears to have used TerraForm to create a volume with the multiattach flag set and it worked, and that volume has multiple attachments. When I look here [2] it appears that the default is:
#"volume:multiattach": "rule:xena_system_admin_or_project_member"
So it looks like, by default, any project member can create a multiattach volume. What am I missing?
Danny Webb Principal OpenStack Engineer Danny.Webb@thehutgroup.com
www.thg.com
On 2/28/23 9:02 PM, Ghanshyam Mann wrote: [snip]
I think removing from client is good way to stop exposing this old/not-recommended way to users but API is separate things and removing the API request parameter 'multiattach' from it can break 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 the 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.
cheers, brian
---- 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 old/not-recommended way to users but API is separate things and removing the API request parameter 'multiattach' from it can break 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 the 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.
I was just thinking from the Tempest test perspective which was 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.
-gmann
cheers, brian
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 old/not-recommended way to users but API is separate things and removing the API request parameter 'multiattach' from it can break 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 the 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 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.
-gmann
cheers, brian
---- On Thu, 02 Mar 2023 19:17:32 -0800 Brian Rosmaita 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 old/not-recommended way to users but API is separate things and removing the API request parameter 'multiattach' from it can break 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 the 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 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
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 Thu, 02 Mar 2023 19:17:32 -0800 Brian Rosmaita 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
old/not-recommended way to users
but API is separate things and removing the API request
parameter 'multiattach' from it can break
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
the
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
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
participants (5)
-
Albert Braden
-
Brian Rosmaita
-
Danny Webb
-
Ghanshyam Mann
-
Rajat Dhasmana