| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-25 | |||
| 11:05:44 | kashyap | And the test from hell passes!~ | |
| 11:06:20 | mdbooth | \o/ | |
| 11:06:37 | stephenfin | mdbooth: If you have time, fancy taking a look at ^ to see if I'm missing anything really obvious? | |
| 11:06:51 | stephenfin | I'm seeing "AttributeError: 'module' object has no attribute 'vif'" but I've no idea why | |
| 11:06:54 | sean-k-mooney | stephenfin: i was just going to ask you about the functional tests | |
| 11:07:17 | sean-k-mooney | do you want me to pull them down and step through them more or did you get them to work? | |
| 11:07:25 | stephenfin | Thought it was a namespace conflict (mocking "objects" or something) but I renamed the import to no effect. Can't find any global mocks or anything | |
| 11:07:30 | stephenfin | sean-k-mooney: That'd be great | |
| 11:07:38 | stephenfin | I've got it to the point where it's attempting to plug | |
| 11:07:47 | sean-k-mooney | stephenfin: cool will do | |
| 11:07:54 | stephenfin | But the plug is failing with the above | |
| 11:08:03 | stephenfin | sean-k-mooney: Awesome. Thank you :) | |
| 12:10:52 | openstackgerrit | Neha Alhat proposed openstack/python-novaclient master: Enable split logging for cinder-novaclient interaction https://review.openstack.org/581218 | |
| 12:22:58 | openstackgerrit | Jan Gutter proposed openstack/nova master: Use vif.vif_name in _set_config_VIFGeneric https://review.openstack.org/571461 | |
| 12:29:34 | mdbooth | lyarwood: https://review.openstack.org/#/c/571410/7 Are we missing a simpler solution here? | |
| 12:29:59 | mdbooth | lyarwood: And another big race condition in a periodic, of course. | |
| 12:42:05 | lyarwood | mdbooth: erm I don't follow, the entire point of that conditional is to not fail, to not raise DiskNotFound? | |
| 12:42:39 | mdbooth | lyarwood: And my point is we're decided whether to fail or not based on data which is really out of date, and why would it ever be better to fail there anyway? | |
| 12:42:57 | mdbooth | lyarwood: i.e. just don't fail. | |
| 12:42:59 | mdbooth | ever. | |
| 12:43:54 | stephenfin | mriedem: FYI, I've got further with that functional test but I'm still seeing some issues (this time "AttributeError: 'module' object has no attribute 'vif'", where module is os_vif). Will keep working on it | |
| 12:44:34 | lyarwood | mdbooth: yeah I guess that's valid sorry, warning when it isn't found would be cleaner I guess | |
| 12:44:55 | mriedem | stephenfin: hmm, i think the FakeLibvirtFixture has a flag for stubbing out os-vif somehow | |
| 12:44:57 | mriedem | might want to check that out | |
| 12:45:19 | stephenfin | mriedem: It does and I think it's enabled by default | |
| 12:45:22 | mriedem | stephenfin: is that just the rebuild functional test on a single host? or are you working on a test that actuall moves the instance from one host to another, like cold migrate? | |
| 12:46:02 | mriedem | i think the latter would be most interesting if you have 3 computes where 2 satisfy the numa affinity network request and we cold migrate and make sure the scheduler picked the other host that supported the network request | |
| 12:46:20 | stephenfin | mriedem: Just rebuild. I'm still stuck attaching an interface to running VM, so until I figure that out I can't do anything else | |
| 12:47:04 | stephenfin | so my test isn't getting past here https://review.openstack.org/#/c/585385/5/nova/tests/functional/libvirt/test_numa_servers.py@449 | |
| 12:47:37 | mriedem | why do you need the attach? to see that the info cache is updated? | |
| 12:48:12 | mriedem | stephenfin: why can't your test just be (1) create server with numa affinity network requested on host A, (2) cold migrate it to host B where host B has the numa affinity you need but we also have another host C which should get filtered out | |
| 12:50:12 | stephenfin | Yup. When I rebuild after attaching an interface, it should take that new interface into account when (re)calculating NUMA topology | |
| 12:50:45 | stephenfin | mriedem: Hmm, that could work. Lemme try | |
| 12:51:25 | sean-k-mooney | stephenfin: so one of the issues is curerntly you are not inialising os_vif i get a little futher in the rebuilt test when i do that | |
| 12:54:24 | sean-k-mooney | because you did not iniallise os-vif https://github.com/openstack/os-vif/blob/master/os_vif/objects/__init__.py was never run so _nova_to_osvif_vif_ovs failed in os_vif_util.py in nova because we import os_vif.objects but not os_vif.objects.vif | |
| 12:54:33 | mriedem | cores: this ironic patch needs another +2 to complete the blueprint, pretty simple https://review.openstack.org/#/c/517921/ | |
| 12:55:58 | stephenfin | sean-k-mooney: Damn, I saw that used elsewhere by figured it would have been called by the fake nova-compute service. That's what I get for assuming | |
| 12:59:35 | sean-k-mooney | stephenfin: there are still a few other errors after that which im looking at now. add import os_vif and then add os_vif.initialize() to the setup function on lin 294 if you want to see them | |
| 13:03:27 | openstackgerrit | Merged openstack/nova master: Use placement context in placement functional tests https://review.openstack.org/585529 | |
| 13:07:22 | openstackgerrit | Merged openstack/nova stable/queens: Add unshelve instance error info to fault table https://review.openstack.org/583418 | |
| 13:12:23 | mriedem | i've got 3 more changes to close out the port binding live migration blueprint, bottom one has a +2 from gibi https://review.openstack.org/#/c/551371/ | |
| 13:12:46 | mriedem | given how long this series was and the meat is already merged, it would be nice to see the final bits make it | |
| 13:13:09 | Kevin_Zheng | mriede | |
| 13:13:48 | Kevin_Zheng | mriedem: I tried some other backends, it’s the same, but maybe I missed something | |
| 13:14:55 | Kevin_Zheng | But I think you can focus on feature freeze now, we can do this latter, I will also check more | |
| 13:15:02 | sean-k-mooney | mriedem: yes plus the effort put in from the neutron side on this also make it a higher proity to get it over the line as both comunity worked hard on this problem | |
| 13:15:39 | mriedem | Kevin_Zheng: i would suggest talking with gcb during your daytime | |
| 13:15:54 | mriedem | to see if he has some ideas on how to use those in-memory backends for functional testing | |
| 13:27:30 | mriedem | newish gate bug https://bugs.launchpad.net/nova/+bug/1783565 | |
| 13:27:31 | openstack | Launchpad bug 1783565 in OpenStack Compute (nova) "ServerGroupTestV21.test_evacuate_with_anti_affinity_no_valid_host intermittently fails with "Instance compute service state on host2 expected to be down, but it was up."" [Undecided,New] | |
| 13:35:09 | openstackgerrit | Matthew Booth proposed openstack/nova master: Fix signature of _FakeImageService.download https://review.openstack.org/585747 | |
| 13:35:10 | openstackgerrit | Matthew Booth proposed openstack/nova master: Don't poison Host._init_events if it's already mocked https://review.openstack.org/585748 | |
| 13:39:52 | Kevin_Zheng | mriedem: I will | |
| 13:44:05 | mdbooth | stephenfin: http://paste.openstack.org/show/726617/ | |
| 13:44:28 | mdbooth | I actually now have a functional test which does evacuate with the libvirt driver | |
| 13:45:55 | sean-k-mooney | mdbooth: cool. im trying to debug why in stephenfin test the interface he is attaching is been added as a disk to the fake instacne... | |
| 13:47:50 | mdbooth | stephenfin: That's a kludge, but the alternative is massive changes to fakelibvirt | |
| 13:48:24 | mdbooth | sean-k-mooney: That sounds pretty weird | |
| 13:48:38 | stephenfin | mdbooth: kludgy and working beats extensive and broken | |
| 13:49:10 | sean-k-mooney | mdbooth: ya its not waht i was expecting to happen | |
| 13:51:14 | sean-k-mooney | i found the issue .. | |
| 13:53:15 | efried | cdent: Can you ack https://review.openstack.org/#/c/585683/ (http://logs.openstack.org/83/585683/1/check/build-openstack-sphinx-docs/e5c8c01/html/admin/configuration/hypervisor-vmware.html#vmware-vcenter-service-account) ? | |
| 13:53:37 | sean-k-mooney | stephenfin: mdbooth so thats a thing https://github.com/openstack/nova/blob/003667ab246d0bcbbbec97e88cd0a0d2f0e6c044/nova/tests/unit/virt/libvirt/fakelibvirt.py#L784-L788 | |
| 13:54:12 | cdent | efried: only in the sense that I trust rado got it right. I know zero about vcenter permissions tree | |
| 13:54:33 | stephenfin | sean-k-mooney: Ha, incomplete mocks strike again | |
| 13:54:56 | mdbooth | sean-k-mooney: Hehe | |
| 13:54:58 | sean-k-mooney | stephenfin: ill fix it locally and see what happens. | |
| 13:55:19 | cdent | efried: I think it's rubber stampable | |
| 13:55:47 | efried | cdent: ight, rubber-stamped. | |
| 13:57:52 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Use placement 1.28 in scheduler report client https://review.openstack.org/583667 | |
| 14:03:49 | stephenfin | mriedem: Can we set different config options for fake services? e.g. test_compute1 has a different '[neutron_physnet_foo] numa_nodes' value from test_compute2 | |
| 14:04:24 | mriedem | ummm | |
| 14:04:30 | mriedem | i don't think so... | |
| 14:04:34 | mriedem | CONF is global | |
| 14:04:40 | mriedem | and everything runs in the same process | |
| 14:05:36 | mriedem | stephenfin: so yeah i'm not sure how we can run multiple compute services in a functional test but 2 of them share the same numa network config and another doesn't | |
| 14:05:54 | stephenfin | Yup, my point exactly | |
| 14:06:06 | dansmith | we can't | |
| 14:06:28 | dansmith | unless they are things that get reported during service start, | |
| 14:06:43 | dansmith | such that we can set the flag, start the service, have it cache/report a thing, and then change it for the next start | |
| 14:06:53 | dansmith | but that's fragile and very few things fit that pattern | |
| 14:07:20 | dansmith | lol | |
| 14:07:34 | mdbooth | Who can kickban? | |
| 14:07:35 | sean-k-mooney | dansmith: i was going to suggest the same thing. e.g. set conf values for compute node 1 then start then set conf for node 2 then start | |
| 14:07:40 | mriedem | stephenfin: could have a negative test where you start with a host that fits the requested numa affinity and then remove the config, start another host and migrate the server | |
| 14:07:49 | dansmith | sean-k-mooney: that works for very few things | |
| 14:07:56 | mriedem | stephenfin: in that case, what do we expect to happen? the numa filter should kick it out right? | |
| 14:08:26 | sean-k-mooney | dansmith: yep but it might work for stephenfin usecase | |
| 14:08:27 | mriedem | and tests that you've properly plumbed the request spec for the cold migration in conductor | |
| 14:08:35 | stephenfin | mriedem: Yup, that's what I'd expect, anyway | |
| 14:08:50 | bauzas | mriedem: btw. saw your req_spec changes | |
| 14:08:54 | bauzas | mriedem: thanks for this | |
| 14:09:00 | mriedem | and commenting out the ensure* line in nova/conductor/tasks/migrate.py would make the test fail | |
| 14:09:21 | mriedem | bauzas: np, i want all that compat code dead in stein | |
| 14:09:58 | bauzas | mriedem: I'll try to review those quickly | |
| 14:10:01 | mriedem | dansmith: is what i described for a cold migate negative test good enough for you for the move patch? | |
| 14:10:47 | dansmith | mriedem: combined with a positive test where we successfully migrate between two hosts that satisfy the requirement I guess | |
| 14:11:05 | bauzas | btw. what's super important to review for the 3st milestone ? | |
| 14:11:13 | bauzas | for the moment, looking at https://review.openstack.org/#/c/576927/19 | |
| 14:11:14 | stephenfin | mriedem: Wait, actually, it wouldn't complain if I simply removed the config. No config = do nothing special (to preserve backwards compatibility) | |