| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-03 | |||
| 13:05:26 | sean-k-mooney | so nova | |
| 13:06:01 | sean-k-mooney | yes in both cases nova could create a temperoy key and it could delete it at the correct time | |
| 13:06:02 | Luzi | i know that discussion from cinder and glance, there cinder creates a key which glance has to delete - that is not a good practise | |
| 13:06:51 | sean-k-mooney | right that is messy | |
| 13:07:36 | Luzi | sean-k-mooney, that seems good from my perspective, so automatic key creation for shelve and cross-cellresize, given a metadata parameter indicating a formerly encrypted image | |
| 13:08:06 | sean-k-mooney | yep and if that metadata paramter is not set just do what we do today | |
| 13:08:17 | Luzi | yes | |
| 13:08:25 | sean-k-mooney | so you can make it complete transparent to the end user and no api changes needed | |
| 13:09:37 | Luzi | that sounds good, I will add this to the spec :) | |
| 13:16:16 | lyarwood | mdbooth: https://review.opendev.org/#/c/668750/ - thoughts on this? Came across it last night while looking at upstream bugs and suddenly noticed this codepath is dead. We could just keep it but it hasn't been tested in ~3 years so it might be easier to start over with a clean volume driver. | |
| 13:18:18 | mdbooth | lyarwood: Hmm. It doesn't sound like we're going to implement native iscsi any time soon, tbh, due to lacking multipath and no upstream motivation to implement it. | |
| 13:18:35 | mdbooth | Making this just dead code which isn't like to be used any time soon. | |
| 13:18:42 | mdbooth | And which probably doesn't work anyway. | |
| 13:19:51 | mdbooth | lyarwood: Is os-brick based iscsi problematic? | |
| 13:20:42 | lyarwood | mdbooth: it has some rough edges wrt to multipath etc but the basic single path stuff also covered by this native support is QEMU is pretty solid | |
| 13:20:53 | lyarwood | s/is/in/g | |
| 13:21:23 | lyarwood | so yeah I'm fine nuking this for now and if anyone cares enough we can reintroduce it later in a broken out volume driver | |
| 13:21:30 | lyarwood | and have it actually tested in the gate etc | |
| 13:21:41 | mdbooth | lyarwood: +1 from me for nuking unusable code | |
| 13:22:21 | lyarwood | ack thanks | |
| 13:25:54 | shilpasd | efried: Hi, need discussion on https://review.opendev.org/#/c/667952/1/nova/conf/scheduler.py@188 | |
| 13:26:07 | efried | Sure shilpasd | |
| 13:26:29 | efried | I'm sure we must have discussed this during the spec review | |
| 13:26:30 | shilpasd | here saying 'enable_forbidden_aggregates_filter' conf option bydefault enabled | |
| 13:26:46 | efried | It must be about the performance penalty. | |
| 13:26:54 | shilpasd | efried: but what we observed for other request filters, its not so | |
| 13:27:33 | efried | Well, this one is doing a pretty expensive db lookup every time. The other filters are (I think) exclusively local processing. | |
| 13:27:48 | efried | Have you done any kind of performance analysis of this filter? | |
| 13:27:54 | efried | especially on a large database with lots of aggregates | |
| 13:28:39 | shilpasd | efried: not on that large scale | |
| 13:29:13 | efried | dansmith: you were also concerned about the performance of this filter. | |
| 13:30:14 | efried | Having thought about it some, we're better off being conservative unless we can demonstrate that the db lookup is super cheap. | |
| 13:30:25 | efried | So - leave the conf option as is shilpasd :) | |
| 13:30:30 | efried | I'll update the review. | |
| 13:31:07 | sean-k-mooney | efried: i think all the prefilters default to false so for consitency i would keep it as false too | |
| 13:31:21 | shilpasd | efried: great, thank you, we are keeping it as False | |
| 13:31:24 | sean-k-mooney | at least until we have info from deploymetns that its cheap and works well | |
| 13:31:56 | efried | sean-k-mooney: In this case I was suggesting that we remove the toggle entirely. You have to do enough other stuff to opt in, the only benefit you get by having it switched off is that you don't run the code. | |
| 13:32:05 | shilpasd | sean-k-mooney: thank you for opinion | |
| 13:32:31 | sean-k-mooney | ah right ok | |
| 13:33:17 | shilpasd | efried: thanks for discussion | |
| 13:33:22 | efried | yw | |
| 13:44:01 | gibi | stephenfin, efried: I'm +2 on the extra_spec validation spec https://review.opendev.org/#/c/638734 . alex_xu and bauzas had comments earlier. do we want to wait for them to chime in? | |
| 13:44:31 | stephenfin | gibi: alex_xu, sure, but I think bauzas is MIA now since it's summer and he's French | |
| 13:44:58 | gibi | stephenfin: OK, cool with me | |
| 13:46:04 | efried | probably want to get sean-k-mooney to re-ack as well | |
| 13:46:24 | sean-k-mooney | which reivew? | |
| 13:46:30 | efried | he has the medal for most comments in a single review on that one. | |
| 13:46:33 | efried | sean-k-mooney: https://review.opendev.org/#/c/638734 | |
| 13:46:37 | efried | extra spec validation | |
| 13:46:54 | sean-k-mooney | ah ok | |
| 13:47:10 | sean-k-mooney | am ya that is a medal i get a little too often | |
| 13:47:48 | sean-k-mooney | ill quickly re review it now | |
| 13:49:32 | efried | stephenfin: any merit in changing the topic of that spec and its patch (https://review.opendev.org/640733) to ...-extended to match the new bp? | |
| 13:49:48 | efried | I could understand if you want to keep them grouped with the old ones, as currently. | |
| 13:49:51 | efried | either way | |
| 13:49:57 | stephenfin | I'm happy with either too | |
| 13:50:14 | stephenfin | so I'll leave it but won't object if you want to change it :) | |
| 13:50:21 | efried | I only glanced at the number of results https://review.opendev.org/#/q/topic:bp/flavor-extra-spec-image-property-validation+(status:open+OR+status:merged) -- no idea how related/unrelated they actually are. | |
| 13:50:38 | efried | is your work really a continuation of that? | |
| 13:50:55 | stephenfin | To be honest, not really now. It's the same idea but on a broader scale | |
| 13:51:14 | stephenfin | Perhaps I should have a totally different name. Open to suggestions | |
| 13:51:39 | stephenfin | Given that I'm not doing image metadata validation now, a new name would definitely be a good idea | |
| 13:51:40 | efried | Oh, let's not go through the paperwork of changing the blueprint and stuff. | |
| 13:51:54 | efried | but yeah, that's a point | |
| 13:55:02 | sean-k-mooney | cool your going with my stevador suggestion for cyborge extra spec valdaditon or other services. | |
| 13:55:05 | sean-k-mooney | almsot done | |
| 13:55:19 | stephenfin | yeah, it was a good one | |
| 13:57:12 | efried | sean-k-mooney: I saw (but did not read) you discussing image encryption with Luzi. Where are you on that spec? | |
| 13:57:34 | efried | I noticed she added a sentence on (no) impact on lifecycle operations; is that sufficient to address your concern from the previous PS? | |
| 13:58:14 | efried | also lyarwood: are you +1 on that? Your words imply so, but there's no vote. | |
| 13:58:37 | efried | talking about https://review.opendev.org/#/c/608696/ btw | |
| 13:58:47 | sean-k-mooney | efried: nova should create a tempory key on shelve and encypte the snapshot if the instance image was encypted and delete the key when unshelving. same for cross cell resize | |
| 13:58:49 | Luzi | efried, i have to reconsider shelve and cross-cell resize | |
| 13:59:00 | lyarwood | efried: yeah sorry the comments were against an older PS so I couldn't vote | |
| 13:59:15 | efried | okay, cool, thanks all. I'll hold off and wait for a new PS | |
| 13:59:27 | sean-k-mooney | either that or we need to expsoe it on the shelve/resize api endpoint for the end user to decide | |
| 14:00:39 | openstackgerrit | Dakshina Ilangovan proposed openstack/nova-specs master: Spec: Provider config YAML file https://review.opendev.org/612497 | |
| 14:00:53 | efried | sean-k-mooney, Luzi: But that changes everything. | |
| 14:01:28 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Init HostState.failed_builds https://review.opendev.org/668911 | |
| 14:01:30 | sean-k-mooney | efried: what changes everything? auto encypte or api endpoint or both | |
| 14:01:49 | efried | Auto encrypt: As previously written, you didn't keep track of whether a server was originated from an encrypted image. Now you would have to. | |
| 14:01:49 | efried | API endpoint: now you need a microversion (right?) | |
| 14:02:06 | sean-k-mooney | yes | |
| 14:02:26 | sean-k-mooney | too ^ we would need a microversion | |
| 14:02:44 | sean-k-mooney | not sure about keeping track of if it was encypted | |
| 14:03:01 | sean-k-mooney | i think Luzi said there was a metadta key on the image/instance | |
| 14:03:22 | efried | I thought the spec said we forget about that. | |
| 14:03:27 | sean-k-mooney | i assume it would be in the embeded copy of hte image meta we store in the request spec | |
| 14:04:20 | efried | "...the encryption-related | |
| 14:04:20 | efried | additional metadata properties become irrelevant and thus may be discarded | |
| 14:04:20 | efried | instead of being carried over into the server's metadata." | |
| 14:04:24 | Luzi | efried, i thought so until sean-k-mooney asked me, we misinterpreted the word snapshot | |
| 14:06:05 | efried | I'm on the fence about whether it's better to encrypt automatically or on-demand when doing things involving snapshot. | |
| 14:06:36 | efried | but it seemed reasonable as written (on-demand) | |
| 14:06:45 | efried | and simpler | |
| 14:06:50 | sean-k-mooney | either would be ok. what i dont think is ok would be that shelve and cross cell resize alway result in unencypted snap shots with no way to encyrpte them | |
| 14:07:20 | efried | (cross-cell resize creates a snapshot?) | |
| 14:07:23 | sean-k-mooney | so if we do the microversion bump or auto encypt i think both are fine | |
| 14:07:25 | sean-k-mooney | efried: yes | |