Open Stack

2019年9月17日(火) 6:23 Eric Harney <eharney at redhat.com>:

> On 9/16/19 8:01 PM, Ghanshyam Mann wrote:
> >   ---- On Tue, 17 Sep 2019 02:40:36 +0900 Eric Harney <
> eharney at redhat.com> wrote ----
> >   > On 9/16/19 6:02 AM, Ghanshyam Mann wrote:
> >   > > ---- On Mon, 16 Sep 2019 18:53:58 +0900 Ghanshyam Mann <
> gmann at ghanshyammann.com> wrote ----
> >   > >   > Hello Everyone,
> >   > >   >
> >   > >   > As per discussion over ML, Tempest started the JSON schema
> strict validation for Volume  APIs response [1].
> >   > >   > Because it may affect the interop certification, it was
> explained to the Interop team as well as in the Board of Director
> meeting[2].
> >   > >   >
> >   > >   > In Train, Tempest started implementing the validation and
> found an API change where the new field was added in API response without
> versioning[3] (Cinder has API microversion mechanism). IMO, that was not
> the correct way to change the API and as per API-WG guidelines[4] any field
> added/modified/removed in API should be with microverison(means old
> versions/user should not be affected by that change) and must for API
> interoperability.
> >   > >   >
> >   > >   > With JSON schema validation, Tempest verifies the API
> interoperability recommended behaviour by API-WG.  But as per IRC
> conversion with cinder team, we have different opinion on API
> interoperability and how API should be changed under microversion
> mechanism.  I would like to have a conclusion on this so that Tempest can
> verify or leave the Volume API for strict validation.
> >   > >
> >   > > I found the same flow chart what Sean created in Nova about "when
> to bump microverison" in Cinder also which clearly say any addition to
> response need new microversion.
> >   > > -
> https://docs.openstack.org/cinder/latest/contributor/api_microversion_dev.html
> >   > >
> >   > > -gmann
> >   > >
> >   >
> >   > I don't believe that it is clear that a microversion bump was
> required
> >   > for the "groups" response showing up in a GET quota-sets response,
> and
> >   > here's why:
> >   >
> >   > This API has, since at least Havana, returned dynamic fields based on
> >   > quotas that are assigned to volume types.  i.e.:
> >   >
> >   > $ cinder --debug quota-show b73b1b7e82a247038cd01a441ec5a806
> >   > DEBUG:keystoneauth:RESP BODY: {"quota_set": {"per_volume_gigabytes":
> -1,
> >   > "volumes_ceph": -1, "groups": 10, "gigabytes": 1000,
> "backup_gigabytes":
> >   > 1000, "snapshots": 10, "volumes_enc": -1, "snapshots_enc": -1,
> >   > "snapshots_ceph": -1, "gigabytes_ceph": -1, "volumes": 10,
> >   > "gigabytes_enc": -1, "backups": 10, "id":
> >   > "b73b1b7e82a247038cd01a441ec5a806"}}
> >   >
> >   > "gigabytes_ceph" is in that response because there's a "ceph" volume
> >   > type defined, same for "gigabytes_enc", etc.
> >   >
> >   > This puts this API alongside something more like listing volume
> types --
> >   > you get a list of what's defined on the deployment, not a pre-baked
> list
> >   > of defined fields.
> >   >
> >   > Complaints about the fact that "groups" being added without a
> >   > microversion imply that these other dynamic fields shouldn't be in
> this
> >   > response either -- but this is how this API works.
> >   >
> >   > There's a lot of talk here about interoperability problems... what
> are
> >   > those problems, exactly?  If we ignore Ocata and just look at Train
> --
> >   > why is this API not problematic for interoperability there, when
> >   > requests on different clouds would return different data, depending
> on
> >   > how types are configured?
> >   >
> >   > It's not clear to me that rectifying the microversion concerns around
> >   > the "groups" field is helpful without also understanding this piece,
> >   > because if the concern is that different clouds return different
> fields
> >   > for this API -- that will still happen.  We need more detail to
> >   > understand how to address this, and what the problem is that we are
> >   > trying to solve exactly.
> >
> > There are two things here.
> > 1. API behaviour depends on backend. This has been discussed two years
> back also and Tempest team along with cinder team decided not to test the
> backend-specific behaviour in Tempest[1].
>
> This is wrong.  Nothing about what is happening in this API is
> backend-specific.
>
> > 2. API is changed without versioning.
> >
> > The second one is the issue here. If any API is changed without
> versioning cause the interoperability issue here. New field is being added
> for older microversion also for same backend.
> >
>
> If the concern is that different fields can be returned as part of quota
> info, it's worth understanding that fixing the Ocata tempest failures
> won't fix your concern, because this API still returns dynamic fields
> when the deployment is using per-type quotas, even on master.
>
> Are those considered "changes"?  Need concrete details here.
>

It is not difficult to answer this question.
The Cinder official API document[1] says the field "groups" is always
returned(not optional) in the API response.
IIUC the above dynamic fields are not written on the document, right?
In addition, Cinder implements microversions which clearly controls the
addition of API field as [2].
But actually the field "groups" has been added without bumping a
microversion against the above [2].
Then Ghanshyam raises the interoperability concern.

What is the interoperability concern?
The concern is if writing an application based on the API official
document, the application can work on newer clouds(Pike+ in this case) but
it cannot work on older clouds(Ocata-).
Actually we can consider Tempest is one of applications by consuming
OpenStack APIs and we implemented JSON-Schema validation based on the API
official document. Then Tempest could not work on older
clouds(stable/ocata) and it is clearly an interoperability issue.
(We Tempest reviewers read the API document carefully for reviewing
JSON-Schema patches, then we found Cinder document issues and fixed them :-)

BTW IMO backwards compatibility is more important than interoperability.
So we should not remove the field "groups" from the base microversion
anyways.

Thanks
Kenichi Omichi

---

[1]:
https://docs.openstack.org/api-ref/block-storage/v3/index.html?expanded=show-quotas-for-a-project-detail#show-quotas-for-a-project
[2]:
https://docs.openstack.org/cinder/latest/contributor/api_microversion_dev.html#when-do-i-need-a-new-microversion
(the part of " the list of attributes and data structures returned")
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20190917/f83685dd/attachment.html>