Open Stack

 ---- 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
 > >   >
 > >   >
 > >   >
 > 
 > 
 >