Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-22
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
23:46:37 sean-k-mooney but we can see if any of the ci job fall over becase of it
23:47:51 sean-k-mooney ok im actully leaving this time o/
#openstack-nova - 2019-02-23
00:31:42 openstackgerrit Merged openstack/nova master: FUP for Id7827fe8dc27112e342dc25c902c8dbc25f63b94 https://review.openstack.org/638370
00:43:29 openstackgerrit Merged openstack/nova master: vmware:add support for the hw_video_ram image property https://review.openstack.org/564193
00:43:37 openstackgerrit Merged openstack/nova master: Remove _legacy_dict methods https://review.openstack.org/636210
00:43:45 openstackgerrit Merged openstack/nova stable/queens: PCI: do not force remove allocated devices https://review.openstack.org/635072
00:43:53 openstackgerrit Merged openstack/nova stable/rocky: Fix typo https://review.openstack.org/633196
00:44:04 openstackgerrit Merged openstack/nova master: Fix irrelevant-files for legacy-grenade-dsvm-neutron-multinode-live-migration https://review.openstack.org/637231
06:10:46 openstackgerrit Merged openstack/nova master: Remove deprecated 'os-server-groups' policy https://review.openstack.org/633672

Earlier   Later