| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-12 | |||
| 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 | So I think you're saying that I should be able to tell based on whether project_id is set? | |
| 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 | or maybe I should just say what I'm actually trying to do: | |
| 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 | mriedem | i thought about it but didn't since this was origially much more special case to this api method | |
| 21:02:38 | efried | power_on | |
| 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 | |
| 21:05:22 | mriedem | if i'm left to do whatever i want, i'm going to address mdbooth's comments and keep the series going. if there are things around this to be done later, i say let's do them later. | |
| 21:05:49 | dansmith | aight | |
| 21:06:56 | sean-k-mooney | efried: for libivrt you might want to create and remove the securte here https://github.com/openstack/nova/blob/7aa88029bbf6311033457c32801963da01e88ecb/nova/virt/libvirt/driver.py#L6188 | |
| 21:07:36 | sean-k-mooney | in libvirt that is | |
| 21:07:52 | sean-k-mooney | you would have to add the secret to the xml before | |
| 21:09:07 | sean-k-mooney | anyway night o/ | |
| 21:17:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Start functional testing for cross-cell resize https://review.opendev.org/636253 | |
| 21:17:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Filter duplicates from compute API get_migrations_sorted() https://review.opendev.org/636224 | |
| 21:17:03 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Validate image/create during cross-cell resize functional testing https://review.opendev.org/642592 | |
| 21:17:03 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Handle target host cross-cell cold migration in conductor https://review.opendev.org/642591 | |
| 21:17:04 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add negative test for cross-cell finish_resize failing https://review.opendev.org/643451 | |
| 21:17:04 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add zones wrinkle to TestMultiCellMigrate https://review.opendev.org/643450 | |
| 21:17:05 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add confirm_snapshot_based_resize_at_source compute method https://review.opendev.org/637058 | |
| 21:17:05 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add negative test for prep_snapshot_based_resize_at_source failing https://review.opendev.org/669013 | |
| 21:17:06 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add confirm_snapshot_based_resize conductor RPC method https://review.opendev.org/637075 | |
| 21:17:06 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add ConfirmResizeTask https://review.opendev.org/637070 | |
| 21:17:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add revert_snapshot_based_resize_at_dest compute method https://review.opendev.org/637630 | |
| 21:17:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Confirm cross-cell resize from the API https://review.opendev.org/637316 | |
| 21:17:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add finish_revert_snapshot_based_resize_at_source compute method https://review.opendev.org/637647 | |
| 21:17:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Deal with cross-cell resize in _remove_deleted_instances_allocations https://review.opendev.org/639453 | |
| 21:17:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add revert_snapshot_based_resize conductor RPC method https://review.opendev.org/638047 | |
| 21:17:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add RevertResizeTask https://review.opendev.org/638046 | |
| 21:17:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Confirm cross-cell resize while deleting a server https://review.opendev.org/638268 | |
| 21:17:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Revert cross-cell resize from the API https://review.opendev.org/638048 | |