| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-03 | |||
| 19:23:28 | openstackgerrit | Matt Riedemann proposed openstack/nova master: VMware: add support for different firmwares https://review.openstack.org/448034 | |
| 19:25:28 | stvnoyes | mriedem: Hi Matt, I was talking to ildikov and she suggested I could help with https://review.openstack.org/#/c/266605 , the tempest multiattach test. I see you've been working on it recently. Any objections to me taking it on? | |
| 19:31:58 | ildikov | stvnoyes: I mentioned it mriedem today that we talked about that test and that we can talk about the desired test coverage in Tempest on the meeting tomorrow | |
| 19:32:19 | mriedem | i'm debugging the failure now - can't create a multiattach volume | |
| 19:32:38 | ildikov | mriedem: it's disabled in Cinder | |
| 19:33:14 | ildikov | mriedem: https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/lvm.py#L279 | |
| 19:33:29 | mriedem | Jan 03 14:42:03.502967 ubuntu-xenial-citycloud-sto2-0001666901 cinder-scheduler[12541]: DEBUG cinder.scheduler.filters.capabilities_filter [None req-ecbc7305-ea6f-438b-af55-028c5b6eeb03 tempest-AttachVolumeMultiAttachTest-1915307512 None] Volume type extra spec requirement "multiattach=<is> True" does not match reported capability "False" {{(pid=12541) _satisfies_extra_specs /opt/stack/cinder/cinder/scheduler/filters/capabili | |
| 19:33:29 | mriedem | _filter.py:74}} | |
| 19:33:56 | stvnoyes | @page { margin: 0.79in } pre { direction: ltr; text-align: left; orphans: 2; widows: 2 } pre.western { font-family: "Times New Roman", serif; font-size: 12pt } pre.cjk { font-family: "SimSun"; font-size: 12pt; so-language: zh-CN } pre.ctl { font-family: "Times New Roman"; font-size: 12pt } p { margin-bottom: 0.1in; direction: ltr; line-height: 120%; text-align: left; orphans: 2; widows: 2 } p.western { fo | |
| 19:33:56 | stvnoyes | ||
| 19:34:03 | stvnoyes | ugh! | |
| 19:34:34 | stvnoyes | - Need the lvm driver to support MA - in cinder/volume/drivers/lvm.py , line 280 > | |
| 19:34:34 | stvnoyes | multiattach = True | |
| 19:35:05 | stvnoyes | there are also issues in tempest detach, I'm working on that now | |
| 19:35:36 | ildikov | stvnoyes: what's the issue with detach? | |
| 19:35:59 | mriedem | tempest assumes that when you detach the volume will eventually be 'available' | |
| 19:36:10 | mriedem | but with multiattach it will go from in-use to in-use | |
| 19:36:20 | mriedem | until the last attachment is gone | |
| 19:36:24 | ildikov | stvnoyes: there's a whole chain to address shared_targets issues: https://review.openstack.org/#/c/529695/ | |
| 19:36:31 | stvnoyes | from what I see so far, at the end of the test it tries to detach, but when it detaches the vol from the first server, the vol status doesn't go to abailable (as we would expect) but the test wants to see available | |
| 19:36:35 | ildikov | however I'm not sure that's applicable for lvm... | |
| 19:36:45 | mriedem | ildikov: different issues | |
| 19:37:00 | mriedem | i didn't realize that cinder hard-coded this in all drivers across the board | |
| 19:37:11 | ildikov | ah ok, sorry, I thought actual issues and not things we need to update | |
| 19:37:19 | mriedem | that should have probably been a global policy change in the API | |
| 19:37:48 | stvnoyes | I think we will need to pass in an expected status to detach so it knows what success means | |
| 19:37:53 | ildikov | I think they also wanted to ensure that driver maintainers think about it for at least two seconds before turning it on | |
| 19:38:20 | mriedem | stvnoyes: not sure that is necessary - we can just count the attachments first, and if the volume has multiattach=True | |
| 19:38:34 | stvnoyes | y, that should work too | |
| 19:38:52 | stvnoyes | and is simpler... | |
| 19:39:06 | mriedem | ildikov: so is https://review.openstack.org/#/c/428365/ going to be reverted at the same time that a policy rule is put in place? | |
| 19:39:36 | mriedem | not even a release note on that change which effectively breaks the API | |
| 19:40:33 | ildikov | well, it was never really supported anyway... | |
| 19:40:55 | ildikov | I would think that we turn back the drivers more one-by-one | |
| 19:41:34 | mriedem | ok so i'll push a WIP cinder patch to turn this on for lvm to at least get some test coverage going | |
| 19:42:02 | ildikov | but I can double check it with jgriffith or someone who feels brave enough to answer that question... :) | |
| 19:42:18 | mriedem | i just need to move the testing along, so i'm ok for now | |
| 19:44:57 | ildikov | mriedem: is there anything stvnoyes can help out or you're dedicated to make this one work? | |
| 19:45:35 | mriedem | i'm ok for now, wait until i'm stuck or don't have time to look | |
| 19:46:12 | stvnoyes | ok, that's fine. If there's something else I can help with, let me know. | |
| 19:46:19 | ildikov | mriedem: ok, cool, tnx | |
| 19:47:01 | mriedem | stvnoyes: have you read my comments in https://review.openstack.org/#/c/271047/ ? | |
| 19:47:04 | openstackgerrit | Chris Dent proposed openstack/nova master: Document lack of side-effects in AllocationList.create_all() https://review.openstack.org/530997 | |
| 19:47:17 | mriedem | could use more eyes on what, if any, restriction we're going to put in the compute api for multiattach and the new microversion | |
| 19:47:58 | stvnoyes | mriedem: i haven't looked at it yet. I'll take a look. | |
| 19:57:58 | mriedem | oh nice, | |
| 19:57:59 | mriedem | http://paste.openstack.org/show/637525/ | |
| 19:58:06 | mriedem | trying to attach a volume to more than one instance results in a 500 | |
| 20:01:05 | mriedem | Unable to create attachment for volume (Invalid volume: Volume 373c2a29-f6e7-4931-adda-6b87a193e9f9 status must be available or downloadi | |
| 20:03:42 | ildikov | mriedem: for a multiattach volume? | |
| 20:03:51 | mriedem | no, not multiattach | |
| 20:03:59 | mriedem | i wanted to make sure that still is prohibited | |
| 20:04:16 | openstackgerrit | Chris Dent proposed openstack/nova master: Fix documentation nits in set_and_clear_allocations https://review.openstack.org/531001 | |
| 20:04:37 | ildikov | oh, ok :) | |
| 20:08:53 | mriedem | hmm, why would i get a 400 in the case of os-reserve but a 500 from cinder in the case of attachment_create? | |
| 20:10:25 | mriedem | i would think the ResourceExceptionHandler would be used in both cases | |
| 20:10:33 | mriedem | that converts InvalidVolume to a 400 response | |
| 20:44:46 | openstackgerrit | Lance Bragstad proposed openstack/nova master: Simplify logic in get_enforcer https://review.openstack.org/531008 | |
| 20:55:10 | edleafe | mriedem: got a minute to talk about resize? | |
| 20:55:35 | mriedem | sure | |
| 20:56:42 | edleafe | we recently changed it to do 'replace_allocation_with_migration' to have the allocations use the migration uuid | |
| 20:57:19 | edleafe | It checks to verify that the instance is allocated against the source node, and if it isn't raises an error | |
| 20:57:57 | edleafe | But with claiming in the scheduler, select_destinations will claim against the target host | |
| 20:58:09 | edleafe | So the replace method always fails | |
| 20:58:56 | mriedem | edleafe: i can see that being true in the reschedule case, | |
| 20:59:12 | edleafe | exactly | |
| 20:59:16 | mriedem | because on the 2nd loop through conductor, the source node allocation will be held by the migration uuid and the first chosen and failed host is owned by the instance | |
| 20:59:33 | edleafe | (I forgot to mention that I'm working on functional tests for reschedules | |
| 20:59:50 | mriedem | ah - i was going to start working on a functional test this afternoon but got sidetracked with cinder | |
| 21:00:12 | mriedem | edleafe: so, we should (1) report a bug for the thing and (2) fix it separately before your alternate hosts patch for resize i think | |
| 21:00:17 | edleafe | I've been working on it, but it took a while to track down exactly why they were all failing | |
| 21:00:22 | mriedem | i was going to push the functional test before your alternate hosts patch in the series | |
| 21:00:39 | mriedem | yeah we need the functional resize + reschedule test regardless of alternate hosts | |
| 21:00:47 | mriedem | because it would have caught this earlier | |
| 21:00:55 | edleafe | makes sense | |
| 21:00:59 | mriedem | cool | |
| 21:01:23 | edleafe | so have you started on it? Should I wait and rebase on top of yours? | |
| 21:01:28 | mriedem | i haven't started | |
| 21:01:50 | mriedem | so feel free to report the regression bug and push a patch in nova/tests/functional/regressions | |
| 21:01:53 | mriedem | for that bug | |
| 21:02:06 | edleafe | ok, I can do it. I only have a little time left today, thanks to IBM adding a 2-hour each way commute to my schedule | |
| 21:02:24 | mriedem | how about report the bug and push what you have for the test | |
| 21:02:39 | mriedem | can work on a fix and such tomorrow | |
| 21:02:41 | edleafe | ok, I can do that. | |
| 21:02:44 | mriedem | thanks | |
| 21:05:38 | mriedem | i should probably start thinking about an rc1 bugs etherpad... | |
| 21:20:04 | openstackgerrit | Hongbin Lu proposed openstack/nova master: Handle not found error on taking snapshot https://review.openstack.org/511074 | |
| 21:26:02 | edleafe | mriedem: https://bugs.launchpad.net/nova/+bug/1741125 | |
| 21:26:03 | openstack | Launchpad bug 1741125 in OpenStack Compute (nova) "Instance resize always fails when rescheduling" [High,New] - Assigned to Ed Leafe (ed-leafe) | |
| 21:27:11 | mriedem | thanks; resize in general is probably just broken for the CachingScheduler | |
| 21:31:31 | mriedem | dansmith: we probably can't error out here https://github.com/openstack/nova/blob/f95f165b49fbc0efe29450b0e858a3ccadecedea/nova/conductor/tasks/migrate.py#L47-L48 | |
| 21:32:01 | mriedem | we won't have allocations if you're using the caching scheduler | |
| 21:32:35 | dansmith | hrm | |
| 21:33:27 | dansmith | and not create them either? that'll kinda screw with all the downstream logic for confirm/delete/undo-swap | |
| 21:33:29 | edleafe | if we do check, we should check against the instance or the migration | |
| 21:33:53 | mriedem | dansmith: compute won't create them once everything is pike+ | |
| 21:34:44 | dansmith | that's not what I meant, | |
| 21:35:00 | dansmith | but I guess we can't create the new allocations (on the dest) if we don't have some to copy from the source | |
| 21:35:03 | mriedem | edleafe: i think we can handle the logic in the conductor task based on whether or not we know we're doing a reschedule | |
| 21:35:41 | mriedem | edleafe: in the case of a reschedule, we'd swap the allocation from desthost1 to desthost2 for the migration uuid | |