| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-22 | |||
| 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. | |
| 22:35:17 | sean-k-mooney | i wasnt following hte context of what you were working on but it look like it will alway be none none | |
| 22:36:52 | sean-k-mooney | melwitt: you could also get the user_id out of the instance if you have that https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L122-L123 | |
| 22:40:04 | melwitt | sean-k-mooney: I'm adding user_id to InstanceMapping and need a reliable way to populate it (data migration of already existing records). and instance was the assumed way. and it occurred to me (from looking at the down cell stuff) that, request spec has user_id too, could I use that? the answer I now know is, no | |
| 22:40:26 | melwitt | I already wrote everything to use instance user_id, so this means no change for me | |
| 22:40:50 | sean-k-mooney | ah ok | |
| 22:44:00 | sean-k-mooney | melwitt: you could be checky and look at the allocation for the instacne and get the user id out of the consumers table in the api db | |
| 22:44:44 | sean-k-mooney | the consumer and allocation should have been there since pike | |
| 22:51:29 | sean-k-mooney | melwitt: :) oh look like you added the consumer table in pike https://github.com/openstack/nova/commit/b9eed6a2862d0d72c77706b49d05ea588d2d81f3 | |
| 23:04:08 | melwitt | yeah, I dunno | |
| 23:06:00 | melwitt | that might not work bc I need to migrate soft-deleted instances (the soft-delete API) too, since they could restored theoretically at any point in the future. and I don't think those would have allocations, as they are supposed to be gone from the user perspective and not consuming resources | |
| 23:07:29 | openstackgerrit | Merged openstack/nova stable/pike: Not set instance to ERROR if set_admin_password failed https://review.openstack.org/608180 | |
| 23:07:31 | melwitt | besides that, it seems most reliable to use instances. we had bugs in the past with orphaned allocations in placement and deployments out there might still have some | |
| 23:07:36 | openstackgerrit | Merged openstack/nova stable/pike: Add functional regression test for bug 1806064 https://review.openstack.org/623935 | |
| 23:07:37 | openstack | bug 1806064 in OpenStack Compute (nova) pike "Volume remains in attaching/reserved status, if the instance is deleted after TooManyInstances exception in nova-conductor" [Medium,In progress] https://launchpad.net/bugs/1806064 - Assigned to Matt Riedemann (mriedem) | |
| 23:07:37 | sean-k-mooney | ya i guess you have to blance the risks. if the cell is donw then using ht user form the instance will fail | |
| 23:08:24 | sean-k-mooney | melwitt: ya the instace is proably the safest whcih is why i said using the consumer recored associated with the instance allocation was a little cheaky | |
| 23:09:15 | melwitt | oh, cheaky? you spelled it "checky" earlier and I didn't get it :) | |
| 23:09:37 | sean-k-mooney | oh ha ya | |
| 23:10:24 | sean-k-mooney | well it shoudl be cheeky if i use the irish/uk spelling | |
| 23:10:52 | sean-k-mooney | anyway i better go. have a good weekend o/ | |
| 23:11:04 | melwitt | have a good weekend o/ | |
| 23:21:28 | openstackgerrit | Merged openstack/nova stable/pike: Create BDMs/tags in cell with instance when over-quota https://review.openstack.org/623937 | |
| 23:22:40 | mriedem | hmm, why do we persist RequestSpec.ignore_hosts | |
| 23:22:46 | mriedem | dansmith: ^ seems dangerous yeah? | |
| 23:22:55 | mriedem | just like why we don't persist retry and requested_destination | |
| 23:23:41 | mriedem | i.e. resize with allow_resize_to_same_host=false sets ignore_hosts=[instance.host], we save that, then resize again with allow_resize_to_same_host=False will always filter out the same host | |
| 23:24:02 | mriedem | er allow_resize_to_same_host=True the 2nd run | |
| 23:24:28 | mriedem | i know request spec is the thing everyone wants to think about at 3:30 on a friday | |
| 23:24:32 | sean-k-mooney | do we clear it on resize confirm/revert | |
| 23:24:55 | sean-k-mooney | i dont recal if we can retry a resize but perhapse that is why | |
| 23:25:09 | sean-k-mooney | that is just a guess | |
| 23:25:18 | mriedem | well, on the 2nd resize we would overwrite RequestSpec.ignore_hosts again | |
| 23:26:08 | mriedem | looks like the only other move operation that really looks at it is evacuate, and you can't evacuate to the same host anyway so .. | |
| 23:26:24 | mriedem | evacuate ignores instance.host as well | |
| 23:26:35 | mriedem | so i guess this is why it's not a problem | |
| 23:27:27 | mriedem | live migrate also uses it while retrying different hosts in conductor, but doesn't persist those changes (until someone comes along and adds code to the live migration task that saves the request spec for some reason) | |
| 23:29:51 | sean-k-mooney | well it doesnt sound like there is an obvious reason to save it to the db so maybe put up a patch to and see what breaks but form the sounds of it might not be needed and might be a source of a future bug | |
| 23:30:26 | mriedem | no time right now, but yeah i'm sure it would be a future bug, i.e. https://review.openstack.org/#/c/636271/ | |
| 23:30:39 | mriedem | these per-operation fields on request spec keep biting us in our collective asses | |
| 23:43:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Workaround resize to same host doubling allocations bug https://review.openstack.org/638791 | |
| 23:44:05 | mriedem | leakypipes: fried_rice: ^ damn near killed me | |
| 23:44:07 | mriedem | fugly as hell | |
| 23:45:46 | openstackgerrit | sean mooney proposed openstack/nova master: [DNM] stop storing request_spec.ignore_hosts in the db. https://review.openstack.org/638792 | |
| 23:46:13 | sean-k-mooney | i belive ^ is syantaticlly corret python. beyond that its just a reminder | |