| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-28 | |||
| 15:18:16 | mriedem | and then spawning from that snapshot in the target cell and then delete the temporary snapshot, like unshelve | |
| 15:18:32 | mriedem | so the fix here about checking the vm_state wont' work for cross-cell resize | |
| 15:19:26 | lyarwood | yeah true | |
| 15:19:30 | mriedem | we could check something like the task_state used during that part of the cross-cell resize spawn, but that's going to be used during a normal resize as well, and i'm not sure if flatten should be used there | |
| 15:19:45 | mriedem | we could temporarily hack something into the image meta or the instance system_metadata to be read by the driver, but again that's super hacky | |
| 15:19:59 | lyarwood | You could make it optional for resize and spawn tbh | |
| 15:20:14 | mriedem | optional how? | |
| 15:20:44 | lyarwood | with a configurable, but it would slightly slow down spawns and resizes etc in any env that used it | |
| 15:20:54 | lyarwood | so maybe not | |
| 15:20:59 | mriedem | i think i know a bit better solution, | |
| 15:21:20 | mriedem | the instance.migration_context should be set during a cross-cell resize, and from that we can get the Migration record which will have a cross_cell_move boolean on it so i'd just check that | |
| 15:21:22 | mriedem | from the driver | |
| 15:21:54 | mriedem | it's not fun but it's better than driver interface changes or hacking metadata temporarily or checking vm/task state | |
| 15:22:12 | lyarwood | ack that works | |
| 15:22:36 | lyarwood | we don't have a migration record for shelve/unshelve right? | |
| 15:22:49 | lyarwood | IOW I haven't missed an easier way of fixing my issue? | |
| 15:23:19 | lyarwood | ignore me, of course we don't. | |
| 15:23:20 | mriedem | nope | |
| 15:24:33 | dansmith | that's interesting, | |
| 15:24:36 | dansmith | I've never thought about that | |
| 15:25:01 | dansmith | a migration record would let us provide some unshelve (whilst offloaded) host affinity | |
| 15:25:14 | dansmith | not sure how important that is really, but... | |
| 15:25:59 | efried | dansmith, mriedem: How do y'all feel about reorganizing how request filters are enabled via conf? | |
| 15:25:59 | efried | As it stands, it looks like we're headed for random options scattered around various groups. | |
| 15:25:59 | efried | Wondering if it would make sense e.g. to collect them into [request_filter] $name = {enabled|disabled} | |
| 15:25:59 | efried | or [scheduler] request_filters_enabled = $name[,...] | |
| 15:25:59 | efried | or ... | |
| 15:25:59 | efried | My hesitation is that that's really more of a dev-oriented structure; I don't know that the filters are really similar enough in spirit that that would make sense to the operator. | |
| 15:25:59 | efried | Thoughts? | |
| 15:26:11 | mriedem | dansmith: what do you mean? unshelve back to the original host? | |
| 15:26:39 | dansmith | efried: I think we should aim for enabling behaviors not filters | |
| 15:26:48 | dansmith | efried: and we should aim for most of those to be always on by default | |
| 15:27:17 | mriedem | i'd like to avoid a list like enabled_filters | |
| 15:27:27 | dansmith | yeah | |
| 15:28:03 | dansmith | people enable or disable filters based on name, without really knowing that their behavior can be composite of multiple things | |
| 15:28:43 | efried | okay, that's kind of where my head was going, thanks for the validation. | |
| 15:30:05 | dansmith | mriedem: yes, for unshelve affinity back to the original host, after offload | |
| 15:30:23 | mriedem | dansmith: we save off the original host in system_metadata | |
| 15:30:38 | dansmith | oh, do we? | |
| 15:30:58 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L5112 | |
| 15:31:21 | dansmith | okay | |
| 15:31:51 | dansmith | a migration record would make that a little less special-to-shelve-y and more a record of the move | |
| 15:31:58 | dansmith | but anyway, just hadn't thought about it before | |
| 15:32:27 | mriedem | yeah i'm not sure how much it matters either, | |
| 15:32:33 | mriedem | the original host could be gone by the time you unshelve | |
| 15:32:43 | dansmith | sure, it'd just be a weight thing | |
| 15:32:50 | mriedem | right, was just going to say that, | |
| 15:32:54 | mriedem | pass a weight hint to the scheduler | |
| 15:32:58 | mriedem | if you can, do else no biggy | |
| 15:33:02 | dansmith | right | |
| 15:33:12 | dansmith | in "edge" cases, | |
| 15:33:29 | dansmith | where you have two boxes at a site that are largely for hosting specific types of things, | |
| 15:33:42 | dansmith | you'd have better image affinity | |
| 15:34:02 | dansmith | if you're enforcing it then it doesn't matter | |
| 15:34:08 | dansmith | anyway, | |
| 15:34:45 | dansmith | just a thought that shelve could be more of a move for harmony with the other ops | |
| 15:44:07 | openstackgerrit | Eric Fried proposed openstack/nova master: Support filtering of hosts by forbidden aggregates https://review.opendev.org/667952 | |
| 15:46:59 | efried | donnyd: Yo Mr. Docs, would you mind giving this a scan and seeing if it makes sense to you with your operator hat on? https://review.opendev.org/#/c/667952/2/doc/source/reference/forbidden-aggregates.rst | |
| 15:47:54 | efried | mriedem: btw, that's what all those handy dandy CLIs were for ^ | |
| 15:50:35 | mriedem | jesus that is a large patch, | |
| 15:50:38 | mriedem | can that be split up? | |
| 15:50:48 | donnyd | It reads fine to me. I would just use the same terminology everywhere. we call the function host-aggregate, so when referring to aggregate i would sub for host-aggregate | |
| 15:50:56 | donnyd | or whatever the proper term is | |
| 15:52:25 | mriedem | maybe it's mostly testing and docs... | |
| 15:52:27 | efried | mriedem: It could maybe be split up, but it would be kind of contrived. It's pretty easy to read imo | |
| 15:52:28 | efried | yes. | |
| 15:53:24 | donnyd | Yea, i had no issue at all understanding what is trying to be conveyed | |
| 15:53:33 | efried | donnyd: Thanks for the look. I actually don't know what the accepted term is, I probbaly flubbed several of those. | |
| 15:54:03 | efried | donnyd: (mriedem was talking about the patch as a whole, not specifically the doc) | |
| 15:54:38 | efried | I'll just go on record as saying I think this is a pretty darn cool feature. | |
| 15:55:20 | efried | Considering how weird and complex the implementation is, the external-facing usage model is pretty slick. | |
| 15:56:04 | efried | the most effed up part is how you have to collect the existing traits before you can add the one you want to the host :P | |
| 15:56:22 | efried | ...which of course has nothing to do with the forbidden aggregates feature. | |
| 15:57:43 | dansmith | is this already merged? | |
| 15:57:48 | mriedem | ok ok you win, we can add the PUT /resource_providers/{uuid}/traits/{trait_name} you've always wanted | |
| 15:58:03 | dansmith | I'm confused about how you get the exclusion just because one flavor is tagged | |
| 15:58:10 | mriedem | just like this fine mess https://developer.openstack.org/api-ref/compute/#server-metadata-servers-metadata | |
| 15:58:27 | dansmith | oh jeez, | |
| 15:58:29 | efried | dansmith: not merged | |
| 15:58:31 | dansmith | this is implementation and docs too | |
| 15:58:36 | dansmith | thought this was just a doc patch | |
| 16:00:10 | efried | mriedem: I'm not pulling for an *API* to add a trait, though that wouldn't be terrible. I don't mind if osc-placement does a get-add-replace. What I mind is that the CLI user has to do that. That sucks. | |
| 16:00:29 | efried | but we've already talked about this, hence the story | |
| 16:05:51 | mriedem | efried: i was joking about the api | |
| 16:06:09 | mriedem | the cli sugar should happen, it just needs a warm body, | |
| 16:06:15 | efried | yuh | |
| 16:06:23 | mriedem | speaking of which, i remember someone telling me recently they wanted to work on some code... | |
| 16:06:44 | efried | I think I have zero client patches to my name | |
| 16:06:58 | efried | actually, I might have an ironicclient patch or two :P | |
| 16:07:00 | mriedem | consider this an opportunity for growth | |
| 16:11:33 | mriedem | efried: dustinc: lots of comments and such in https://review.opendev.org/#/c/662881/ but +2; i'm going to post to the ML that i think other cores should be on board with this or nack it before EOD next tuesday for the spec review sprint | |
| 16:12:26 | efried | I count | |
| 16:12:26 | efried | 3 python-ironicclient | |
| 16:12:26 | efried | 1 python-glanceclient | |
| 16:12:26 | efried | 1 python-cinderclient | |
| 16:12:26 | efried | And pretty sure none of those were actually CLI. | |
| 16:12:37 | efried | thanks mriedem | |
| 16:36:10 | sean-k-mooney | i havent reviewed https://review.opendev.org/#/c/662881/8/specs/train/approved/openstacksdk-in-nova.rst | |
| 16:36:30 | sean-k-mooney | but im in favor of using the sdk in general | |
| 16:37:01 | sean-k-mooney | ill try to read it while i wait for dinner but ill proably be +1 on it | |