| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-26 | |||
| 17:55:09 | dansmith | mriedem: that's why I'm saying we might as well use the same default microversion in the whole file if we're going to require this, which i think defeats a lot of the point of microversioning this at all | |
| 17:55:27 | mriedem | see first bullet in #2 here https://docs.openstack.org/nova/latest/user/upgrade.html#rolling-upgrade-process | |
| 17:55:46 | mriedem | i haven't been paying attention to this conversation so don't have context | |
| 17:56:49 | dansmith | so by the same argument, | |
| 17:56:54 | dansmith | if a flavor has any required trait in it, | |
| 17:56:57 | sean-k-mooney | mriedem: that likely should be expanded to "before and openstack service that uses placement" rather then nova but ya. | |
| 17:57:03 | dansmith | we must fail if placement doesn't support 1.17 for that rev I linked above | |
| 17:57:13 | mriedem | sean-k-mooney: these are nova upgrade docs... | |
| 17:57:17 | dansmith | sean-k-mooney: those are nova upgrade notes | |
| 17:57:18 | dansmith | heh | |
| 17:57:21 | efried | dansmith: Or write a post-filter. Yeah. | |
| 17:57:31 | sean-k-mooney | hehe ok fair point :) | |
| 17:57:45 | dansmith | efried: we can't do that, AFAIK | |
| 17:57:53 | mriedem | dansmith: i thought we did fail if a flavor has required traits in it but 1.17 isn't available? | |
| 17:57:55 | dansmith | efried: filters don't have access to the actual a-c I don't think | |
| 17:58:12 | dansmith | mriedem: we do, but they are arguing that it should fall back to 1.14 | |
| 17:58:21 | efried | dansmith: I'm not talking about a Filter. Talking about code in the report client that manually does the trait filtering. | |
| 17:58:23 | dansmith | and I'm saying I don't know what we do in that case, because 1.14 isn't enough | |
| 17:58:25 | mriedem | i disagree, | |
| 17:58:37 | mriedem | if the flavor has required traits, the admin is saying, 'i require traits for this flavor so make it happen or fail' | |
| 17:58:56 | dansmith | mriedem: agreed, and same for my aggregate requirement | |
| 17:58:59 | dansmith | there's no point in falling back | |
| 17:59:26 | efried | dansmith: Except that putting a trait in a flavor is a new thing the admin would do. | |
| 17:59:30 | mriedem | https://docs.openstack.org/nova/latest/user/flavors.html has something about required traits but doesn't say we fail explicitly if 1.17 isn't available | |
| 17:59:36 | dansmith | efried: so? | |
| 17:59:46 | efried | dansmith: But AZ filtering is something we're trying to make compatible with existing setups... no? | |
| 18:00:18 | dansmith | efried: but we can't test for membership from the client side | |
| 18:00:32 | dansmith | efried: at least not providing the same semantics the user is expecting | |
| 18:00:45 | dansmith | we can approximate it by looking in nova's notion of the aggregate, but it's not the same | |
| 18:01:15 | efried | dansmith: Sure, why not? That method has received the list of aggregate UUIDs. We can glean RP UUIDs by looking at placement. We just do the filtering on the client side rather than at the placement server. | |
| 18:01:25 | efried | I mean, it's likely to be horribly inefficient. | |
| 18:01:27 | dansmith | mriedem: that text implies NoValidHost if we can't find RPs with the traits requested, which is even less obvious about why it's failing | |
| 18:01:51 | efried | mriedem: TBC, I think we agree it was a miss that this issue wasn't considered one way or another in that traits review. We're kind of post-morteming what we *should* have done there. | |
| 18:02:12 | dansmith | well, fwiw, I had been assuming what our docs say, which is placement goes first | |
| 18:02:17 | efried | s/glean RP UUIDs/glean aggregate UUIDs for those RPs/ | |
| 18:02:38 | cdent | Is there a reason we can't assume placement goes first? | |
| 18:03:05 | cdent | I mean can we decide today that's how it is going to be? | |
| 18:03:21 | dansmith | or acknowledge that we already did? :) | |
| 18:03:21 | mriedem | efried: eh? if placement 1.17 isn't available for required traits in a flavor, i think we should fail | |
| 18:03:52 | cdent | dansmith: well, sure, yeah | |
| 18:04:18 | efried | mriedem: That seems like a reasonable decision to me. I wasn't necessarily advocating for anything else, just pointing out that there are other options. | |
| 18:04:28 | efried | ableit sucky ones. | |
| 18:04:28 | dansmith | efried: BS you were too :) | |
| 18:04:48 | efried | dansmith: Well I certainly was advocating for that in the case of your patch. | |
| 18:04:49 | dansmith | <efried>Whoah. Yes, we do, every time. | |
| 18:04:52 | dansmith | ^ | |
| 18:04:56 | dansmith | heh, okay | |
| 18:05:12 | efried | dansmith: Yeah, that was before I realized we had screwed the pug on the traits one. | |
| 18:05:33 | mriedem | for the traits one, the pug needed to get laid imo | |
| 18:05:44 | mriedem | not every case is a fallback situation | |
| 18:05:53 | dansmith | right | |
| 18:05:59 | efried | I (now) agree with that. | |
| 18:06:53 | efried | But dansmith I'm still needing to be told that it won't represent a regression if we go the failure path in the agg filtering case. | |
| 18:07:22 | efried | Like, does an existing flavor in an existing cloud quit working because we made this path fail (if they don't have recent placement)? | |
| 18:07:32 | dansmith | az is per user request | |
| 18:07:41 | dansmith | nothing to do with flavor | |
| 18:07:43 | dansmith | in this case | |
| 18:07:57 | efried | same thing as far as I'm concerned. Clouds have scripts. Or orchestrators, or whatever. That expect it to work a certain way. | |
| 18:08:31 | efried | If we put a real stake in the ground about upgrading placement first, it's moot. We can rip out the existing 406 code, never have to implement it again going forward. | |
| 18:08:32 | dansmith | okay I'm not sure what you're looking for here | |
| 18:08:53 | dansmith | efried: as mriedem said, I'm not opposed to all failback code | |
| 18:09:05 | dansmith | by any means, but this one has no legit alternate path imho | |
| 18:09:06 | efried | I would love to see us say placement must always be upgraded first. I just wasn't aware that was even a possibility. | |
| 18:09:18 | dansmith | for example, | |
| 18:09:37 | dansmith | the compute code paths may be able to tolerate older or newer placement because they're mostly just massaging data into the right format, | |
| 18:09:48 | dansmith | and we changed POST (or GET) once already and meh they can handle it | |
| 18:09:58 | dansmith | but this is more than just format and it's something the scheduler is hard depending on | |
| 18:10:27 | dansmith | so, I can use 1.17 if no aggregates are required, and only request 1.21 if they are, if you think that's better, but I don't think falling back to 1.17 is a thing for this | |
| 18:10:38 | dansmith | so we'll tolerate older placement if we don't need it, | |
| 18:10:50 | dansmith | although that just means the operator says "half these requests work, wtf?" | |
| 18:11:06 | dansmith | and, I think if we're going to do any of that, we need a real grenade test where the service is left unupgraded | |
| 18:11:10 | dansmith | otherwise we have nfi if it works or not | |
| 18:11:23 | jaypipes | -1 just because efried just said "screwed the pug" | |
| 18:11:36 | efried | I was wondering when you would catch up with that. | |
| 18:12:45 | efried | dansmith: My concern is this: will there be some script "out there" that says "do this AZ thing" that works on Queens, and then when they upgrade to Rocky, that same script will no longer work IF they're downlevel placement. | |
| 18:13:10 | dansmith | efried: yeah and I don't see that as a problem | |
| 18:13:13 | efried | And if the answer is yes, are we accepting that and our response would be "go upgrade placement" | |
| 18:13:16 | dansmith | efried: if the rule is placement goes first | |
| 18:13:38 | dansmith | efried: any idea how much will break if you upgrade nova-compute and nothing else? | |
| 18:13:57 | efried | dansmith: Then w00t and let's update the docs (mandatory) and let's rip out all the fallback code (optional) and let's forget anyone ever mentioned a grenade test. | |
| 18:14:19 | efried | dansmith: And in this case, use 1.21 and be done. | |
| 18:14:38 | efried | dansmith: Although actually it'll be 1.2x whenever the list-of-tuples business is approved and written. | |
| 18:14:49 | sean-k-mooney | efried: well if you boot a vm via horizon i think it forces you to specify an availablity zone so if nova has a hard dependcy on 1.21 for AZs to work that would break horizon booted vms if you did not upgrade placement | |
| 18:14:54 | dansmith | efried: see, I don't think that's the way to go.. using 1.21 means every time I need to bump the level for g-a-c, I have to go fix all the other code in that file which is perfectly fine with 1.1 | |
| 18:15:07 | dansmith | efried: that's the whole point of microversions, as I see it.. that you don't need to do that | |
| 18:15:19 | efried | dansmith: I'm talking about using 1.2x for just this method, not for the whole file. | |
| 18:15:20 | dansmith | efried: you opt into new features (and maintenance) on a per-call basis as you need it | |
| 18:15:26 | dansmith | efried: ack, okay | |
| 18:15:49 | efried | dansmith: But I *am* saying the fallback code elsewhere in the file (two places, I think) becomes dead and can be removed. | |
| 18:15:49 | jaypipes | right, exactly. | |
| 18:16:03 | dansmith | jaypipes: who are you agreeing with? | |
| 18:16:08 | jaypipes | dansmith: you. | |
| 18:16:16 | jaypipes | "you opt in ..." | |
| 18:16:18 | dansmith | jaypipes: okay I'm totes confused about where you sit on this then ;) | |
| 18:16:23 | efried | I was agreeing with that too. So everybody agrees. | |
| 18:16:58 | jaypipes | dansmith: I guess my butt hurts from the fence. | |
| 18:18:14 | jaypipes | dansmith: on the one hand I don't want to *always* force users to upgrade placement first when it's not necessary to. on the other hand, I see the futility of operators upgrading nova-scheduler first, placement second and having a time when some requests involving AZs suddenly start failing. | |
| 18:18:56 | jaypipes | dansmith: I was referring to sitting on the fence, there... | |
| 18:19:02 | jaypipes | in case that wasn't obvious ;) | |
| 18:19:08 | dansmith | jaypipes: yeah, and I also don't think operators are at all complaining about having a clear order of services for upgrades | |