Open Stack

Hi Matthew,

Thanks for your response.

2014-04-28 11:02 GMT+09:00 Matthew Treinish <mtreinish at kortar.org>:
> On Mon, Apr 28, 2014 at 01:01:00AM +0000, Kenichi Oomichi wrote:
>>
>> Now we are working for adding Nova API responses checks to Tempest[1] to
>> block backward incompatible changes.
>> With this work, Tempest checks each response(status code, response body)
>> and raises a test failure exception if detecting something unexpected.
>> For example if some API parameter, which is defined as 'required' Tempest
>> side, does not exist in response body, Tempest test fails.
>>
>> We are defining API parameters as 'required' if they are not API extensions
>> or they are not depended on Nova configuration. In addition now Tempest
>> allows additional API parameters, that means Tempest does not fail even if
>> Nova response includes unexpected API parameters. Because I think the removal
>> of API parameter causes backward incompatible issue but the addition does not
>> cause it.
>
> So, AIUI we can only add parameters to an API with a new extension. The API
> change guidelines also say that adding new properties must be conditional:
>
> https://wiki.openstack.org/wiki/APIChangeGuidelines#Generally_Considered_OK
>
> Adding or removing a parameter to an API is a backwards incompatible change IMO
> for the exact reasons you mentioned here. If we have to worry about it in
> tempest then end users do as well.
>
> This is also why there are a bunch of nova v2 extensions that just add
> properties to an existing API. I think in v3 the proposal was to do this with
> microversioning of the plugins. (we don't have a way to configure
> microversioned v3 api plugins in tempest yet, but we can cross that bridge when
> the time comes) Either way it will allow tempest to have in config which
> behavior to expect.

Good point, my current understanding is:
When adding new API parameters to the existing APIs, these parameters should
be API extensions according to the above guidelines. So we have three options
for handling API extensions in Tempest:

1. Consider them as optional, and cannot block the incompatible
changes of them. (Current)
2. Consider them as required based on tempest.conf, and can block the
incompatible changes.
3. Consider them as required automatically with microversioning, and
can block the incompatible changes.

Now current implementation is the option 1 on the bp work, but I
prefer the option 3.
That would be smart way.

>> In this situation, there is a problem related to branchless Tempest.
>> When we define new API parameter as 'required', Tempest against old release
>> would fail.
>
> So I feel that if we are marking something in the API as required in tempest
> makes the test fail in a previous release than that should be considered a bug
> in the old release (assuming it was correct to mark it as required), and that
> should be backportable fix.

When I said the above my comment, I didn't consider the option you provided.
and the above 'required' change is just my bug;)

>> I think we need to define new parameters, which do not depended on the
>> configuration, as 'required' in Tempest when we have added them in the
>> development cycle because of blocking backward incompatible changes in
>> the future. However these parameters are new and old releases don't contain
>> them, so the Tempest change causes failures against old releases tests.
>
> I agree we should mark the required parameters as those that are used without
> any extensions. If we can also conditionally check those not marked as required
> based on the enabled extensions or features in the tempest config that would
> provide the best coverage.

+1

> So for master branch development on tempest I think we should only concern
> ourselves with getting these things to work against Juno and Icehouse. I think
> Havana support using master is a lost cause at this point so we'll keep the
> stable branch around until it's EOL. So hopefully we can lock down things in
> tempest with the new api attribute tests quickly so we can block the Juno
> additions that would violate the stability guidelines. It would be a shame if
> we managed to allow a breaking API change into an API since the release. (but
> hopefully it would be an easy backport or revert)

+1, nice direction.

>> Case: add new parameter 'A' in Juno cycle
>>
>>            Icehouse            Juno                K                   L
>>          --*-------------------*-------------------*-------------------*--
>>  Nova:        new parameter 'A'
>>  Tempest:         define 'A' as 'required'
>>                                                    block 'A' removal   block ..
>>            test fails due to non-existent 'A'
>
> So in this example I feel that parameter 'A' can only be added as an extension.
> (or some other condition) If it's not then it's a breaking api change which will
> be caught which is the desired behavior from tempest here.

Yes, right. Thanks for clarify it.

>> I have not found enough idea for this yet.

Thanks Matt,
I have gotten one idea for it!


Thanks
Ken'ichi Ohmichi