Earlier  
Posted Nick Remark
#openstack-nova - 2017-12-07
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
15:46:29 mriedem ildikov: link to the line please
15:46:35 stephenfin Ideal. Thanks, jaypipes
15:47:03 ildikov mriedem: what you pasted above: https://review.openstack.org/#/c/525787/4/nova/virt/block_device.py@589
15:47:27 mriedem ildikov: what don't you get about ^ ?

Earlier   Later