| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-20 | |||
| 18:16:43 | mdbooth | However, if you want to review that series ;) | |
| 18:16:53 | artom | Also, if I read your patch correctly, it still uses volume UUID as the serial for volumes | |
| 18:16:56 | artom | So nothing changes there | |
| 18:17:05 | mdbooth | artom: Right, that's correct. | |
| 18:17:12 | mdbooth | So this will continue to work for detach_volume. | |
| 18:17:12 | artom | Then we should be fine | |
| 18:17:23 | artom | Let me check whether I included this in the tempest tests | |
| 18:17:23 | mdbooth | But it wouldn't work for detach_ephemeral | |
| 18:17:29 | mdbooth | ...which doesn't exist. | |
| 18:17:30 | artom | ... | |
| 18:17:34 | artom | which haven't merged yet | |
| 18:17:40 | mdbooth | orly? | |
| 18:17:47 | mdbooth | I need to write tempest tests for my stuff. | |
| 18:17:56 | mdbooth | Can you ping me the reviews? | |
| 18:17:58 | artom | mdbooth, they'll merge in time for brexit | |
| 18:18:20 | artom | mdbooth, https://review.openstack.org/#/c/391947/ | |
| 18:18:44 | mdbooth | artom: You mean they're a complete trainwreck with inexplicable momentum marching inevitably towards their doom? | |
| 18:18:48 | artom | Yeah, they only test the attach case | |
| 18:18:51 | artom | I should add the detach case | |
| 18:19:03 | artom | mdbooth, isn't that all of openstack? ;) | |
| 18:19:19 | mdbooth | Ooh, harsh! :) | |
| 18:19:32 | artom | I'm an arse because I care? | |
| 18:21:14 | mdbooth | artom: Ok, looks like that needs a rebase. | |
| 18:21:47 | mdbooth | I'll take a look at that tomorrow, as I also need to write a test which boots with a tagged root/ephemeral/swap disk. | |
| 18:23:58 | openstackgerrit | rahul bardia proposed openstack/python-novaclient master: nova limits ERROR (Exception): Field names must be unique https://review.openstack.org/527922 | |
| 18:29:13 | artom | mdbooth, sure, thanks :) | |
| 18:29:22 | artom | I'll try to get to yours as well | |
| 18:29:24 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-status check for ironic flavor migration https://review.openstack.org/527541 | |
| 18:45:34 | rybridges | Hey guys I have a quick question. Is there a way to list all instances for a particular user across all projects? | |
| 18:46:24 | rybridges | Is that what openstack server list --user <username> does? | |
| 18:49:21 | openstackgerrit | Chris Dent proposed openstack/nova master: Do not set allocation.id in AllocationList.create_all() https://review.openstack.org/529397 | |
| 18:49:41 | cdent | mriedem: there's ^ a first stab, I went for the simplest thing, for reasons listed in the commit message, gonna dine now. | |
| 18:52:17 | melwitt | rybridges: it appears so, it might require user uuid. and you might also need to pass --all-projects | |
| 18:53:21 | rybridges | ok | |
| 18:53:27 | rybridges | Thanks i will give it a shot! | |
| 18:55:38 | melwitt | let me know if it doesn't work. not that familiar with openstackclient but a glance at the code looks like it should do what you want | |
| 18:56:59 | rybridges | looks like this works: openstack server list --user rybridges --all-projects | |
| 18:57:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass RequestSpec to ConductorTaskAPI.build_instances https://review.openstack.org/515495 | |
| 18:57:10 | rybridges | but this does not: openstack server list --user rybridges | |
| 18:57:16 | rybridges | so ya you need --all-projects | |
| 18:58:27 | melwitt | rybridges: okay, thanks for confirming. without --all-projects it will default to filtering that user under the project making the request | |
| 18:58:47 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass RequestSpec to ConductorTaskAPI.build_instances https://review.openstack.org/515495 | |
| 18:59:30 | rybridges | that is correct melwitt | |
| 19:04:48 | edleafe | cdent: huh, that was way simpler than I thought it would be. | |
| 19:25:01 | cfriesen | maybe odd python style question, but why does nova code use super(ClassName, self).func() rather than super(type(self), self).func()? | |
| 19:25:43 | melwitt | I dunno. is the latter a more common pattern elsewhere? | |
| 19:26:58 | cfriesen | melwitt: well, in python 3 you can just use super(). the nice thing about the second one is that you don't need to update it if you subclass it and copy/paste a snippet of code. | |
| 19:29:03 | cfriesen | melwitt: heh...some additional googling answered my question. turns out in python2 it needs to be explicit to allow for multiple levels of inheritance. | |
| 19:29:39 | melwitt | ah, okay. now I know | |
| 19:39:22 | cdent | edleafe: I could have made it way more complex, but figured, what's the point? | |
| 19:45:59 | openstackgerrit | Chris Dent proposed openstack/nova master: Do not set allocation.id in AllocationList.create_all() https://review.openstack.org/529397 | |
| 19:46:33 | edleafe | cdent: I'm not disagreeing | |
| 19:47:56 | cdent | I know | |
| 19:50:56 | mriedem | edleafe: i think we're pretty close on https://review.openstack.org/#/c/511358/ but i don't know why you had to modify the FilterScheduler in there | |
| 19:52:50 | mriedem | i think you might be working around something in the claims code in the conductor build_instances method that i pointed out | |
| 19:53:07 | mriedem | also, i think claim_resources might be the wrong method to call from conductor, seems put_allocations is what we want instead | |
| 19:55:15 | cdent | I got things done today, but none of the things I mean to to do, so I'm giving up | |
| 19:55:38 | mriedem | that's most of my days | |
| 19:55:55 | cdent | true | |
| 19:56:14 | cdent | but today seemed especially bad. or maybe I just feel guilty for not doing the stuff I said I'd do for efried_cya_jan | |
| 19:56:16 | cdent | anyway | |
| 19:56:19 | cdent | goodnight | |
| 19:56:26 | SamYaple | /win/win 20 | |
| 20:03:05 | edleafe | mriedem: the filter scheduler change was because the functional tests turned up an issue: that the Selection objects in the alternates had no allocation_request in them. Since the Selection objects weren't used until this patch, it never showed up | |
| 20:03:47 | mriedem | ummm | |
| 20:04:15 | mriedem | edleafe: then what is this? https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L380 | |
| 20:04:36 | mriedem | i realize the first selected host doesn't have an allocation_request in it https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L348 | |
| 20:04:51 | mriedem | which you did by design because you said conductor wouldn't need it, because we claim on that host during scheduling | |
| 20:05:01 | mriedem | but the alternates should have the allocation requests | |
| 20:08:59 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Enable limiting GET /allocation_candidates https://review.openstack.org/513526 | |
| 20:09:04 | edleafe | mriedem: it looks like when I rebased on your patch, it pulled an older version. I'm not sure how | |
| 20:09:12 | mriedem | also realized just now that the claim logic in conductor won't work for the caching scheduler, which does have alternates but doesn't do claims | |
| 20:09:24 | mriedem | edleafe: ok, it seems like this is easily fixed | |
| 20:09:26 | edleafe | well, maybe no | |
| 20:09:41 | edleafe | yeah, I'll fix it, but I'm curious how that happened | |
| 20:09:53 | mriedem | my guess would just be like you said, rebase wonk | |
| 20:10:00 | mriedem | when there were lots of patches | |
| 20:10:13 | mriedem | note my latest comment in the conductor manager about the caching scheduler | |
| 20:10:34 | mriedem | we should have a test for that - where we have alternates, but they don't have allocation_request entries | |
| 20:10:40 | mriedem | we still reschedule, but we don't claim | |
| 20:11:05 | edleafe | ok, let me switch gears and start working on that | |
| 20:20:05 | mriedem | i think we should probably run one of our CI jobs with the caching scheduler, but not sure which one | |
| 20:20:17 | mriedem | could be the nova-next job | |
| 20:20:52 | mriedem | if we threw it in an experimental queue job we'd likely break it and not realize | |
| 20:33:15 | mriedem | https://review.openstack.org/529410 | |
| 20:48:57 | edleafe | mriedem: so for the caching scheduler case, I should just assume that if there is no allocaiton_request, that it's fine to build on the host? | |
| 20:50:20 | mriedem | edleafe: i think so yeah | |
| 20:50:35 | mriedem | b/c we still get an alternates list for the caching scheduler | |
| 20:50:42 | mriedem | it just won't have anything to do with placement | |
| 20:50:55 | edleafe | do we get alternates for caching? | |
| 20:51:12 | edleafe | or is it just a single-element list per instance? | |
| 20:51:48 | edleafe | oh wait | |
| 20:52:01 | edleafe | yeah, it still uses the filter scheduler underneath | |
| 21:02:58 | edleafe | so calling put_allocations() instead of claim_resources() would mean having to hack into the allocation_request to extract the resource provider | |
| 21:03:15 | edleafe | One of the goals of allocation_request was it should be opaque | |
| 21:03:38 | edleafe | Just because we've violated that once doesn't mean we should continue to do so in other cases. | |
| 21:04:01 | edleafe | So what if I added a "don't check for moves" param to claim_resources? | |
| 21:08:50 | mriedem | let me look at what you're talking about | |
| 21:10:15 | mriedem | edleafe: oh yeah you're right, | |
| 21:10:19 | mriedem | didn't think about the rp_uuid parameter | |