| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-07 | |||
| 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 ^ ? | |
| 15:47:43 | mriedem | ildikov: in that code, nova is creating the volume | |
| 15:47:44 | mriedem | agree? | |
| 15:47:57 | ildikov | mriedem: that why we need to create a new attachment here too: refresh_connection_info | |
| 15:48:21 | mriedem | ildikov: do you mean https://review.openstack.org/#/c/525787/4/nova/virt/block_device.py@473 ? | |
| 15:48:24 | jaypipes | mdbooth: you working on fixing those unit test failures on the BDM uuid patch? | |
| 15:48:42 | ildikov | yeah, I agree with that, I just thought we covered everything for that case of BFV as well as we removed check attach, so things needed to be ok on the Cinder side as well regarding volume state, etc. | |
| 15:49:29 | ildikov | mriedem: no, I meant line 589, I don't get why we need to add that one | |
| 15:50:36 | mriedem | ildikov: if we create the volume but don't create the attachment, | |
| 15:50:43 | mriedem | when we call super(...).attach() | |
| 15:50:54 | mriedem | https://review.openstack.org/#/c/525787/4/nova/virt/block_device.py@511 will be True | |
| 15:51:02 | mriedem | and we will go down _legacy_volume_attach | |
| 15:51:16 | ildikov | sorry, my bad, mixed it up | |
| 15:51:19 | ildikov | forget that one | |
| 15:51:24 | openstackgerrit | Ed Leafe proposed openstack/nova master: Refactor the code to check for sufficient hosts https://review.openstack.org/520242 | |
| 15:51:24 | openstackgerrit | Ed Leafe proposed openstack/nova master: Return Selection objects from the scheduler driver https://review.openstack.org/495854 | |
| 15:51:25 | openstackgerrit | Ed Leafe proposed openstack/nova master: Move the to_dict() method to the Selection object https://review.openstack.org/523492 | |
| 15:51:25 | openstackgerrit | Ed Leafe proposed openstack/nova master: Modify select_destinations() to return objects and alts https://review.openstack.org/510159 | |
| 15:51:26 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change RPC for select_destinations() https://review.openstack.org/516707 | |
| 15:51:26 | openstackgerrit | Ed Leafe proposed openstack/nova master: Move the claim_resources method to scheduler utils https://review.openstack.org/511357 | |
| 15:51:27 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358 | |
| 15:51:27 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change compute RPC to use alternates for resize https://review.openstack.org/526436 | |
| 15:51:32 | edleafe | mriedem: jaypipes: ^^ rebased | |
| 15:51:46 | edleafe | mriedem: also added the alternates for resize patch | |