Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Artom Lifshitz]

The more urgent stuff has indeed merged - many thanks to Matt and
other cores for getting this in quickly before rc1. The fixes to tests
do indeed need more attention, which I will provide :)

On Mon, Jan 30, 2017 at 8:49 PM, Matt Riedemann  wrote:
> On 1/26/2017 8:32 PM, Artom Lifshitz wrote:
>>
>> Since the consensus is to fix this with a new microversion, I've
>> submitted some patches:
>>
>> * https://review.openstack.org/#/c/426030/
>>   A spec for the new microversion in case folks want one.
>
>
> Merged.
>
>>
>> * https://review.openstack.org/#/c/424759/
>>   The new microversion itself. I've already had feedback from Alex and
>> Ghanshyam (thanks guys!), and I've tried to address it.
>
>
> +2 from me, +1 from gmann. The Tempest patch for the 2.42 microversion is
> here:
>
> https://review.openstack.org/#/c/426991/1
>
>>
>> * https://review.openstack.org/#/c/425876/
>>   A patch to - as Alex and Sean suggested - stop passing plain string
>> version to the schema extension point.
>>
>
> Needs some work.
>
>
> --
>
> Thanks,
>
> Matt Riedemann
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



-- 
--
Artom Lifshitz

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Matt Riedemann]

On 1/26/2017 8:32 PM, Artom Lifshitz wrote:

Since the consensus is to fix this with a new microversion, I've
submitted some patches:

* https://review.openstack.org/#/c/426030/
  A spec for the new microversion in case folks want one.


Merged.



* https://review.openstack.org/#/c/424759/
  The new microversion itself. I've already had feedback from Alex and
Ghanshyam (thanks guys!), and I've tried to address it.


+2 from me, +1 from gmann. The Tempest patch for the 2.42 microversion 
is here:


https://review.openstack.org/#/c/426991/1



* https://review.openstack.org/#/c/425876/
  A patch to - as Alex and Sean suggested - stop passing plain string
version to the schema extension point.



Needs some work.

--

Thanks,

Matt Riedemann

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Artom Lifshitz]

Since the consensus is to fix this with a new microversion, I've
submitted some patches:

* https://review.openstack.org/#/c/426030/
  A spec for the new microversion in case folks want one.

* https://review.openstack.org/#/c/424759/
  The new microversion itself. I've already had feedback from Alex and
Ghanshyam (thanks guys!), and I've tried to address it.

* https://review.openstack.org/#/c/425876/
  A patch to - as Alex and Sean suggested - stop passing plain string
version to the schema extension point.

On Tue, Jan 24, 2017 at 10:38 PM, Matt Riedemann  wrote:
> On 1/24/2017 8:16 PM, Alex Xu wrote:
>>
>>
>>
>> One other thing: we're going to need to also fix this in
>> python-novaclient, which we might want to do first, or work
>> concurrently, since that's going to give us the client side
>> perspective on how gross it will be to deal with this issue.
>>
>>
>
> This is Andrey's patch to at least document the limitation:
>
> https://review.openstack.org/#/c/424745/
>
> We'll have to fix the client to use the new microversion in Pike (or at
> least release the fix in Pike) since the client release freeze is Thursday.
>
>
> --
>
> Thanks,
>
> Matt Riedemann
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



-- 
--
Artom Lifshitz

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Matt Riedemann]

On 1/24/2017 8:16 PM, Alex Xu wrote:



One other thing: we're going to need to also fix this in
python-novaclient, which we might want to do first, or work
concurrently, since that's going to give us the client side
perspective on how gross it will be to deal with this issue.




This is Andrey's patch to at least document the limitation:

https://review.openstack.org/#/c/424745/

We'll have to fix the client to use the new microversion in Pike (or at 
least release the fix in Pike) since the client release freeze is Thursday.


--

Thanks,

Matt Riedemann

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Alex Xu]

2017-01-25 0:27 GMT+08:00 Matt Riedemann :

> On 1/24/2017 9:18 AM, Matt Riedemann wrote:
>
>>
>> First, thanks to Kevin and Alex for finding this issue and explaining it
>> in detail so we can understand the scope.
>>
>> This is a nasty unfortunate issue which I really wish we could just fix
>> without a microversion bump but we have microversions for a reason,
>> which is to fix issues in the API. In thinking about if this were the
>> legacy 2.0 API, we always had a rule that you couldn't fix bugs in the
>> API if they changed the behavior, no matter how annoying.
>>
>> So let's fix this with a microversion. I don't think we need to hold it
>> to the feature freeze deadline as it's a microversion only for a bug
>> fix, it's not a new feature. So that's a compromise at least and gives
>> us some time to get this done correctly and still have it fixed in
>> Ocata. We'll also want to document this in the api-ref and REST API
>> version history in whatever way makes it clear about the limitations
>> between microversions.
>>
>> As for testing, I think using a mix of test inheritance and using
>> 2.latest is probably a good step to take. I know we've had a mix of that
>> in different places in the functional API samples tests, but there was
>> never a clear rule about what do to with testing microversions and if
>> you should use inheritance to build on existing tests.
>>
>>
> One other thing: we're going to need to also fix this in
> python-novaclient, which we might want to do first, or work concurrently,
> since that's going to give us the client side perspective on how gross it
> will be to deal with this issue.


