| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-12 | |||
| 16:32:30 | mriedem | this https://review.openstack.org/#/c/390225/ | |
| 16:34:31 | mriedem | hmmm, so the nova unit test is going to need to transform the object to it's known good version, which would be 1.0 | |
| 16:39:01 | mnestratov | mriedem: hi | |
| 16:39:32 | mnestratov | mriedem: sorry, just saw your message | |
| 16:40:03 | mriedem | np | |
| 16:42:50 | mnestratov | mriedem: thanks for fixing the review for us ) | |
| 16:47:01 | mdbooth | artom: It occurs to me that we don't call _build_device_metadata often enough | |
| 16:47:31 | mdbooth | We need to call it every time we regenerate libvirt domain xml, because all hw address will change every time we do that | |
| 16:52:52 | openstackgerrit | Matt Riedemann proposed openstack/nova: WIP: Make unit tests work with os-vif 1.4.0 https://review.openstack.org/419558 | |
| 16:52:55 | mriedem | dansmith: jaypipes: i've got a problem with ^ because of http://paste.openstack.org/show/594736/ | |
| 16:53:16 | mriedem | i'm backporting the VIFHostUser object to 1.0 but it's nested 'network' object isn't getting backported recursively | |
| 16:53:25 | mriedem | well, i think it's trying, but there is no rule for it | |
| 16:54:08 | mriedem | i do see this though https://github.com/openstack/os-vif/blob/1.4.0/os_vif/objects/network.py#L51 | |
| 16:55:08 | openstackgerrit | Davanum Srinivas (dims) proposed openstack/nova: [WIP][py35] Trying to get cinder+rally working https://review.openstack.org/419476 | |
| 16:57:39 | dtp | dansmith | |
| 16:58:48 | dansmith | dtp | |
| 16:59:10 | dansmith | mriedem: is it calling super in make compatible? | |
| 16:59:19 | mriedem | dansmith: yup | |
| 16:59:25 | dtp | do i understand your comment on https://review.openstack.org/#/c/418141 that instead of relying on the mapping being there, we need to rely instead on cells.enable setting? | |
| 16:59:29 | mriedem | looks like _obj_relationships_for doesn't have a manifest on the thing i'm backporting | |
| 16:59:34 | dansmith | mriedem: does the manifest call for the older version/ | |
| 17:00:01 | mriedem | where does the manifest get set on the object? | |
| 17:00:19 | mriedem | oh i see it doesn't have a manifest | |
| 17:00:29 | mriedem | so it tries to use the old school self.obj_relationships[field] | |
| 17:00:33 | dansmith | dtp: which comment? | |
| 17:00:35 | mriedem | and we don't have a rule for it | |
| 17:00:45 | dtp | dansmith the commit message at the top | |
| 17:00:46 | dansmith | mriedem: yeah, manifest is just passed into the call, not stored anywhere | |
| 17:01:38 | dansmith | dtp: we need to ignore the fact that the mapping is in place, if we're still on cellsv1, so that we don't bypass the replication and go straight to the cell database, which might have not-yet-replicated stuff in it | |
| 17:01:59 | dtp | ok, thank you | |
| 17:02:19 | mriedem | ok so i need to call obj_make_compatible_from_manifest() instead and provide my own manifest on how to handle this nested field? | |
| 17:02:23 | dansmith | dtp: cells.enable is really cellsv1.enable | |
| 17:02:42 | dtp | yeah i have that burned to my brain roms :D | |
| 17:02:47 | dtp | +1! | |
| 17:02:59 | dansmith | mriedem: https://github.com/openstack/nova/blob/master/nova/tests/unit/objects/test_objects.py#L886-L956 | |
| 17:03:14 | mdbooth | artom: Replied. | |
| 17:10:13 | edleafe | Can someone explain the failure here: http://logs.openstack.org/51/416751/6/check/gate-nova-tox-db-functional-ubuntu-xenial/fec7ddc/testr_results.html.gz | |
| 17:10:38 | edleafe | It seems all the tests passed, but the returncode was 1 | |
| 17:11:41 | mriedem | edleafe: this looks odd http://logs.openstack.org/51/416751/6/check/gate-nova-tox-db-functional-ubuntu-xenial/fec7ddc/console.html#_2017-01-12_15_25_20_540113 | |
| 17:11:47 | mriedem | 2017-01-12 15:25:20.506 4337 INFO nova.osapi_compute.wsgi.server [req-471b1e60-9717-479e-a053-06834d8fe63f fake fake - - -] (4337) wsgi exited, is_accepting=True | |
| 17:12:12 | jaypipes | mriedem: lookin... | |
| 17:13:31 | edleafe | mriedem: odd is a good word | |
| 17:18:30 | jaypipes | mriedem: ah, yeah... I ran into similar recently when trying to add a UUID field to the NUMACell object. Objects that had their versions changed but never got an obj_make_compatible() method run into this failure. To fix, you need to manually set the obj_relationships class attribute on the object in osvif :( | |
| 17:18:46 | mriedem | what's really crazy, | |
| 17:19:08 | mriedem | when this test was added to nova https://review.openstack.org/#/c/334048/11/nova/tests/unit/network/test_os_vif_util.py@575 | |
| 17:19:13 | jaypipes | mriedem: we aren't yet sending osvif objects over the wire, though, so it may be we could do a hack of the object manifests with dansmith's help? | |
| 17:19:31 | mriedem | it's setting the vif_name which passed even though vif_name wasn't a field on that type in os-vif 1.3.0 which is what we were testing against | |
| 17:19:51 | mriedem | so...i guess when the test converts it to a primitive, | |
| 17:19:59 | mriedem | because vif_name wasn't a 'registered' field it just excluded it? | |
| 17:20:05 | mriedem | so the equality check was ok? | |
| 17:20:50 | mriedem | if that's the case, i'm not sure why vif_name isn't showing up on the actual object when we do the compare with os-vif 1.4.0 http://logs.openstack.org/21/418421/4/check/gate-cross-nova-python27-db-ubuntu-xenial/376a0f3/console.html#_2017-01-11_10_21_37_885392 | |
| 17:21:34 | jaypipes | mriedem: vif_name is a field: https://github.com/openstack/os-vif/blob/master/os_vif/objects/vif.py#L65 | |
| 17:21:37 | mriedem | jaypipes: these have obj_make_compatible methods | |
| 17:21:43 | dansmith | jaypipes: obj_relationships should not be used anymore | |
| 17:21:48 | jaypipes | https://github.com/openstack/os-vif/blob/master/os_vif/objects/vif.py#L77 | |
| 17:21:53 | mriedem | jaypipes: vif_name wasn't a field in VIFHostUser until 1.1 | |
| 17:22:04 | jaypipes | dansmith: yeah, I know, but I found no other way to fix that bug. | |
| 17:22:05 | mriedem | and ^ was just released in os-vif 1.4.0 | |
| 17:22:07 | jaypipes | mriedem: ah.' | |
| 17:22:20 | mriedem | https://review.openstack.org/#/c/390225/7/os_vif/objects/vif.py | |
| 17:22:53 | jaypipes | mriedem: you sure about that? https://github.com/openstack/os-vif/commit/11af752a5af8a58a4f8fd5af196d1fce7729463d | |
| 17:22:59 | jaypipes | mriedem: shows added in 2015. | |
| 17:23:26 | mriedem | jaypipes: on which object? | |
| 17:23:34 | mriedem | we're not dealing with vif generic or bridge, | |
| 17:23:37 | mriedem | it's VIFHostUser | |
| 17:23:44 | mriedem | see https://review.openstack.org/#/c/390225/7/os_vif/objects/vif.py | |
| 17:24:07 | mriedem | that adds the vif_name field to VIFHostUser and bumps it's version to 1.1, and that change is released in os-vif 1.4.0, which is now breaking nova unit tests | |
| 17:24:17 | mriedem | http://logs.openstack.org/21/418421/4/check/gate-cross-nova-python27-db-ubuntu-xenial/376a0f3/console.html#_2017-01-11_10_21_37_885392 | |
| 17:24:33 | mriedem | because vif_name is in our expected object on the nova side, but it's not in the actual obj we're getting from os-vif for some reason | |
| 17:25:27 | jaypipes | mriedem: hmm, yes, you're totally correct. | |
| 17:25:47 | mriedem | even though both primitives are showing | |
| 17:25:48 | mriedem | 'versioned_object.name': 'VIFVHostUser', | |
| 17:25:52 | mriedem | 'versioned_object.version': '1.1'} | |
| 17:25:54 | jaypipes | yup you're right. | |
| 17:26:32 | jaypipes | mriedem: weird... I wonder how https://github.com/openstack/os-vif/commit/345ff074b5dadc965b2298583124f32fc29dd2e4#diff-a2fd3d05a3b0a72438d0399e19c04469R152 ever worked then? | |
| 17:27:06 | mriedem | well that seems straight forward | |
| 17:28:09 | jaypipes | mriedem: sorry, I mean how the nova side didn't break when that was merged... | |
| 17:28:24 | mriedem | you mean when vif_name didn't exist in VIFHostUser? | |
| 17:28:38 | mriedem | i assume because it's not a registered field so the obj_to_primitive() ignored the field | |
| 17:28:42 | mriedem | so it wasn't in the primitive | |
| 17:28:47 | mriedem | you can set random attributes on versioned objects | |
| 17:28:58 | dansmith | correct | |
| 17:29:06 | mriedem | but the only thing that goes into the primitive is the fields | |
| 17:29:17 | mriedem | which in VIFHostUser 1.0 when that test was written didn't have a vif_name field | |
| 17:29:24 | mriedem | so it wouldn't be in the primitive and the nova test passed | |
| 17:29:24 | jaypipes | that are in the manifest for that version, yes. | |
| 17:29:47 | mriedem | unless i'm mistaken there isn't a manifest used in any of these tests | |
| 17:33:35 | mriedem | i think i have an easier fix than what i was trying | |
| 17:33:41 | jaypipes | mriedem: so I'm looking at that output in the console log from the unit test failure. really weird that the actual object has a version of 1.1 but doesn't include the vif_name field... | |
| 17:34:14 | jaypipes | mriedem: I mean, version 1.1 added the vif_name field and nothing else, no? so if it knows of the 1.1 object version, there should be a vif_name field. | |
| 17:34:32 | mriedem | jaypipes: right | |
| 17:34:38 | mriedem | i don't get that at all | |
| 17:35:11 | mriedem | unless it's somehow thinking it needs to backport to 1.0 for some reason | |
| 17:35:18 | mriedem | but i don't see anything telling it to do that | |
| 17:38:57 | openstackgerrit | Matt Riedemann proposed openstack/nova: Make unit tests work with os-vif 1.4.0 https://review.openstack.org/419558 | |
| 17:39:00 | mriedem | so ^ should fix the nova test, but i'm kind of worried about what's actually going on on the os-vif side | |
| 17:40:59 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova: Updated from global requirements https://review.openstack.org/415978 | |
| 17:41:25 | jaypipes | mriedem: will try to look into this further in a bit. have meetings currently.. | |
| 17:41:35 | mriedem | ok, i'm going to get some lunch | |
| 17:54:38 | armax | mriedem: hi | |