Open Stack

Sorry, forgot to mention tempest patch -
https://review.openstack.org/#/c/424572/1 (but broken cases seems clear
without that patch also)

Regards
Ghanshyam Mann
+818011120698

On Tue, Jan 24, 2017 at 7:34 PM, Ghanshyam Mann <ghanshyammann at gmail.com>
wrote:

> Thanks Alex for writing in details.
>
>
> On Tue, Jan 24, 2017 at 6:05 PM, Alex Xu <soulxu at gmail.com> wrote:
>
>> Unfortunately the device tag support in the API was broken in the old
>> Microversion https://bugs.launchpad.net/nova/+bug/1658571, which thanks
>> to Kevin Zheng to find out that.
>>
>> Actually there are two bugs, just all of them are about device tag. The
>> first one [0] is a mistake in the initial introduce of device tag. The new
>> schema only available for the version 2.32, when the request version >
>> 2.32, the schema fallback to the old one.
>>
>> The second one [1] is that when we bump the API to 2.37, the network
>> device tag was removed accidentally which also added in 2.32 [2].
>>
>> So the current API behavior is as below:
>>
>> 2.32: BDM tag and network device tag added.
>> 2.33 - 2.36: 'tag' in the BDM disappeared. The network device tag still
>> works.
>> 2.37: The network device tag disappeared also.
>>
>
>
> ​I am modifying the tempest tests in (feel free to modify it for more
> verification) and running all above cases combination with ​version, 2.32,
> 2.33 and >2.37 and we should be able to verify what we broke actually from
> tests results.
>
> But from looking at api-ref and db, i am little bit confused. API-ref says
> device tag as "An arbitrary tag." [6] and DB also have it as
> Column(String(255)) [7]
>
> But our tag validation we mistakenly removed is more strict actually as
> per [8] which seems good so we should fix/mention the same in DB and
> api-ref clearly.
>
>
>
>>
>> There are few questions we should think about:
>>
>> 1. Should we fix that by Microversion?
>>     Thanks to Chris Dent point that out in the review. I also think we
>> need to bump Microversion, which follow the rule of Microversion.
>>
>
> Initially i thought it only validation part we loosen and feature still
> works but with additionalPropoerties=False, its all disallowed to add tag
> :(.
>
>
>
>>
>> 2. If we need Microversion, is that something we can do before release?
>>     We are very close to the feature freeze. And in normal, we need spec
>> for microversion. Maybe we only can do that in Pike. For now we can update
>> the API-ref, and microversion history to notice that, maybe a reno also.
>>
>> 2. How can we prevent that happened again?
>>    Both of those patches were reviewed multiple cycles. But we still miss
>> that. It is worth to think about how to prevent that happened again.
>>
>
> ​This is very nice point. Actually we planned a BP [9] to cover the (Top,
> Bottom & Changed) testing of each microversion, but my bad here as i did
> not get time to work on that. May be this BP can be helpful here and add to
> verify those tests in review guidelines.
>
>
>
>>
>>    Talk with Sean. He suggests stop passing plain string version to the
>> schema extension point. We should always pass APIVersionRequest object
>> instead of plain string. Due to "version == APIVersionRequest('2.32')" is
>> always wrong, we should remove the '__eq__'. The developer should always
>> use the 'APIVersionRequest.matches' [3] method.
>>
>>    That can prevent the first mistake we made. But nothing help for
>> second mistake. Currently we only run the test on the specific Microversion
>> for the specific interesting point. In the before, the new tests always
>> inherits from the previous microversion tests, just like [4]. That can test
>> the old API behavior won't be changed in the new microversion. But now, we
>> said that is waste, we didn't do that again just like [5]. Should we change
>> that back?
>>
>
> ​Another issue we have in functional tests for device tagging is we did
> not make "supports_device_tagging" as true in fake driver and it was
> something never got worked in https://github.com/openstack/
> nova/blob/master/nova/compute/manager.py#L1730​
>
> which seems failure on this - http://logs.openstack.org/
> 52/423952/5/check/gate-nova-tox-db-functional-ubuntu-xenial/ff1c09a/
> I will check and fix functional tests bits.
>
>
>
>>
>> Thanks
>> Alex
>>
>> [0] https://review.openstack.org/#/c/304510/64/nova/api/openstac
>> k/compute/block_device_mapping.py
>> [1] https://review.openstack.org/#/c/316398/37/nova/api/open
>> stack/compute/schemas/servers.py at 88
>> [2] https://review.openstack.org/#/c/316398/37/nova/api/open
>> stack/compute/schemas/servers.py at 79
>> [3] https://github.com/openstack/nova/blob/master/nova/api/
>> openstack/api_version_request.py#L219
>> [4] https://github.com/openstack/nova/blob/master/nova/
>> tests/unit/api/openstack/compute/test_evacuate.py#L415
>> [5] https://github.com/openstack/nova/blob/master/nova/
>> tests/unit/api/openstack/compute/test_serversV21.py#L3584
>>
>> ​[6] http://developer.openstack.org/api-ref/compute/?expanded=
> create-server-detail​
> [7] https://github.com/openstack/nova/blob/master/
> nova/db/sqlalchemy/models.py#L624
> ​[8] http://developer.openstack.org/api-ref/compute/#server-
> tags-servers-tags
> [9] https://blueprints.launchpad.net/nova/+spec/nova-
> microversion-functional-tests ​
>
> ​​
>
>> ____________________________________________________________
>> ______________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscrib
>> e
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20170124/31bcac54/attachment.html>