+1, thanks for this good point!


>
>
> --
>
> Thanks,
>
> Matt Riedemann
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Zhenyu Zheng]

Thanks Alex for raising this up widely, as Chinese holiday is comming and
Alex and me might be away for a week, And it will be better to fix this
faster, so thanks Artom taking over to fix it :)

On Wed, Jan 25, 2017 at 7:50 AM, Ghanshyam Mann 
wrote:

>
> On Wed, Jan 25, 2017 at 1:18 AM, Matt Riedemann 
> wrote:
>
>> On 1/24/2017 2:05 AM, Alex Xu 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.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>>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?
>>>
>>> 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/openstac
>>> k/compute/schemas/servers.py@88
>>> [2] https://review.openstack.org/#/c/316398/37/nova/api/openstac
>>> k/compute/schemas/servers.py@79
>>> [3] https://github.com/openstack/nova/blob/master/nova/api/opens
>>> tack/api_version_request.py#L219
>>> [4] https://github.com/openstack/nova/blob/master/nova/tests/uni
>>> t/api/openstack/compute/test_evacuate.py#L415
>>> [5] https://github.com/openstack/nova/blob/master/nova/tests/uni
>>> t/api/openstack/compute/test_serversV21.py#L3584
>>>
>>>
>>>
>>> 
>>> __
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe: openstack-dev-requ...@lists.op
>>> enstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>> First, thanks to Kevin and Alex for finding this issue and explaining it
>> in detail so we can understand the scope.
>>
>> This is a nasty unfortunate issue which I really wish we could just fix
>> without a microversion bump but we have microversions for a reason, which
>> is to fix issues in the API. In thinking about if this were the legacy 2.0
>> API, we always had a rule that you couldn't fix bugs in the API if they
>> changed the behavior, no matter how annoying.
>>
>> So let's fix this with a microversion. I don't think we need to hold it
>> to the feature freeze deadline as it's a microversion only for a bug fix,
>> it's not a new feature. So that's a compromise at least and gives us some
>> time to get this done correctly and still have it fixed in Ocata. We'll
>> also want to document this in the api-ref and REST API version history in
>> whatever way makes it clear about the limitations between microversions.
>>
>
> ​+1 for fixing in Ocata itself. We have fix up just need to put that under
> new version. I can modify the tests to cover this bug scenario. ​
>
>
>
>>

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Ghanshyam Mann]

On Wed, Jan 25, 2017 at 1:18 AM, Matt Riedemann  wrote:

> On 1/24/2017 2:05 AM, Alex Xu 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.
>>
>> 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.
>>
>> 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.
>>
>>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?
>>
>> 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/openstac
>> k/compute/schemas/servers.py@88
>> [2] https://review.openstack.org/#/c/316398/37/nova/api/openstac
>> k/compute/schemas/servers.py@79
>> [3] https://github.com/openstack/nova/blob/master/nova/api/opens
>> tack/api_version_request.py#L219
>> [4] https://github.com/openstack/nova/blob/master/nova/tests/uni
>> t/api/openstack/compute/test_evacuate.py#L415
>> [5] https://github.com/openstack/nova/blob/master/nova/tests/uni
>> t/api/openstack/compute/test_serversV21.py#L3584
>>
>>
>>
>> 
>> __
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscrib
>> e
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
> First, thanks to Kevin and Alex for finding this issue and explaining it
> in detail so we can understand the scope.
>
> This is a nasty unfortunate issue which I really wish we could just fix
> without a microversion bump but we have microversions for a reason, which
> is to fix issues in the API. In thinking about if this were the legacy 2.0
> API, we always had a rule that you couldn't fix bugs in the API if they
> changed the behavior, no matter how annoying.
>
> So let's fix this with a microversion. I don't think we need to hold it to
> the feature freeze deadline as it's a microversion only for a bug fix, it's
> not a new feature. So that's a compromise at least and gives us some time
> to get this done correctly and still have it fixed in Ocata. We'll also
> want to document this in the api-ref and REST API version history in
> whatever way makes it clear about the limitations between microversions.
>

​+1 for fixing in Ocata itself. We have fix up just need to put that under
new version. I can modify the tests to cover this bug scenario. ​



