| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-26 | |||
| 18:03:05 | cdent | I mean can we decide today that's how it is going to be? | |
| 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:21 | dansmith | or acknowledge that we already did? :) | |
| 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 | dansmith | efried: BS you were too :) | |
| 18:04:28 | efried | ableit sucky ones. | |
| 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 | jaypipes | right, exactly. | |
| 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: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 | sean-k-mooney | jaypipes: well we would only be frocing them to upgrade placement first if they need a placement feature in nova | |
| 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 | |
| 18:19:37 | jaypipes | dansmith: ok, then I'm cool with all of this, then. :) | |
| 18:19:46 | jaypipes | carry on. | |
| 18:19:47 | dansmith | aight | |
| 18:19:56 | jaypipes | sorry for being dense. | |
| 18:20:08 | sean-k-mooney | dansmith: well from an install tool poing of view ya most installers would perfer to always have a clear order of operations even if it wasnt strictly required. | |
| 18:20:31 | efried | jaypipes: Ugh, why is maxlength for a RP name 200? (As opposed to 255, like for trait and RC?) | |
| 18:20:43 | dansmith | sean-k-mooney: in the early days of making nova upgrade smoothly, that was the #1 request.. "just tell me which order and I'll do it." | |
| 18:21:03 | jaypipes | efried: UGH. Why would you need an rp name longer than 200 characters. UGH! | |
| 18:21:31 | efried | jaypipes: Sorry, the ugh was because inconsistent with trait/RC. | |
| 18:21:39 | sean-k-mooney | dansmith: i think that is still the case although "keep it running without any downtime magically while i upgrade" may have over taken it | |
| 18:21:48 | jaypipes | efried: no idea | |
| 18:22:19 | efried | cdent: any idea? | |
| 18:23:27 | sean-k-mooney | efried: its saves you half a KB per RP? do you have a reason to have a name over 200 | |
| 18:23:51 | cdent | well, rp name came first, so I reckon it was arbitrary decision at the time and then when custom traits and resource classes came along people expressed concern about wanting to make them super long and 255 some some random compromise. AKA: I don't think there were _reasons_ as such | |
| 18:24:06 | efried | sean-k-mooney: No. I'm writing code to sanitize RP/RC/trait names and the inconsistency is gonna make me write >1 method instead of just 1. | |
| 18:24:47 | sean-k-mooney | efried: or jsut assume make 200 for all names | |
| 18:25:29 | sean-k-mooney | there is no harm in makeing it a 255 lenght however you would need a sql schema update which likely is not worth it for this allow | |
| 18:25:43 | cdent | ? | |
| 18:25:58 | melwitt | tssurya: hey, thanks for bringing up the user_id column in instance_mappings table thing. I agree it should be a separate spec to handle the "quota when cell-down" issue. I have an old spec for it that I can resurrect. my spec will depend on yours because I also need the queued_for_delete column | |
| 18:27:05 | tssurya | melwitt: okay, :) I guess we can co-ordinate on this then, I will let you know as soon as I put up a spec for it then. | |
| 18:27:27 | melwitt | tssurya: cool, thank you :) | |
| 18:27:38 | efried | cdent: Not quite. I was reviewing 520313 which has code that will wind up sending down an RP name that will fail - https://review.openstack.org/#/c/520313/23/nova/tests/unit/virt/xenapi/test_driver.py@443. Letting the API fail in this case will be worse than suboptimal, because by the time that happens, we're outside of where virt can do anything about it. | |
| 18:27:58 | efried | cdent: This isn't the first time I've seen a need for a method to "slugify" a placement identifier, so I thought I would write it. | |
| 18:28:01 | tssurya | melwitt: the only concern I have is that at the PTG, we discussed that we would not use placement and do a solution of allowing the users to create VMs if they don't have any in the down cell | |
| 18:28:07 | cdent | efried: I was teasing | |
| 18:28:12 | cdent | gentle ribbing and all that | |
| 18:28:38 | tssurya | melwitt: however if we are going to use placement, then this changes things i guess | |
| 18:28:38 | melwitt | tssurya: you mean disallowing? | |
| 18:28:47 | efried | cdent: It's a valid concern that you should check me on constantly, even though (or perhaps especially because) you and I fundamentally disagree on whether such pre-optimizations are desirable. | |
| 18:29:02 | efried | cdent: But in this case, it's more than that. | |
| 18:29:08 | tssurya | melwitt: no, I mean allowing VM creation as long as there are no living VMs in the down cell | |
| 18:29:23 | tssurya | melwitt: since in that case the quota calculation will be correct | |
| 18:29:46 | melwitt | tssurya: yeah, that was when we had thought being able to count instances while cells are down would require adding a "type" to placement allocations. that is something that will take a lot of work to figure out | |
| 18:29:50 | sean-k-mooney | efried: maybe just take the lenght as an optional param that you defalt to 255 and the caller can set 200 or what ever if they no its less for that field | |
| 18:29:50 | cdent | Fair enough. I never really said they are not desierable, I suggested that they should wait for the road to show they are needed. | |