<div dir="ltr">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 :)</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 25, 2017 at 7:50 AM, Ghanshyam Mann <span dir="ltr"><<a href="mailto:ghanshyammann@gmail.com" target="_blank">ghanshyammann@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace;color:#000000"><br></div><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Wed, Jan 25, 2017 at 1:18 AM, Matt Riedemann <span dir="ltr"><<a href="mailto:mriedemos@gmail.com" target="_blank">mriedemos@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 1/24/2017 2:05 AM, Alex Xu wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Unfortunately the device tag support in the API was broken in the old<br>
Microversion <a href="https://bugs.launchpad.net/nova/+bug/1658571" rel="noreferrer" target="_blank">https://bugs.launchpad.net/nov<wbr>a/+bug/1658571</a>, which thanks<br>
to Kevin Zheng to find out that.<br>
<br>
Actually there are two bugs, just all of them are about device tag. The<br>
first one [0] is a mistake in the initial introduce of device tag. The<br>
new schema only available for the version 2.32, when the request version<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2.32, the schema fallback to the old one.<br>
</blockquote>
<br>
The second one [1] is that when we bump the API to 2.37, the network<br>
device tag was removed accidentally which also added in 2.32 [2].<br>
<br>
So the current API behavior is as below:<br>
<br>
2.32: BDM tag and network device tag added.<br>
2.33 - 2.36: 'tag' in the BDM disappeared. The network device tag still<br>
works.<br>
2.37: The network device tag disappeared also.<br>
<br>
There are few questions we should think about:<br>
<br>
1. Should we fix that by Microversion?<br>
Thanks to Chris Dent point that out in the review. I also think we<br>
need to bump Microversion, which follow the rule of Microversion.<br>
<br>
2. If we need Microversion, is that something we can do before release?<br>
We are very close to the feature freeze. And in normal, we need spec<br>
for microversion. Maybe we only can do that in Pike. For now we can<br>
update the API-ref, and microversion history to notice that, maybe a<br>
reno also.<br>
<br>
2. How can we prevent that happened again?<br>
Both of those patches were reviewed multiple cycles. But we still<br>
miss that. It is worth to think about how to prevent that happened again.<br>
<br>
Talk with Sean. He suggests stop passing plain string version to the<br>
schema extension point. We should always pass APIVersionRequest object<br>
instead of plain string. Due to "version == APIVersionRequest('2.32')"<br>
is always wrong, we should remove the '__eq__'. The developer should<br>
always use the 'APIVersionRequest.matches' [3] method.<br>
<br>
That can prevent the first mistake we made. But nothing help for<br>
second mistake. Currently we only run the test on the specific<br>
Microversion for the specific interesting point. In the before, the new<br>
tests always inherits from the previous microversion tests, just like<br>
[4]. That can test the old API behavior won't be changed in the new<br>
microversion. But now, we said that is waste, we didn't do that again<br>
just like [5]. Should we change that back?<br>
<br>
Thanks<br>
Alex<br>
<br>
[0]<br>
<a href="https://review.openstack.org/#/c/304510/64/nova/api/openstack/compute/block_device_mapping.py" rel="noreferrer" target="_blank">https://review.openstack.org/#<wbr>/c/304510/64/nova/api/openstac<wbr>k/compute/block_device_mapping<wbr>.py</a><br>
[1] <a href="https://review.openstack.org/#/c/316398/37/nova/api/openstack/compute/schemas/servers.py@88" rel="noreferrer" target="_blank">https://review.openstack.org/#<wbr>/c/316398/37/nova/api/openstac<wbr>k/compute/schemas/servers.py@8<wbr>8</a><br>
[2] <a href="https://review.openstack.org/#/c/316398/37/nova/api/openstack/compute/schemas/servers.py@79" rel="noreferrer" target="_blank">https://review.openstack.org/#<wbr>/c/316398/37/nova/api/openstac<wbr>k/compute/schemas/servers.py@7<wbr>9</a><br>
[3] <a href="https://github.com/openstack/nova/blob/master/nova/api/openstack/api_version_request.py#L219" rel="noreferrer" target="_blank">https://github.com/openstack/n<wbr>ova/blob/master/nova/api/opens<wbr>tack/api_version_request.py#L2<wbr>19</a><br>
[4] <a href="https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_evacuate.py#L415" rel="noreferrer" target="_blank">https://github.com/openstack/n<wbr>ova/blob/master/nova/tests/uni<wbr>t/api/openstack/compute/test_e<wbr>vacuate.py#L415</a><br>
[5] <a href="https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_serversV21.py#L3584" rel="noreferrer" target="_blank">https://github.com/openstack/n<wbr>ova/blob/master/nova/tests/uni<wbr>t/api/openstack/compute/test_s<wbr>erversV21.py#L3584</a><br>
<br>
<br>
<br>
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
<br>
</blockquote>
<br>
First, thanks to Kevin and Alex for finding this issue and explaining it in detail so we can understand the scope.<br>
<br>
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.<br>
<br>
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.<br></blockquote><div><br></div></div></div><div><div class="gmail_default" style="color:rgb(0,0,0)"><font face="arial, helvetica, sans-serif">+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. </font></div><br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
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.</blockquote></span><div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"></div><div class="gmail_default" style="color:rgb(0,0,0)"><font face="arial, helvetica, sans-serif">Also along with that, I'd like to add tempest job with 'latest' microversion (we thought of this earlier) and run as nv on nova side. Current tests will fail due to behavior changes in versions and need version cap etc. We can start capping/fixing tempest tests and make as much as tests to pass with 'latest'.</font></div><div class="gmail_default" style="color:rgb(0,0,0)"><font face="arial, helvetica, sans-serif"><br></font></div><div class="gmail_default" style="color:rgb(0,0,0)"><font face="arial, helvetica, sans-serif">During new version changes, that job can clearly tell what kind of behavior is changing and we can catch that.</font></div><br></div><span class=""><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="m_2246139391607839466HOEnZb"><font color="#888888"><br>
<br>
-- <br>
<br>
Thanks,<br>
<br>
Matt Riedemann<br>
<br>
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
</font></span></blockquote></span></div><br></div></div>
<br>______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.<wbr>openstack.org?subject:<wbr>unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/<wbr>cgi-bin/mailman/listinfo/<wbr>openstack-dev</a><br>
<br></blockquote></div><br></div>