Thanks Alex for writing in details.

On Tue, Jan 24, 2017 at 6:05 PM, Alex Xu <> wrote:

> Unfortunately the device tag support in the API was broken in the old
> Microversion, 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

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

which seems failure on this -

I will check and fix functional tests bits.

> Thanks
> Alex
> [0]
> openstack/compute/
> [1]
> openstack/compute/schemas/
> [2]
> openstack/compute/schemas/
> [3]
> [4]
> nova/tests/unit/api/openstack/compute/
> [5]
> nova/tests/unit/api/openstack/compute/
> ​[6]



> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe:
OpenStack Development Mailing List (not for usage questions)

Reply via email to