>
> As for testing, I think using a mix of test inheritance and using 2.latest
> is probably a good step to take. I know we've had a mix of that in
> different places in the functional API samples tests, but there was never a
> clear rule about what do to with testing microversions and if you should
> use inheritance to build on existing tests.

​
Also along with that, I'd like to add 

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Matt Riedemann]

On 1/24/2017 9:18 AM, Matt Riedemann wrote:


First, thanks to Kevin and Alex for finding this issue and explaining it
in detail so we can understand the scope.

This is a nasty unfortunate issue which I really wish we could just fix
without a microversion bump but we have microversions for a reason,
which is to fix issues in the API. In thinking about if this were the
legacy 2.0 API, we always had a rule that you couldn't fix bugs in the
API if they changed the behavior, no matter how annoying.

So let's fix this with a microversion. I don't think we need to hold it
to the feature freeze deadline as it's a microversion only for a bug
fix, it's not a new feature. So that's a compromise at least and gives
us some time to get this done correctly and still have it fixed in
Ocata. We'll also want to document this in the api-ref and REST API
version history in whatever way makes it clear about the limitations
between microversions.

As for testing, I think using a mix of test inheritance and using
2.latest is probably a good step to take. I know we've had a mix of that
in different places in the functional API samples tests, but there was
never a clear rule about what do to with testing microversions and if
you should use inheritance to build on existing tests.



One other thing: we're going to need to also fix this in 
python-novaclient, which we might want to do first, or work 
concurrently, since that's going to give us the client side perspective 
on how gross it will be to deal with this issue.


--

Thanks,

Matt Riedemann

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Matt Riedemann]

On 1/24/2017 2:05 AM, Alex Xu 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.

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.

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.

   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?

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@88
[2] 
https://review.openstack.org/#/c/316398/37/nova/api/openstack/compute/schemas/servers.py@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



__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



First, thanks to Kevin and Alex for finding this issue and explaining it 
in detail so we can understand the scope.


This is a nasty unfortunate issue which I really wish we could just fix 
without a microversion bump but we have microversions for a reason, 
which is to fix issues in the API. In thinking about if this were the 
legacy 2.0 API, we always had a rule that you couldn't fix bugs in the 
API if they changed the behavior, no matter how annoying.


So let's fix this with a microversion. I don't think we need to hold it 
to the feature freeze deadline as it's a microversion only for a bug 
fix, it's not a new feature. So that's a compromise at least and gives 
us some time to get this done correctly and still have it fixed in 
Ocata. We'll also want to document this in the api-ref and REST API 
version history in whatever way makes it clear about the limitations 
between microversions.


As for testing, I think using a mix of test inheritance and using 
2.latest is probably a good step to take. I know we've had a mix of that 
in different places in the functional API samples tests, but there was 
never a clear rule about what do to with testing microversions and if 
you should use inheritance to build on existing tests.


--

Thanks,

Matt Riedemann

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Matt Riedemann]

On 1/24/2017 3:34 AM, Ghanshyam Mann wrote:


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.



The server tags and device tags in the DB are using different length 
columns, but they have a consistent schema in the REST API don't they? 
The REST API restricts the tags to 60 characters so the backing DB model 
doesn't really matter (and an API user shouldn't need to know or care 
what the data model is).


--

Thanks,

Matt Riedemann

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Artom Lifshitz]

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

Thanks for the summary. For the visual minded like me, I made some
ASCII art of the above:

http://paste.openstack.org/raw/596225/

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

I don't think we have a choice - we'd be adding new API parameters
that didn't exist in, for example, 2.39.

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

I think it's too late before FF to do any functional fixes. I vote we
document our screw up in the api-ref at least, and during Pike we can
merge a new microversion that fixes this mess.

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

This looks like a smart way to make sure all API version comparisons
are of the less than/greater than kind.

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

An idea would be to run all functional tests against 2.latest. This
doesn't cover all microversions, but since as time progresses and
2.latest increases, all previous microversions will have been covered
in the past, and it gives us some confidence that we didn't break
anything. This doesn't work for patches that removed an API parameter
for example, so those kinds of changes will have to be an exception.

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Ghanshyam Mann]

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 
wrote:

> 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 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@88
>> [2] https://review.openstack.org/#/c/316398/37/nova/api/open
>> stack/compute/schemas/servers.py@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] 

Re: [openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Ghanshyam Mann]

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 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@88
> [2] https://review.openstack.org/#/c/316398/37/nova/api/
> openstack/compute/schemas/servers.py@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 

[openstack-dev] [nova] Device tag in the API breaks in the old microversion

[Alex Xu]

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.

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.

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.

   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?

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@88
[2]
https://review.openstack.org/#/c/316398/37/nova/api/openstack/compute/schemas/servers.py@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
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev