| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-14 | |||
| 21:15:18 | mriedem | jogo: thanks, and o/ | |
| 21:17:34 | jogo | mriedem: o/ glad gerrit lets you drop a -2 even if you can't -2 new things | |
| 21:20:04 | mriedem | jogo: you can always come back | |
| 21:20:44 | jaypipes | mriedem, dansmith, efried: don't forget about this other aggregate-related spec... https://review.openstack.org/#/c/529135/ | |
| 21:21:16 | jaypipes | dansmith: https://blueprints.launchpad.net/nova/+spec/placement-mirror-host-aggregates is ready for you to link to in your spec. | |
| 21:21:18 | efried | jaypipes: I can look again; maybe my opinion has changed. | |
| 21:21:34 | jaypipes | dansmith: working on the spec for that one now. | |
| 21:21:37 | efried | (the "opinion" that I don't understand most of it) | |
| 21:21:47 | dansmith | jaypipes: thanks | |
| 21:21:55 | jaypipes | efried: np. was just pointing out all my agg-related stuffs... | |
| 21:22:16 | efried | No need to get all aggro | |
| 21:22:30 | dansmith | jaypipes: the one I really need is the bp for adding aggregates to the allocation_candidates api | |
| 21:22:43 | efried | dansmith: https://review.openstack.org/#/c/544694/1 | |
| 21:23:09 | dansmith | ah cool | |
| 21:25:08 | jaypipes | dansmith: https://blueprints.launchpad.net/nova/+spec/alloc-candidates-member-of | |
| 21:25:16 | jaypipes | dansmith: yup, pushed it up about 20 minutes ago | |
| 21:25:19 | dansmith | jaypipes: yeah, found it thanks | |
| 21:25:35 | jaypipes | np | |
| 21:26:38 | dansmith | efried: yawna look at my replies before I push up this revision with the linkages? | |
| 21:26:46 | mriedem | god i see i need to read https://review.openstack.org/#/c/544585/ again | |
| 21:27:05 | efried | dansmith: You mean on ^ ? Looking... | |
| 21:27:14 | dansmith | yeah | |
| 21:29:02 | mnaser | functional tests done (and failing), crossing fingers that putting the change on top fixes things | |
| 21:29:09 | jogo | mriedem: :) | |
| 21:29:40 | openstackgerrit | Dan Smith proposed openstack/nova master: Remove deprecated aggregate DB compatibility https://review.openstack.org/544698 | |
| 21:31:58 | jaypipes | aggregates are just the topic du jour I guess. | |
| 21:32:08 | efried | dansmith: Can you please clarify the traits thing for me? What I suggested in L119 was yes, "bulk adding traits to large swaths of compute nodes" - but instead of getting the trait to the API call by pulling it from the flavor, you get it from the RequestSpec (or whatever) and fold it into any traits provided from the flavor. | |
| 21:32:30 | efried | dansmith: Is that what you're suggesting could be a solution for boot-from-volume in the future? | |
| 21:32:31 | dansmith | efried: clarify how a trait in a flavor doesn't help? | |
| 21:32:38 | dansmith | efried: ha | |
| 21:32:40 | dansmith | *ah | |
| 21:32:58 | dansmith | efried: you could have flavors you allow BFV from but that aren't dedicated to bfv | |
| 21:32:59 | efried | dansmith: Getting the trait from not-flavor to avoid Skittles. | |
| 21:33:47 | dansmith | efried: so if you want to make sure that BFV requests using generic flavors don't get routed to any old host, you might want to slap a trait on those requests so that you can send them to the right place | |
| 21:33:59 | mriedem | my idea was that the pre-placement filter thing dan's spec is adding, would shove a trait into the flavor embedded in the reqspec b/c that's currently how we get the traits from the user to the scheduler to placement | |
| 21:34:28 | efried | Right, so we're merging traits-from-flavor with traits-from-elsewhere | |
| 21:34:32 | dansmith | we could do that too | |
| 21:34:41 | mriedem | so if we know we're doing bfv on a multiattach volume (or a tagged bdm), this code puts a CUSTOM_COMPUTE_SUPPORTS_MULTIATTACH on the flavor within the reqspec | |
| 21:34:47 | mriedem | efried: yeah | |
| 21:35:10 | efried | mriedem: Not on the flavor - on the allocation_candidates request | |
| 21:35:12 | mriedem | we could always do something else like the pre-placement filters add RequestSpec.system_traits (like system_metadata) or something | |
| 21:35:25 | mriedem | efried: but we have to get them to the allocation candidates request somehow | |
| 21:35:26 | dansmith | efried: on the flavor in the reqspec if we want it to be honored for a resize or migration kinda thing | |
| 21:36:25 | efried | Do we make other "modifications" to flavor data between when we pull it from the flavor db and when we attach it to the instance? | |
| 21:36:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove old flavor_create db api method https://review.openstack.org/544703 | |
| 21:36:51 | dansmith | efried: we suggested doing this for zero-root-disk BFV things I think | |
| 21:37:12 | dansmith | efried: we also probably need traits to be stored in the reqspec proper for when the user asked for them right/ | |
| 21:37:22 | mriedem | https://review.openstack.org/#/c/428505/ ? | |
| 21:37:54 | dansmith | mriedem: no, mine below that | |
| 21:37:56 | efried | My point is that it feels kinda weird for me to say 'boot an instance with this flavor, that has traits X,Y,Z' and then when I look at the flavor on the instance later, it also has traits A,B,C. I'm like where tf did those come from? | |
| 21:38:09 | mriedem | efried: you wouldn't see those | |
| 21:38:15 | efried | Okay, then I'm cool. | |
| 21:38:25 | efried | If they're on the RequestSpec and not on the flavor per se. | |
| 21:38:26 | mriedem | the instance.flavor would show the public facing flavor you used to create the instace | |
| 21:38:45 | dansmith | instance.flavor not reqspec.flavor | |
| 21:38:46 | mriedem | right, they'd be on the flavor embedded within the request spec, which doesn't get exposed out of the api today | |
| 21:38:55 | mriedem | ? | |
| 21:39:11 | mriedem | GET /servers/{id}/detail shows the flavor embedded in the instance | |
| 21:39:17 | dansmith | although we probably create instance.flavor from reqspec.flavor | |
| 21:39:28 | mriedem | we don't | |
| 21:40:05 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L910 | |
| 21:40:12 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L891 | |
| 21:40:19 | efried | So yeah. dansmith What I'm saying is that this is an alternative that should be at least documented in the Alternatives section; *and* some more explanation along these lines is needed for the reader to make sense of the statement on L47 that mentions traits for the first/last/only time. | |
| 21:40:20 | dansmith | it doesn't really matter anyway, as that'd be a conversation for how we're going to do a thing with traits, | |
| 21:40:32 | dansmith | unrelated to the general idea of augmenting the request to placement | |
| 21:40:57 | dansmith | efried: I'm not sure I get what is an alternative | |
| 21:41:10 | dansmith | efried: traits for the tenant isolation is not an option, IMHO | |
| 21:41:33 | dansmith | if you mean it's an alternative to the "and also maybe later we could do a bfv thingy", then agree, but it's not really the subject of this spec | |
| 21:41:46 | efried | So | |
| 21:41:53 | mriedem | "Although not covered by this spec directly, the mechanism added here | |
| 21:41:53 | mriedem | would enable other such request-limiting filters in the future. For | |
| 21:41:53 | mriedem | example," | |
| 21:42:00 | mriedem | that seems pretty clear that this is a framework type thing | |
| 21:42:08 | mriedem | the spec is dealing with the specific tenant aggregate problem, | |
| 21:42:10 | dansmith | this could have been two specs, but I felt it was way better to not have a tiny spec for each | |
| 21:42:14 | mriedem | but is generic enough that we can build on it for other problems | |
| 21:42:32 | mriedem | other problems like bfv | |
| 21:42:33 | dansmith | maybe I'm missing what efried is saying is an alternative though | |
| 21:43:43 | mriedem | i think he's saying, if i have A, B, C in aggregate X, rather than model it as an aggregate, the resource providers for A,B,C would have an X trait | |
| 21:43:49 | mriedem | which is a PITA | |
| 21:43:53 | mriedem | so i agree we don't want to do that | |
| 21:44:10 | dansmith | for tenant isolation for sure | |
| 21:44:11 | efried | dansmith: I agree that [traits for tenant isolation if you have to specify the traits in flavors] is not an option - but you describe it in Alternatives and explain why it's not an option (Skittles). IMO, [traits for tenant isolation where we merge the traits from the RequestSpec that we got from conf just like you're talking about doing with aggregates] *may* also not be viable, but it doesn't suffer from your main objec | |
| 21:44:11 | efried | therefore at least closer to being a viable option, and should therefore be described in the Alternatives section, and dismissed with an explanation as to why it's being dismissed if indeed there's a good reason for that. | |
| 21:44:53 | mnaser | mriedem: good call on suggesting functional test. while the codebase seems to actually not stop there, it looks like it's not getting any bdms in _delete() .. BlockDeviceMappingList(objects=[]) | |
| 21:45:03 | efried | And then you get the added bonus of being able to refer to that model from L46-7, because otherwise L46-7 doesn't make sense in the context of the rest of the spec. | |
| 21:45:06 | mnaser | is it possible that bdms are created later? :< | |
| 21:45:18 | dansmith | efried: ah okay, that's what I'm saying is also not an option IMHO, but for other reasons than flavor explosion | |
| 21:45:21 | mriedem | mnaser: i bet i know why, | |
| 21:45:27 | mriedem | mnaser: i bet the bdms aren't buried in cell0 | |
| 21:45:32 | efried | dansmith: Cool, please say that in the spec and I'm good. | |
| 21:45:50 | mnaser | yay.. | |
| 21:45:58 | mnaser | should they be buried in cell0? | |
| 21:45:58 | mriedem | mnaser: just a guess, but that's my guess | |
| 21:46:01 | dansmith | efried: because we have to label each compute individually with the trait and I think that's what aggregates are for, but yep, I'll put it in as a slightly different thing.. just for you :) | |
| 21:46:20 | efried | Thanks dansmith. Kisses. | |
| 21:46:40 | mriedem | it's a valentines day miracle | |
| 21:46:54 | efried | dansmith: Having to label each compute individually with the trait - what I'm confused about on L46-7 is why that argument doesn't apply for BFV too. | |
| 21:47:47 | dansmith | efried: well, the way I phrased that is silly, since that's traits all up and down, but let me say it in the spec and then you can argue about it, if it's clear | |
| 21:47:51 | mriedem | efried: for one thing, nova-compute can report the BFV traits capabilities automatically, the operator / admin doesn't have to do that for every tenant | |