| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-22 | |||
| 21:05:59 | mriedem | so on this same host resize bug, i started down the path of, from conductor, just trying to PUT allocations for the max of the old/new flavor to see if that can fit the host, | |
| 21:06:13 | mriedem | but then remembered, oh yeah the new flavor can have required/forbidden traits which could filter out the same host | |
| 21:06:24 | mriedem | f me right in the eye | |
| 21:06:32 | mriedem | leakypipes: ^ | |
| 21:07:46 | mriedem | i basically have to do a GET /a_c call from conductor and see if the same host provider is in the results | |
| 21:08:07 | mriedem | and then not swap allocations | |
| 21:08:33 | edleafe | mriedem: that sounds like the same problem Watcher had | |
| 21:08:50 | mriedem | the ol scheduler dry run | |
| 21:09:49 | fried_rice | mriedem: You can use ?in_tree with microversion 1.31 | |
| 21:10:18 | mriedem | that's not a thing in stein right | |
| 21:10:29 | fried_rice | Could be | |
| 21:10:44 | fried_rice | I mean, it's not merged yet, and we agreed not to use it from nova if it does merge. | |
| 21:11:12 | mriedem | this change isn't making stein anyway | |
| 21:11:14 | fried_rice | We could end up resizing to same-host-but-different-providers, couldn't we. | |
| 21:11:20 | mriedem | sure can | |
| 21:11:30 | mriedem | if there are nested providers in the new flavor then it's all f'ed either way | |
| 21:12:00 | fried_rice | I don't know about f'ed. Just have to be pretty careful about which ones we allow to move and which we don't. | |
| 21:12:19 | fried_rice | Like, I can see moving numa nodes, as long as all the affined resources move together. | |
| 21:12:31 | fried_rice | But I can't see moving FPGAs probably. | |
| 21:12:51 | fried_rice | for now I wouldn't remotely object to "fail if any nested/sharing in play". | |
| 21:12:53 | mriedem | i think i would put a big fat "don't go down this side path if you have complex allocations" condition | |
| 21:13:01 | fried_rice | yeah, that. | |
| 21:13:13 | fried_rice | bbiab | |
| 21:21:03 | mriedem | this would also bypass anything that has limits to claim in the resource tracker, like numa | |
| 21:21:11 | mriedem | so no numa, no complex allocations | |
| 21:27:15 | melwitt | hm, I wonder why we populate the instance mapping with a cell in build_instances when it is _not_ a reschedule. build_instances method + not a reschedule = cells v1, I thought. so why update instance mapping | |
| 21:28:02 | mriedem | we do'nt know the cell in that case until the scheduler tells us the host right? | |
| 21:28:05 | leakypipes | mriedem: don't forget ye old aggregate image properties filter too. :) | |
| 21:28:16 | mriedem | leakypipes: image doesn't change on resize | |
| 21:28:18 | mriedem | thank f | |
| 21:28:50 | leakypipes | ah, yes, was thinking rebuild/evac | |
| 21:29:03 | mriedem | image doesn't change on evac either | |
| 21:29:04 | mriedem | silly pants | |
| 21:29:28 | mriedem | we need a jump to conclusions mat for these operations | |
| 21:29:34 | mriedem | new host? yes/no/maybe | |
| 21:29:38 | mriedem | new flavor? yes/no/maybe | |
| 21:29:43 | mriedem | new image? yes/no/maybe | |
| 21:30:01 | mriedem | aneurysm? definitely. | |
| 21:30:31 | melwitt | there's a note in the code saying that if it's a reschedule, it's already been set to a cell on the first schedule attempt https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L717-L718 | |
| 21:30:57 | melwitt | so how can the first schedule attempt in a cells v2 env ever be calling build_instances (and not schedule_and_build_instances) | |
| 21:30:58 | mriedem | melwitt: right, so that if is False | |
| 21:31:26 | mriedem | https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L721 is the first time through for cells v1 | |
| 21:31:34 | mriedem | after picking a host, need to update the instance mapping with the cell of the selectd host | |
| 21:31:44 | mriedem | else you are rescheduling https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L738 | |
| 21:32:04 | melwitt | I see that, but we need to update instance mapping for cells v1? | |
| 21:32:32 | mriedem | what we need to do is delete cells v1 | |
| 21:32:38 | melwitt | yes we do | |
| 21:33:01 | mriedem | for cells v1 i assume the instance mapping is updated for the same reason as v2 - to know where to pull the instance information on a GET | |
| 21:33:17 | mriedem | because if it's not in the mapping, we'd pull from the top level API DB i geuss which has the synced info | |
| 21:33:23 | melwitt | yeah, because code path is the same? yeah, guh | |
| 21:33:42 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2449 | |
| 21:33:49 | mriedem | we don't look at the mapping in that case | |
| 21:34:02 | mriedem | so idk | |
| 21:34:05 | melwitt | ok. just going through this realizing I'm not going to have a "populate instance mapping user_id" opportunity during a reschedule, at least not from any already existing instance mapping update | |
| 21:34:08 | mriedem | maybe it's for the v1 -> v2 transition | |
| 21:34:33 | mriedem | why would you need to update the user_id during a reschedule? | |
| 21:34:35 | melwitt | yeah, must be | |
| 21:34:59 | mriedem | we only reschedule (from compute) in 2 cases, server create and resize | |
| 21:35:02 | melwitt | I don't have to, but I had been thinking use all the opportunities for setting user_id if it's not set | |
| 21:35:10 | mriedem | in either of those cases, you could have already updated the instance mapping at the top | |
| 21:35:52 | mriedem | i.e. https://github.com/openstack/nova/blob/master/nova/conductor/tasks/migrate.py#L167 | |
| 21:36:02 | melwitt | yeah, once the new host is picked and saved, that should be where. I guess I missed that mapping update | |
| 21:36:31 | mriedem | the host shouldn't have anything to do with the instance mapping... | |
| 21:36:38 | mriedem | you're just looking for places that InstanceMapping.save() happens yes? | |
| 21:37:13 | mriedem | hell you could do it right here and hit all move operations https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L75 | |
| 21:37:19 | melwitt | oh wait no, during a reschedule we wouldn't update the mapping bc that's just for the cell, not the host. gah I am just confusing myself | |
| 21:37:20 | melwitt | yes | |
| 21:38:00 | melwitt | yeah, I was piggy backing on any existing save. there won't be a new save for a reschedule, is what I realized | |
| 21:38:31 | mriedem | nor any move as far as i know, unless you add one to ^ | |
| 21:38:31 | melwitt | which I could just punt on, but that code in build_instances made me wonder what it was for | |
| 21:39:47 | melwitt | yeah | |
| 21:40:32 | melwitt | initially I thought, since I saw "populate_instance_mapping" in build_instances, that there was a save() to piggy back. but looking closer, there isn't | |
| 21:40:46 | melwitt | not a big deal | |
| 22:21:18 | melwitt | I wonder if I could use request specs to populate user_id instead of looking in cells at all? hmmm | |
| 22:22:42 | sean-k-mooney | melwitt: https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py#L65-L66 | |
| 22:22:51 | sean-k-mooney | the user and project ides are stored in it | |
| 22:23:10 | melwitt | yeah, I was noticing that in compute_api.get() | |
| 22:24:22 | melwitt | I'm a bit wary if there's any way it couldn't be relied on though. being that request specs are only removed at archive_deleted_rows time, I wonder if it's possible that old, unarchived things could have a null user_id | |
| 22:24:52 | melwitt | maybe best not to chance it | |
| 22:25:43 | sean-k-mooney | i honest taught the scduler depending on this to look up the users limets | |
| 22:26:21 | melwitt | it looks at build requests, in addition to instances | |
| 22:26:59 | melwitt | which are ephemeral (only live until scheduling completes) | |
| 22:27:25 | melwitt | request specs are not looked at for any quota limit checking | |
| 22:28:00 | sean-k-mooney | hum ok | |
| 22:29:03 | sean-k-mooney | are you concured that the user may be delete and we some how have a non archived instance belonving to that user and try to look it up and get an error | |
| 22:29:21 | sean-k-mooney | or are you concured that the request spec will have a null user id | |
| 22:30:23 | melwitt | null user_id, or worse, no user_id field. I didn't look at the history of when that field was added | |
| 22:30:50 | melwitt | I guess "no user_id field" isn't a thing, it would be null in that case | |
| 22:31:32 | sean-k-mooney | melwitt: https://github.com/openstack/nova/commit/6e49019fae80586c4bbb8a7281600cf6140c176a | |
| 22:31:46 | melwitt | request spec has a wild west reputation, so I'm just not sure if I could rely on its user_id field. being that I didn't dig into it yet | |
| 22:32:18 | melwitt | ah, wow. relatively new | |
| 22:32:37 | sean-k-mooney | well its been there since queens? | |
| 22:32:52 | melwitt | and no data migration, so would be null for already existing request specs when that code landed | |
| 22:33:19 | melwitt | yeah, I just expected it would be older than that | |
| 22:33:30 | melwitt | ok, nevermind my request spec flighty idea | |
| 22:34:35 | sean-k-mooney | well looking at the change it look like there are check in the conductor that depend on both the user id and porject id being set | |
| 22:34:56 | mriedem | fried_rice: well, i got something working for the functional regression test at least | |
| 22:35:00 | melwitt | hah, ok | |
| 22:35:05 | mriedem | of course it's rife with TODOs and FIXMEs | |
| 22:35:09 | fried_rice | neat. | |