| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-07 | |||
| 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 | |
| 15:43:52 | ildikov | mriedem: or do we not do that still for the case when Nova creates the volume? | |
| 15:44:04 | stephenfin | mriedem: Eek, seems like forever since I reviewed that. I'll take another look shortly | |
| 15:44:05 | mriedem | ildikov: that ^ | |
| 15:44:24 | mriedem | ildikov: when you pass a volume to bfv, we reserve it in the api by creating an attachment (with the new patch) | |
| 15:44:33 | mriedem | when nova creates the volume, we don't create an attachment (on the compute) | |
| 15:44:36 | mriedem | so we attach using the old flow | |
| 15:44:46 | mriedem | because self['attachment_id'] isn't set in the DriverBlockDevice | |
| 15:44:52 | mriedem | hence the todo i left in that code | |
| 15:45:11 | ildikov | mriedem: isn't that refresh used for live_migrate? | |
| 15:45:22 | stephenfin | jaypipes, efried: Am I correct in saying libvirt doesn't currently report traits to placement? | |
| 15:45:30 | mriedem | ildikov: refresh? | |
| 15:45:34 | jaypipes | stephenfin: correct. | |
| 15:45:59 | stephenfin | jaypipes: Phew. And that's something not targeted for Queens | |
| 15:46:00 | stephenfin | ? | |
| 15:46:02 | jianghuaw_ | mriedem, are you happy to have a quick look on https://review.openstack.org/#/c/523360/ which patch you ever +2; now only changed the picture's background back to transparent. | |
| 15:46:08 | jianghuaw_ | so hope a simple one:-) | |
| 15:46:10 | ildikov | mriedem: sorry, looked at the wrong line, but you added a note into that one as well | |
| 15:46:17 | ildikov | mriedem: which I didn't really get | |
| 15:46:24 | jaypipes | stephenfin: no, not targeted | |