| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-07 | |||
| 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 | |
| 15:41:13 | jianghuaw_ | stephenfin, Changed the picture back to having transparent background for this patch: https://review.openstack.org/#/c/523360/ | |
| 15:41:28 | jianghuaw_ | stephenfin, thanks for the comment. | |
| 15:41:32 | mriedem | https://bugs.launchpad.net/nova/+bug/1736976 | |
| 15:41:34 | openstack | Launchpad bug 1736976 in OpenStack Compute (nova) "test_live_migration_actions functional test randomly fails with "AssertionError: The migration table left empty."" [Undecided,New] | |
| 15:41:42 | mriedem | ildikov: about the boot from volume thing? | |
| 15:41:53 | ildikov | mriedem: I mean even if the volume is created by Nova having or not having an attachment_id has nothing to do with that | |
| 15:42:04 | ildikov | mriedem: the multiattach flag will indeed not be set by Nova | |
| 15:42:13 | ildikov | mriedem: yeah, BFV | |
| 15:42:20 | mriedem | ildikov: doesn't have anything to do with multiattach | |
| 15:42:30 | stephenfin | jianghuaw_: +2. Thanks for the follow-up | |
| 15:42:41 | mriedem | ildikov: i added todos like this https://review.openstack.org/#/c/525787/4/nova/virt/block_device.py@589 | |
| 15:42:44 | mriedem | does that make it clear? | |
| 15:42:55 | jianghuaw_ | stephenfin, thanks. | |
| 15:43:18 | ildikov | mriedem: no :) | |
| 15:43:20 | edleafe | mriedem: I got the "AssertionError: The migration table left empty." error in one of my series too | |
| 15:43:20 | mriedem | stephenfin: are you still ok with this https://review.openstack.org/#/c/507473/ ? | |
| 15:43:35 | edleafe | mriedem: passed locally | |
| 15:43:38 | ildikov | mriedem: we reserve the volume now for BFV | |