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 <[email protected]> wrote: > Thanks Alex for writing in details. > > > On Tue, Jan 24, 2017 at 6:05 PM, Alex Xu <[email protected]> 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] 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: [email protected]?subject:unsubscrib >> e >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
