| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-03 | |||
| 20:20:40 | dansmith | sure, | |
| 20:20:52 | dansmith | but then we can move on once identified right? :) | |
| 20:21:02 | sdague | anyway, I tried to build a summary email from all the conversations I saw today | |
| 20:21:03 | cfriesen | dansmith: fair enough...so in mriedem's scenario would we allow user B to replace user A's "mykey" with user B's "mykey" on a rebuild? | |
| 20:21:30 | dansmith | IMHO, if you specify a key, it's looked up based on your context, and if that's your key instead of the original one, then so be it | |
| 20:21:50 | sdague | dansmith: sure, I just think it might be an unexpected thing | |
| 20:21:53 | dansmith | if you don't, then we should keep the same I think | |
| 20:22:14 | dansmith | sdague: if you specify a key? you can't list other people's keys so what other intent could the user have if they specify one? | |
| 20:22:30 | penick | Well for preserving the host key, the private key will need to be stored as well | |
| 20:22:38 | sdague | penick: we don't do that | |
| 20:22:41 | penick | there's no point in keeping a public ssh host key, if you don't preserve the private key | |
| 20:22:43 | dansmith | penick: we do nothing with the host key | |
| 20:22:55 | penick | Oh hell, I completely misunderstood this spec. | |
| 20:22:55 | dansmith | these are all user access keys we're talking about | |
| 20:22:58 | sdague | this is all just the root ssh key | |
| 20:23:02 | sdague | root user | |
| 20:23:09 | dansmith | not even root, | |
| 20:23:14 | sdague | being configed on cloud init | |
| 20:23:17 | dansmith | whatever user the image has for access | |
| 20:23:22 | sdague | dansmith: yeh, right | |
| 20:24:35 | mriedem | cfriesen: ok, so i think what would probably need to happen for rebuild, is if another user does the rebuild, then we have to lookup the keypair by the instance.user_id, | |
| 20:24:38 | mriedem | not the context.user_id | |
| 20:24:56 | dansmith | and doesn't specify a key.. agreed | |
| 20:25:12 | dansmith | we just have to be careful not to expose non-project-user keys when we're doing that override, | |
| 20:25:17 | dansmith | since it happens deep in the db layer I think | |
| 20:25:28 | sdague | yeh, I just wonder what happens in the current code | |
| 20:25:34 | dansmith | sdague: we fail I think | |
| 20:25:46 | penick | ahhh ok. So not a security issue then. Usually people want to preserve the ssh host key between reimages. I do think this is an operability issue if keys stay scoped to only a user, instead of a tenant. | |
| 20:25:48 | mriedem | you can't specify a new key on rebuild today, so i'm lost as to what the concerns are about the current code | |
| 20:26:12 | dansmith | I thought we break if another user tries to rebuild today, no? | |
| 20:26:22 | sdague | dansmith: I don't know if we do, that was the question | |
| 20:26:35 | sdague | rebuild is an instance operation that's project scoped | |
| 20:26:43 | sdague | and keys are user scoped | |
| 20:27:20 | sdague | and I actually haven't checked if B rebuilds a server that A built with A's key if it: a) still has that key, b) has no key, c) errors out | |
| 20:27:53 | sdague | I'll do some testing around that in the morning, my end of day is now | |
| 20:28:06 | dansmith | sdague: right, I thought it was an oopsie on our part that other users couldn't rebuild successfully | |
| 20:28:09 | mriedem | on a rebuild today, we'll use the original key associated with the instance when it was created | |
| 20:28:20 | mriedem | regardless of who rebuilds the isntance | |
| 20:28:24 | dansmith | people want a key during rebuild for taking ownership I think, but that's a destructive operation and not a good argument, IMHO | |
| 20:28:25 | sdague | mriedem: ok, cool | |
| 20:28:52 | dansmith | mriedem: okay, i thought there was a whole big stink about us not being able to look up the key again on rebuild if triggered by another user | |
| 20:29:10 | sdague | dansmith: no, it was a question because of the scoping difference. | |
| 20:29:38 | mriedem | as far as i know, TODAY, we don't ever look up the key during rebuild, | |
| 20:29:48 | mriedem | because you can't change it | |
| 20:29:54 | dansmith | okay | |
| 20:30:17 | mriedem | the only thing i think that happens today, | |
| 20:30:21 | sdague | mriedem: well, it has to be looked up to go in the config drive, right? | |
| 20:30:32 | mriedem | is when driver.spawn happens, we rebuild config drive, and that's going to call the InstanceMetadata code to get the keypair | |
| 20:30:36 | mriedem | to shove into the config drive | |
| 20:30:41 | mriedem | sdague: yes | |
| 20:31:00 | mriedem | and that is the keypair that's stashed in the instance | |
| 20:31:01 | mriedem | just like a flavor | |
| 20:31:03 | dansmith | but we store that on the instance | |
| 20:31:04 | dansmith | right | |
| 20:31:13 | sdague | we store keypair name | |
| 20:31:13 | dansmith | maybe this was broken before and now isn't | |
| 20:31:18 | dansmith | sdague: no we store the whole thing | |
| 20:31:20 | mriedem | we store the whole gd thing | |
| 20:31:25 | sdague | oh | |
| 20:31:25 | dansmith | sdague: keypairs live in the api db and compute can't get them | |
| 20:31:35 | mriedem | https://github.com/openstack/nova/blob/cfff910b0d1a0d9f24b6c1596ceef8dd6b8b3ac6/nova/api/metadata/base.py#L355 | |
| 20:31:46 | sdague | ok, so yeh, maybe cells v2 did move this around and fix a thing | |
| 20:32:24 | dansmith | um, ya'll'er welcome? | |
| 20:32:52 | sdague | dansmith: so, the use case that people seem to want is because keeping IP and device model (mac address) is useful to folks | |
| 20:33:13 | dansmith | but changing ownership via key, yeah | |
| 20:33:14 | mriedem | you also keep your volumes attached | |
| 20:33:23 | sdague | mriedem: right | |
| 20:33:29 | dansmith | I get why people want that | |
| 20:33:38 | dansmith | I wish they didn't want it, but.. | |
| 20:34:01 | sdague | I'd still like a more detailed set of use cases in the spec. | |
| 20:36:57 | openstackgerrit | Matt Riedemann proposed openstack/nova master: api-ref: add note about rebuild not replacing volume-backed root disk https://review.openstack.org/509282 | |
| 20:37:17 | mriedem | ^ is my answer to the buttload of duplicate bugs | |
| 20:43:50 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient stable/newton: Fix aggregate_update name and availability_zone clash https://review.openstack.org/507816 | |
| 20:44:19 | sdague | mriedem: so rebuild on bfv is just a reboot? | |
| 20:44:28 | sdague | it might be better to actually make that a 400 | |
| 20:44:46 | sdague | because it's not actually doing what the user expects | |
| 20:45:02 | dansmith | sdague: but evacuate (rebuild) on BFV is hiiiiiiighly utilized | |
| 20:45:10 | dansmith | which is a little different, granted, but.. | |
| 20:45:17 | mriedem | can't specify a new image on evacuate | |
| 20:45:19 | dansmith | rebuild on bfv with a key becomes more useful | |
| 20:45:24 | dansmith | right I know | |
| 20:45:25 | mriedem | the issue here is you specify a different image on rebuild | |
| 20:45:29 | sdague | right | |
| 20:45:31 | cfriesen | sdague: you can specify a different personality on rebuild | |
| 20:45:32 | dansmith | ah, if you specify an image sure | |
| 20:45:57 | mriedem | i really need to just get a devstack setup and play with some of this a bit, because a lot of the bug reports are really old and confusing, and mixing issues | |
| 20:46:04 | sdague | mriedem: yeh | |
| 20:46:19 | sdague | I was thinking about doing that tomorrow morning | |
| 20:46:24 | mriedem | the linked bug in there shows a recreate where the rebuild doesn't fail but doesn't change the root disk either | |
| 20:46:26 | sdague | and just mapping out the space a bit | |
| 20:46:39 | mriedem | it does change the instance image ref | |
| 20:46:57 | cfriesen | mriedem: I think we hit that one....was sort of confusing due to the mismatch | |
| 20:46:58 | mriedem | but, that's probably because we change that in the api | |
| 20:47:03 | mriedem | i bet the rebuild actually fails on the compute | |
| 20:47:06 | mriedem | because we can't detach the root disk | |
| 20:49:41 | mriedem | also, unrelated, i just updated an approved change and removed something in the commit message, and it re-applied the +W on the patch | |
| 20:49:46 | mriedem | which seems like odd new behavior | |
| 20:49:58 | mriedem | https://review.openstack.org/#/c/507816/5..6 | |
| 20:50:27 | sdague | if it's litterally exactly the same patch, I though all the votes come back | |
| 20:50:54 | sdague | https://review.openstack.org/#/c/507816/1..6 | |