Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-11-12
20:30:20 sean-k-mooney for a migration object
20:30:26 sean-k-mooney and ya i could
20:30:42 mriedem we could just say f it and do like get_all and filter out by uuid after we've already processed one
20:31:13 mriedem this https://github.com/openstack/nova/blob/master/nova/compute/api.py#L2883
20:31:49 dansmith mriedem: yeah
20:32:12 mriedem the migration from the first cell processed in the results is always the one that is returned,
20:32:20 mriedem which could be wrong, but i don't really care at this point
20:32:40 dansmith does it matter/
20:32:51 dansmith you just want to not return dupes right?
20:33:09 mriedem yes, but also figure that since we can know which is "newer" we should return that,
20:33:29 mriedem because the migration in the target cell is the one that's going to be getting updated as the resize progresses
20:33:42 mriedem including when it goes to 'finished' status and the server is in VERIFY_RESIZE status
20:34:16 sean-k-mooney mriedem ya i think the nestest makes sense
20:34:17 mriedem which is kind of important since i think i have functional tests later in the series asserting the migratoins api returns 1 and it's the correct status
20:34:26 dansmith mriedem: once you create the new one, couldn't you set the hidden flag on the old one or something/
20:34:48 mriedem sure, that's the alternative mentioned in the code and the commit message
20:34:59 mriedem but it has implications for the api because that hidden field has never been set before
20:35:01 dansmith the *newest* only makes sense if we never update the old migration
20:35:41 dansmith mriedem: I don't understand what the problem with that is, as I said.. api ignores migrations with the hidden field already ...
20:35:50 mriedem i don't think we do update the source cell migration unless we revert the resize
20:36:13 dansmith we just leave it pending forever?
20:36:36 mriedem no, when you confirm the resize in the target cell we hard destroy the source cell instance which will hard destroy it's related migratoin in the source cell db,
20:36:44 mriedem same (in reverse) for revert
20:37:02 dansmith what I meant is.. we leave it in the half-finished state until that time comes
20:37:04 mriedem otherwise you can never resize back to that cell b/c of the duplicate instance uuid
20:37:13 mriedem pretty sure yeah
20:37:21 sean-k-mooney so in that case cant we have teh db return the latest version based on update or created time
20:37:29 mriedem sean-k-mooney: different dbs
20:37:30 mriedem so no
20:38:10 sean-k-mooney but we only need to look at the one of the dbs if we only want the latest one right? i gues we would have too look at all of them during a migration
20:38:18 sean-k-mooney ya ok
20:38:25 dansmith sean-k-mooney: maybe you should read the patch we're discussing?
20:38:43 sean-k-mooney i am but im also about to head off for the night
20:39:51 sean-k-mooney the reason i changed my mind half way true was the Verify_resize comment
20:43:37 mriedem so it's not just the active migration that is a duplicate either, it's all migrations linked to the instance - those get copied into the target cell db early in TargetDBSetupTask, because once we flip the instance mapping that's where the api is going to go to get migrations for that server,
20:43:59 mriedem so if we started setting the hidden field on migrations, we'd have to do it on all of them until we're sure where the server is going to ultimately be
20:44:38 efried If I initiate an operation as an admin user, does the context have is_admin set, or does that only happen when nova generates an admin context explicitly?
20:44:51 mriedem is_admin is set
20:45:23 mriedem the difference is one has a token/user/project and the one nova creates for some periodics and stuff doesnt
20:46:35 mriedem https://github.com/openstack/nova/blob/master/nova/context.py#L141 https://github.com/openstack/nova/blob/master/nova/policy.py#L197 https://github.com/openstack/nova/blob/master/nova/policies/base.py#L24
20:48:25 sean-k-mooney mriedem: without your patch do we ignore the cell maping and simple cast to all cells by the way or just the cell where the instnce is
20:48:55 mriedem for what api?
20:49:01 mriedem GET /os-migrations iterates all cells
20:49:49 efried mriedem: perhaps the better question is: can I create an instance as an admin user?
20:49:50 sean-k-mooney based on what you jsut said i would not need too right since we would be copying all the migrations to the new cell
20:50:10 artom sean-k-mooney, yeah I am le much flabbergast
20:51:46 sean-k-mooney efried: an admin user is just a normal user with extra privladtes so it can have a project and a quota
20:52:20 sean-k-mooney efried: so the admin use that gets set up with devstack for example can
20:52:37 efried or maybe I should just say what I'm actually trying to do:
20:52:37 efried While generating libvirt xml, I need a context. The way the code is set up right now, sometimes the context comes from the user and sometimes it comes from get_admin_context(). I want to detect (and reject) the latter.
20:52:37 efried So I think you're saying that I should be able to tell based on whether project_id is set?
20:53:13 sean-k-mooney why do you want to reject the later
20:53:24 sean-k-mooney admin users shoudl be able to create vms with vtpm
20:53:45 sean-k-mooney just not on behalf of another user/project
20:53:54 mriedem sean-k-mooney: we need to copy the migrations to the target cell db because, like i said above, if the user confirms the resize there we hard-destroy the instance and it's old migrations in the source cell db
20:54:31 sean-k-mooney mriedem: yes im just wondering of we can optimese the /os_migration api to only look at the cell where the vm is currently
20:54:46 mriedem efried: a context created by nova.context.get_admin_context won't have a project_id, correct
20:54:57 sean-k-mooney that info should be in the instnace maping in the api db right?
20:54:59 efried sean-k-mooney: Like we were talking about yesterday, I need to be able to bounce on code paths where $not_the_owner_of_the_instance is trying to generate xml with a vtpm secret in it.
20:55:25 mriedem sean-k-mooney: what info?
20:55:27 efried I guess I could just let the key retrieval fail.
20:55:37 sean-k-mooney mriedem: which cell the vm is in
20:55:41 dansmith mriedem: sean-k-mooney has not read the patch and doesn't realize we're listing all migrations, not just for a specific instance
20:55:43 mriedem correct
20:55:50 sean-k-mooney dansmith: yes i have
20:56:07 sean-k-mooney oh i though you were listing all migrtion for the instace
20:56:09 mriedem sean-k-mooney: but the instance mapping saying the instance is in cell X doesn't mean the migration records from cell Y are in cell X unless we copy them there
20:56:12 sean-k-mooney not all migrtions in general
20:56:18 mriedem GET /os-migrations is not instance specific
20:56:22 mriedem it's like GET /servers
20:56:39 mriedem GET /servers/{server_id}/migrations is instance and cell specific
20:56:44 sean-k-mooney right sorry i was confusing it with the iserver one
20:57:23 dansmith efried: I would prefer you try and fail to get/list the key vs. inspect the context and try to detect that it isn't "real"
20:57:55 sean-k-mooney efried: you dont need to reject it you can let barbican do that as it will reject your token when you try to get the secret
20:57:58 efried dansmith: ack. I'm trying to verify whether there are situations where I want to generate the XML but am not trying to boot the instance.
20:58:13 efried In such cases it would be okay to inject the secret_uuid but not define the secret itself to libvirt
20:58:45 dansmith efried: we pass pieces of xml to libvirt in operations like attach volume, but not the whole thing I think, so probably not relevant
20:59:17 efried relevant in that I would need to make sure I don't hit the key manager in such operations.
20:59:30 dansmith efried: but still, I imagine you can't define the xml for the guest referencing a secret that you've since deleted (which you said you were going to do) right?
20:59:46 efried I can, as long as I don't try to boot the thing.
21:00:03 dansmith okay I would have expected that to fail at define time
21:00:05 efried the guest XML only contains the UUID, which (if the instance was ever booted) I've stored in its sysmeta.
21:00:20 sean-k-mooney ya i dont think you will hit that in novas usage
21:00:29 efried I may be misunderstanding, but I think _get_guest_xml just returns an etree, doesn't actually try to libvirtify it.
21:00:40 dansmith efried: sure
21:00:59 dansmith efried: I guess I thought you meant generate the xml as in... update the definition in libvirt
21:01:05 sean-k-mooney the only case i am aware off was the requet to "create a server but not boot it" but we dont supprot that currently
21:01:20 efried I possibly need to rethink the flow here...
21:01:42 efried because I *actually* only want to define the secret to libvirt on boot-ish operations anyway. And then delete it right after.
21:02:00 dansmith right so in driver.spawn()
21:02:20 mriedem dansmith: if moving this get_newer_obj method to NovaObjectBase makes it less gross for you i can do that
21:02:21 efried also cold boot?
21:02:32 dansmith efried: cold boot? :)
21:02:36 dansmith efried: start/
21:02:38 efried power_on
21:02:38 mriedem i thought about it but didn't since this was origially much more special case to this api method
21:02:52 sean-k-mooney cold boot or start call power_on which for libvirt calls hard_reboot which calls spawn
21:03:12 dansmith mriedem: honestly I just hate the way that logic is laid out, probably irrationally, but before even looking for test gaps I knew it would be annoying to validate
21:03:31 dansmith mriedem: the fact that there was one just makes me scared, but I'm clearly the only one, so just do whatever you want, I say
21:04:31 mriedem i expected something like this to already exist but didn't find anything

Earlier   Later