| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-07 | |||
| 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 | |
| 15:33:24 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Allow multi-attach in compute api https://review.openstack.org/271047 | |
| 15:33:26 | mriedem | gibi: ^ fixed | |
| 15:33:40 | gibi | mriedem: looking... | |
| 15:33:57 | mriedem | i looked at the functional py35 failure in https://review.openstack.org/#/c/330285/ | |
| 15:34:19 | mriedem | it was really hard to see from the actual output what the issue was, and i'm assuming it's intermittent, but i can dig again | |
| 15:34:24 | mriedem | the py27 functional didn't fail | |
| 15:34:41 | mriedem | http://logs.openstack.org/85/330285/175/check/openstack-tox-functional-py35/9a23bfd/testr_results.html.gz | |
| 15:34:50 | gibi | mriedem: could be some timing issue. py35 thend to uncover those | |
| 15:35:41 | mriedem | AssertionError: The migration table left empty. | |
| 15:36:15 | mriedem | hmm | |
| 15:36:15 | mriedem | 2017-12-07 10:11:01,711 WARNING [nova.virt.block_device] Detaching volume from unknown instance | |
| 15:36:44 | mriedem | TestInstanceNotificationSampleWithMultipleComputeOldAttachFlow would suggest this is a latent issue | |
| 15:37:05 | ildikov | I don't recall seeing this before | |
| 15:37:31 | mriedem | http://logstash.openstack.org/#dashboard/file/logstash.json?query=message%3A%5C%22AssertionError%3A%20The%20migration%20table%20left%20empty.%5C%22%20AND%20tags%3A%5C%22console%5C%22&from=7d | |
| 15:37:40 | mriedem | gibi: there are other unrelated patches hitting the same thing | |
| 15:37:44 | mriedem | so it's intermittent and latent | |
| 15:38:33 | gibi | mriedem: OK, your logstash link is convincing | |
| 15:38:36 | mriedem | i'll open a bug | |
| 15:38:54 | gibi | mriedem: I will put back my +2 | |
| 15:40:45 | ildikov | mriedem: BTW, I didn't fully get that part of the commit message of the block_device patch that gibi asked about | |