| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-24 | |||
| 12:22:33 | kashyap | sean-k-mooney: Good point; I did split the test. I'm lifting the restriction we added here: https://bugs.launchpad.net/nova/+bug/1766208 | |
| 12:22:42 | sean-k-mooney | kashyap: if openstack had an email only interface it would never work on it again. but if email via gerrit work then that would be cool | |
| 12:23:02 | kashyap | In the earlier patch, for 'host-model' it was invalid to specify 'extra_flags'. But now we allow it (as it is valid libvirt form) | |
| 12:23:23 | kashyap | sean-k-mooney: s/"it would"/I would/ ? | |
| 12:23:37 | sean-k-mooney | ah the restiction we added for backporting | |
| 12:24:54 | sean-k-mooney | ya i think being able to extend host-model makes sense. it was just one extra vairible that we did not want to backport originally so i dont think that bug is controverial | |
| 12:25:46 | kashyap | sean-k-mooney: Yeah, now I'm lifting that | |
| 12:26:18 | kashyap | git diff --cached | |
| 12:26:18 | sean-k-mooney | kashyap: it is technically a feature not a bug but even a specless blueprint seams to heavy for that so bug makes sense | |
| 12:26:22 | kashyap | Wrong window | |
| 12:26:30 | kashyap | sean-k-mooney: Yep | |
| 12:26:40 | sean-k-mooney | kashyap: password: :) | |
| 12:26:48 | kashyap | Heh | |
| 12:30:49 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Lift the restriction of choices for `cpu_model_extra_flags` https://review.openstack.org/563926 | |
| 12:31:09 | kashyap | sean-k-mooney: ^ When you can, mind having a gander? | |
| 12:31:40 | kashyap | (Pinged you explicitly as you have context from the prior patch.) | |
| 12:31:57 | sean-k-mooney | kashyap: ill be away for like 20 mins or so but i have it open in a tab for when i get back | |
| 12:32:11 | kashyap | Thanks! | |
| 12:35:50 | bhagyashris | efried; Hi, I just want to discuss regarding the zuul failure on patches https://review.openstack.org/#/c/560444/ https://review.openstack.org/#/c/560459/ . I have some observations; 1. Yesterday I had list down the test which are failing on both the patches [1][2] and then put the recheck in morning and checked wheather the same test are failing or not but I saw that the different tests are failing | |
| 12:36:12 | efried | bhagyashris: Unit or functional? | |
| 12:36:17 | bhagyashris | efried: Most of the test are failing with the error like (No valid host is found and Server <id> failed to build and is in ERROR status) so it seems that the host is not return properly from scheduler side. | |
| 12:36:36 | efried | oh, tempest | |
| 12:36:39 | bhagyashris | efried: tempest-full, tempest-full-py3 etc | |
| 12:36:48 | efried | Have you looked into the compute logs? | |
| 12:37:21 | bhagyashris | efried: I have checked the n-sch logs #link: http://logs.openstack.org/59/560459/3/check/tempest-full/b9c97a9/controller/logs/screen-n-sch.txt.gz#_Apr_24_06_52_41_192819 | |
| 12:38:31 | bhagyashris | efried: and it shows that the get_alocation_candidates() at https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L148 is returning the empty list | |
| 12:41:14 | bhagyashris | efried: As n-sch is not returning the host and our patch has changes at n-cpu side so I am not getting where exactly it's failing? | |
| 12:44:03 | efried | bhagyashris: http://logs.openstack.org/59/560459/3/check/tempest-full/b9c97a9/controller/logs/screen-placement-api.txt.gz#_Apr_24_06_52_41_190125 | |
| 12:44:31 | efried | bhagyashris: But requests before that are succeeding. It's like the host is running out of resources. This could be an allocation ratio thing. | |
| 12:44:41 | bhagyashris | efried: I have debug the tempest test tempest.api.compute.servers.test_multiple_create.MultipleCreateTestJSON.test_multiple_create on patch https://review.openstack.org/#/c/560459/ and observed that the allocation_candidates() return at https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L148 is empty list and same test i have ran on master there the allocation_candidates() return is successfully | |
| 12:47:02 | bhagyashris | efried: sorry i am getting fully . Means we will need some changes at placement side to fix the failing tests on zuul | |
| 12:48:40 | efried | bhagyashris: We definitely shouldn't need placement changes, no. | |
| 12:49:16 | efried | bhagyashris: You say you have a way to step through the tempest test and reproduce the failure? | |
| 12:49:47 | efried | bhagyashris: And presumably you would be able to step through that same test in an environment that did *not* include the patches in question (i.e. from the tip of the master branch)? | |
| 12:50:32 | efried | bhagyashris: So what we should do is query placement for the inventory records right before the meat of the test is run. | |
| 12:50:44 | efried | bhagyashris: They should be the *same* with or without our patch. | |
| 12:54:28 | bhagyashris | efried: I am not clear what will need to fix this | |
| 12:55:28 | bhagyashris | efried: As the same test are passing on master and our patches are not making any change at placement or n-sch side | |
| 12:56:40 | efried | bhagyashris: I am also not clear what is needed to fix this, which is why I suggested the above to debug. If we can demonstrate that the inventories in placement are different between master and our patch, then we've done something wrong with the transition from get_inventory to update_provider_tree. | |
| 12:56:59 | efried | bhagyashris: But if they're the same... then we need to look somewhere else (not sure where). | |
| 12:57:37 | bhagyashris | efried: ohh ok | |
| 12:58:02 | efried | bhagyashris: The tests fail the same way with the bottom patch as with the top, right? | |
| 12:58:10 | efried | So the problem isn't with our conditional DISK_GB thing. | |
| 12:58:21 | efried | It's with the translation from get_inventory to update_provider_tree. | |
| 12:58:34 | efried | Could be in the resource tracker. | |
| 12:58:39 | efried | or in the report client. | |
| 13:00:15 | bhagyashris | efried: just checked on both the patches different test are failing | |
| 13:01:40 | efried | bhagyashris: If we're reporting the inventory wrong, it could mean that we're running out of capacity sooner than we used to. But the tempest tests run in parallel in zuul. So which test runs out of capacity first could vary from one run to another. | |
| 13:01:56 | bhagyashris | on parent patch the tempest-full 2 test are failing and on child patch tempest-full 7 tests are failing | |
| 13:02:22 | efried | Let's just focus on the first patch compared to master. | |
| 13:03:15 | efried | bhagyashris: You said you were able to reproduce the failure manually? | |
| 13:03:34 | bhagyashris | efried: no | |
| 13:04:37 | bhagyashris | efried: I have debugged tempest test tempest.api.compute.servers.test_multiple_create.MultipleCreateTestJSON.test_multiple_create on patch https://review.openstack.org/#/c/560459/ and observed that the allocation_candidates() return at https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L148 is empty list and same test i have ran on master there the allocation_candidates() return is successfully | |
| 13:06:10 | efried | bhagyashris: What do you mean by "debugged" then - you mean just looking at logs? | |
| 13:07:53 | bhagyashris | efried: I mean I have put pdb at https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L121 and ran the tempest test using the tox -e py27 tempest test tempest.api.compute.servers.test_multiple_create.MultipleCreateTestJSON.test_multiple_create | |
| 13:08:20 | efried | bhagyashris: And you were able to reproduce the fact that GET /allocation_candidates returns no results? | |
| 13:08:25 | efried | ...with our patch | |
| 13:08:32 | efried | ...but on master it returns good results? | |
| 13:08:50 | bhagyashris | efried: yes | |
| 13:09:24 | efried | bhagyashris: Okay, so what I'm asking is for you to do the exact same thing, but when you hit your breakpoint, manually query the placement API and pull back the inventories and allocations for the compute host. | |
| 13:10:07 | efried | bhagyashris: If you do that for both environments - one with our patch and one with master - we should be able to determine whether those inventories/allocations are the same or different. | |
| 13:10:39 | efried | bhagyashris: I'm especially interested in whether the total, reserved, and allocation ratio match; and whether the allocations match. | |
| 13:19:22 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Lift the restriction of choices for `cpu_model_extra_flags` https://review.openstack.org/563926 | |
| 13:19:23 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: Do-Not-Merge: Test multiple CPU flags https://review.openstack.org/563948 | |
| 13:42:35 | openstackgerrit | Julia Kreger proposed openstack/nova master: Ironic: Lock the instance if early vif plug https://review.openstack.org/563714 | |
| 13:51:03 | openstackgerrit | Jens Harbott (frickler) proposed openstack/nova master: DNM: Test devstack + ceph plugin patches https://review.openstack.org/563097 | |
| 13:51:14 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix dropped check for boot_index 0 in _validate_bdm https://review.openstack.org/563732 | |
| 14:01:35 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Improve performance when list instances with IP filter https://review.openstack.org/563972 | |
| 14:26:32 | mriedem | stephenfin: can https://review.openstack.org/#/c/541290/ be updated now? | |
| 14:27:19 | stephenfin | mriedem: Sure. I was working through PoC to make sure the idea worked and it does. I'll update the spec pronto | |
| 14:27:32 | stephenfin | *a PoC | |
| 14:27:45 | mriedem | cool, thanks | |
| 14:28:28 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Drop MIN_LIBVIRT_BLOCK_LM_WITH_VOLUMES_VERSION https://review.openstack.org/563984 | |
| 14:33:22 | mriedem | johnthetubaguy_: can you go over https://review.openstack.org/#/c/552078/ again since i've updated it? | |
| 14:37:26 | esberglu | mriedem: Thoughts on my last comment here? https://review.openstack.org/#/c/526094/45/nova/virt/powervm/driver.py@559 | |
| 14:38:10 | esberglu | Ignore the 1st UT comment, that was some confusion on our end | |
| 14:38:29 | mriedem | so it's a DriverVolumeBlockDevice (which is a dict) rather than a BlockDeviceMapping object | |
| 14:38:37 | mriedem | DriverVolumeBlockDevice wraps BlockDeviceMapping | |
| 14:38:49 | mriedem | i think the ironic driver has the same need for the is_volume getting exposed from the internally wrapped bdm | |
| 14:38:51 | mriedem | sec | |
| 14:39:26 | mriedem | https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L392-L396 | |
| 14:40:08 | mriedem | so you could do that and then when we handle the ironic driver TODO we can also cleanup the powervm driver | |
| 14:40:40 | esberglu | mriedem: Sounds good thanks! | |
| 14:41:00 | mriedem | that also forces you to use DriverVolumeBlockDevice objects in your unit tests, but i think that's a good thing, given a BDM can be one of at least 3 or 4 things at any given point in the code | |
| 14:42:52 | esberglu | mriedem: We already are using DriverVolumeBlockDevice objects :) | |
| 14:43:28 | mriedem | then you get a root beer scented scratch-n-sniff | |
| 14:45:21 | stephenfin | mriedem, jaypipes, bauzas, gibi: I'm thinking most of the information from here down is too detailed for the spec (it was mostly to help me figure out the problem boundary myself). I think I could drop the whole piece. Thoughts? https://review.openstack.org/#/c/541290/7/specs/rocky/approved/numa-aware-vswitches.rst@38 | |
| 14:47:02 | mriedem | stephenfin: given my lack of knowledge on numa stuff, i'll likely appreciate details in the problem description | |
| 14:48:00 | stephenfin | mriedem: This is more about OVS-DPDK internals. If you think that's be helpful, I can keep it | |
| 14:48:12 | stephenfin | I'd like to add it to a different section but that upsets pep8 :) | |
| 14:48:29 | gibi | stephenfin: I have the same mental debate about the bandwidth spec. It is too detailed for a reader who is familar with the problem and the proposed solution, but it has a lot of nice details and reasoning for a new reader. | |
| 14:48:44 | gibi | stephenfin: for me it is OK to remove that section from your spec | |
| 14:49:09 | gibi | stephenfin: you can add new subsections but you cannot add a new top level section | |
| 14:49:11 | stephenfin | maybe I can publish it as a separate blog and link to that from there | |
| 14:49:23 | mriedem | stephenfin: i was just thinking that | |
| 14:49:50 | mriedem | 'for more information on how this relates to dpdk, see $link' | |
| 14:50:02 | stephenfin | mriedem: Sounds good to me. I'll do that | |
| 14:53:55 | jaypipes | stephenfin: I don't mind that detail. | |
| 15:05:54 | kashyap | stephenfin: Yeah, it comes useful for that poor soul who will look at it 5 years down the line | |
| 15:14:03 | melwitt | lyaaaaaaaaarwood: could you please hit this again? pike change has merged https://review.openstack.org/#/c/561613/ | |