| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-07 | |||
| 14:40:20 | jianghuaw_ | that's why I said *had* | |
| 14:42:10 | bauzas | k | |
| 14:44:02 | jianghuaw_ | bauzas, but I do wish very very much to get the patches on single vGPU can be merged soon. Particularly the long vacation is approaching:-) | |
| 14:46:22 | jianghuaw_ | bauzas, but feel free to let me know if further change needed for https://review.openstack.org/#/c/516899/ | |
| 14:46:33 | jianghuaw_ | I'm happy to rework it. | |
| 15:03:47 | mriedem | gibi: ildikov: replied in https://review.openstack.org/#/c/525787/ | |
| 15:03:53 | mriedem | gibi: thanks for the thorough review as always | |
| 15:04:20 | ildikov | mriedem: Tnx, will check in a bit | |
| 15:04:26 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/ocata: Fix 'force' parameter in os-quota-sets PUT schema https://review.openstack.org/526426 | |
| 15:06:16 | mriedem | sdague: dansmith: bauzas: can we get these patches in for the recent cve fix (pike) and then i'll start prepping for a release https://review.openstack.org/#/q/topic:alternate/1664931+status:open | |
| 15:06:35 | dansmith | ya | |
| 15:07:22 | dansmith | mriedem: what is the pike only workaround? that hash in the commit message links to this | |
| 15:07:31 | mriedem | lyarwood: there are several patches with +2s on pike, can you go through those? https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/pike | |
| 15:07:48 | dansmith | oh wait | |
| 15:07:50 | mriedem | dansmith: this https://review.openstack.org/#/c/523213/4/nova/tests/functional/test_servers.py | |
| 15:07:52 | mriedem | the setUp | |
| 15:08:10 | mriedem | i should have said, "in the functional test" or something | |
| 15:08:10 | dansmith | yeah, okay I was expecting something functional | |
| 15:08:12 | dansmith | yeah | |
| 15:08:30 | dansmith | also it just links back to this series which was confusing me about where the thing was added | |
| 15:08:39 | dansmith | outside of gerrit that will link properly though so that's fine | |
| 15:09:15 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/newton: Fix 'force' parameter in os-quota-sets PUT schema https://review.openstack.org/526427 | |
| 15:09:19 | lyarwood | mriedem: yup will try, still handing over all of my tripleo work atm, hopefully back to nova full time next week btw | |
| 15:09:37 | mriedem | i anticipate your glorious return | |
| 15:12:52 | mriedem | sdague: you were +2 on this before i robustified the tests (copying efried's work) https://review.openstack.org/#/c/521947/ - that is something we'll need to get backported also | |
| 15:13:01 | gibi | mriedem: thanks for the reply in the attach flow patch. Now the BFV case is clear for me | |
| 15:13:05 | edleafe | dansmith: I have a question about the changes to the filter_scheduler._claim_resources() method | |
| 15:13:21 | mriedem | gibi: yeah that took me some time to figure out why those tests weren't working as i'd expected | |
| 15:13:39 | edleafe | dansmith: the docstring about the params doesn't match the params to the method | |
| 15:14:05 | edleafe | dansmith: https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L272-L293 | |
| 15:14:25 | mriedem | jgriffith: do you remember why you added this? https://review.openstack.org/#/c/525787/3/nova/virt/block_device.py@448 | |
| 15:14:35 | mriedem | jgriffith: doesn't seem necessary so i plan on removing it unless there was a need for this | |
| 15:14:40 | dansmith | edleafe: is that a change I made? | |
| 15:15:40 | edleafe | dansmith: I think so. Were there any other changes to _claim_resources? | |
| 15:15:58 | edleafe | dansmith: just the rebuild check, right? | |
| 15:15:58 | dansmith | edleafe: I dunno, let me dig up history.. what's your question though? | |
| 15:16:45 | edleafe | dansmith: rebasing my series. The docstring says that it is getting and instance UUID, a HostState object, and allocation_requests | |
| 15:17:00 | mriedem | gibi: do you want me to remove the 4 changes at the top of https://review.openstack.org/#/c/525787/3/nova/tests/unit/virt/test_block_device.py where i'm setting the attachment_id in those *_driver_bdm class dicts? | |
| 15:17:07 | edleafe | but it's still just getting a spec_obj, instance uuid and allocation_requests | |
| 15:17:13 | dansmith | edleafe: This is the change I didn't change the params: https://review.openstack.org/#/c/521662/10/nova/scheduler/filter_scheduler.py | |
| 15:17:28 | mriedem | gibi: maybe i should just remove the key from the *_driver_bdm variables since they are set in setUp anyway | |
| 15:17:33 | mriedem | to avoid confusion about why they are there | |
| 15:17:36 | edleafe | but it's still just getting a spec_obj, instance uuid and allocation_requests | |
| 15:17:39 | edleafe | oops | |
| 15:18:02 | edleafe | ok, then I'll just change the docstring to be correct | |
| 15:18:12 | dansmith | edleafe: yeah, I'm not sure where that docstring diverged from the signature, but it's clearly wrong | |
| 15:18:18 | edleafe | I was wondering if there were other changes being merged that affected it | |
| 15:18:31 | edleafe | dansmith: got it | |
| 15:18:49 | gibi | mriedem: just remove that is anyhow set in the setUp | |
| 15:19:34 | gibi | mriedem: but I that is the 4 change in top of the test_block_device.py :) | |
| 15:21:02 | edleafe | dansmith: huh, it was in jaypipes's change for adding claiming back in July. I guess I had corrected it in my series so long ago, that I thought the merge conflict from your change also changed the docstring | |
| 15:21:06 | sdague | mriedem: yeh, +2 on that still | |
| 15:21:20 | mriedem | sdague: thanks | |
| 15:21:26 | dansmith | edleafe: that sounds like him | |
| 15:21:35 | edleafe | dansmith: :) | |
| 15:23:03 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_tree https://review.openstack.org/520663 | |
| 15:23:03 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient._get_providers_in_aggregates https://review.openstack.org/521097 | |
| 15:23:04 | openstackgerrit | Eric Fried proposed openstack/nova master: Traits ops on ProviderTree https://review.openstack.org/521605 | |
| 15:23:04 | openstackgerrit | Eric Fried proposed openstack/nova master: Aggregate ops on ProviderTree https://review.openstack.org/526216 | |
| 15:23:05 | openstackgerrit | Eric Fried proposed openstack/nova master: Move aggregates from report client to ProviderTree https://review.openstack.org/521685 | |
| 15:23:05 | openstackgerrit | Eric Fried proposed openstack/nova master: Track provider traits in report client https://review.openstack.org/521686 | |
| 15:23:06 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: ProviderTree.populate_from_iterable https://review.openstack.org/520756 | |
| 15:23:06 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098 | |
| 15:23:07 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187 | |
| 15:23:07 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246 | |
| 15:23:13 | efried | jaypipes cdent As threatened ^ | |
| 15:23:16 | jaypipes | edleafe: I blame efried. | |
| 15:23:29 | jaypipes | let the blame game commence. | |
| 15:23:38 | efried | wait, what am I getting blamed for? | |
| 15:23:41 | mriedem | i know some intrepid core wants to +W this https://review.openstack.org/#/c/521947/ | |
| 15:23:45 | jaypipes | efried: nothing :) was just kiddin. | |
| 15:23:55 | efried | jaypipes I figured, but still want to know. | |
| 15:24:11 | jaypipes | efried: apparently I goofed a patchset of edleafe's a while ago. | |
| 15:24:27 | cdent | since multiple placement people are gathered at the mo, does everyone agree with takeshi’s assertion that some unit tests ought to happen on https://review.openstack.org/#/c/521640/ We haven’t historically been super aggressive about unit tests in the handlers | |
| 15:24:40 | cdent | sorry takashi. typing is hard | |
| 15:24:43 | jaypipes | mriedem: looking at the bug now.. | |
| 15:25:34 | mriedem | cdent: is it covered by gabbits? | |
| 15:25:58 | mriedem | oh it's a format thing https://review.openstack.org/#/c/521640/7/nova/tests/functional/api/openstack/placement/gabbits/allocations.yaml | |
| 15:26:07 | mriedem | not utcnow vs updated_at | |
| 15:26:18 | jaypipes | cdent: the only things that I think need to go in unit tests are those situations (edge cases) that can't be easily reproduced in a func test | |
| 15:27:01 | mriedem | hmm | |
| 15:27:10 | cdent | mriedem: it is covered, but the not the specific conditional that is being pointed at | |
| 15:27:15 | mriedem | so the last-modified can be created_at, updated_at, the max in a collection, or utcnow | |
| 15:27:25 | mriedem | and that is per-handler | |
| 15:27:34 | cdent | there are tests for the utility that is used | |
| 15:27:39 | cdent | but not for its use in the handlers | |
| 15:27:44 | mriedem | yeah i see that | |
| 15:27:55 | cdent | (except in the fact that the gabbits test that a last-modified is returned) | |
| 15:28:01 | mriedem | and it's a datetime format | |
| 15:28:40 | mriedem | idk, asserting that the handler is doing the one right thing it should be doing would only be there for regression | |
| 15:28:57 | mriedem | so i can see value in it, but i'm not sure how much extra work that is | |
| 15:29:43 | cdent | it’s a fair chunk because of the way the handlers do their own serializing | |
| 15:29:48 | cdent | but not super duper onerous | |
| 15:30:01 | mriedem | so....maybe a follow up? | |
| 15:30:06 | mriedem | as a compromise | |
| 15:31:32 | cdent | fine with me, if thats the consensus | |
| 15:32:42 | mriedem | commented inline | |
| 15:33:22 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add new style volume attachment support to block_device.py https://review.openstack.org/525787 | |
| 15:33:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 15:33:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: libvirt: Allow multiple volume attachments https://review.openstack.org/267587 | |