Open Stack

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/
> openstack/compute/block_device_mapping.py
> [1] https://review.openstack.org/#/c/316398/37/nova/api/
> openstack/compute/schemas/servers.py at 88
> [2] https://review.openstack.org/#/c/316398/37/nova/api/
> openstack/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:unsubscribe
> 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/81350db7/attachment.html>