<div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">Thanks Alex for writing in details.</div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"><br></div><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 24, 2017 at 6:05 PM, Alex Xu <span dir="ltr"><<a href="mailto:soulxu@gmail.com" target="_blank">soulxu@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Unfortunately the device tag support in the API was broken in the old Microversion <a href="https://bugs.launchpad.net/nova/+bug/1658571" target="_blank">https://bugs.launchpad.net/<wbr>nova/+bug/1658571</a>, which thanks to Kevin Zheng to find out that.<div><br></div><div>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. </div><div><br></div><div>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].</div><div><br></div><div>So the current API behavior is as below:</div><div><br></div><div>2.32: BDM tag and network device tag added.</div><div>2.33 - 2.36: 'tag' in the BDM disappeared. The network device tag still works.</div><div>2.37: The network device tag disappeared also.</div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">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.</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">But from looking at api-ref and db, i am little bit confused. API-ref says device tag as "<span style="color:rgb(51,51,51);font-family:"open sans",helvetica,arial,sans-serif;font-size:14px">An arbitrary tag.</span>" [6] and DB also have it as Column(String(255)) [7]<br></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">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. </div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"><br></div><div class="gmail_default"> <br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>There are few questions we should think about:</div><div><br></div><div>1. Should we fix that by Microversion?</div><div> Thanks to Chris Dent point that out in the review. I also think we need to bump Microversion, which follow the rule of Microversion.</div></div></blockquote><div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">Initially i thought it only validation part we loosen and feature still works but with additionalPropoerties=False, its all disallowed to add tag :(.<br></div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>2. If we need Microversion, is that something we can do before release?</div><div> 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.</div><div><br></div><div>2. How can we prevent that happened again?</div><div> 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.</div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">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.</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div> 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.</div><div><br></div><div> 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?</div></div></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">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 <a href="https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1730">https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1730</a></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"><br></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">which seems failure on this - <a href="http://logs.openstack.org/52/423952/5/check/gate-nova-tox-db-functional-ubuntu-xenial/ff1c09a/">http://logs.openstack.org/52/423952/5/check/gate-nova-tox-db-functional-ubuntu-xenial/ff1c09a/</a> </div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">I will check and fix functional tests bits.</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>Thanks</div><div>Alex</div><div><br></div><div>[0] <a href="https://review.openstack.org/#/c/304510/64/nova/api/openstack/compute/block_device_mapping.py" target="_blank">https://review.openstack.org/#<wbr>/c/304510/64/nova/api/<wbr>openstack/compute/block_<wbr>device_mapping.py</a></div><div>[1] <a href="https://review.openstack.org/#/c/316398/37/nova/api/openstack/compute/schemas/servers.py@88" target="_blank">https://review.openstack.<wbr>org/#/c/316398/37/nova/api/<wbr>openstack/compute/schemas/<wbr>servers.py@88</a></div><div>[2] <a href="https://review.openstack.org/#/c/316398/37/nova/api/openstack/compute/schemas/servers.py@79" target="_blank">https://review.openstack.<wbr>org/#/c/316398/37/nova/api/<wbr>openstack/compute/schemas/<wbr>servers.py@79</a></div><div>[3] <a href="https://github.com/openstack/nova/blob/master/nova/api/openstack/api_version_request.py#L219" target="_blank">https://github.com/<wbr>openstack/nova/blob/master/<wbr>nova/api/openstack/api_<wbr>version_request.py#L219</a></div><div>[4] <a href="https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_evacuate.py#L415" target="_blank">https://github.com/<wbr>openstack/nova/blob/master/<wbr>nova/tests/unit/api/openstack/<wbr>compute/test_evacuate.py#L415</a></div><div>[5] <a href="https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_serversV21.py#L3584" target="_blank">https://github.com/<wbr>openstack/nova/blob/master/<wbr>nova/tests/unit/api/openstack/<wbr>compute/test_serversV21.py#<wbr>L3584</a></div><div><br></div></div></blockquote><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">[6] <a href="http://developer.openstack.org/api-ref/compute/?expanded=create-server-detail">http://developer.openstack.org/api-ref/compute/?expanded=create-server-detail</a></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">[7] <a href="https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L624">https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/models.py#L624</a> </div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">[8] <a href="http://developer.openstack.org/api-ref/compute/#server-tags-servers-tags">http://developer.openstack.org/api-ref/compute/#server-tags-servers-tags</a></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)">[9] <a href="https://blueprints.launchpad.net/nova/+spec/nova-microversion-functional-tests">https://blueprints.launchpad.net/nova/+spec/nova-microversion-functional-tests</a> </div><div><br></div><div class="gmail_default" style="font-family:monospace,monospace;color:rgb(0,0,0)"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">______________________________<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></div>