Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-03-26
17:41:11 dansmith efried: we've been saying placement goes first in the upgrade stack
17:41:18 efried edleafe: Not as of the last time I checked, which I think was yesterday.
17:41:35 dansmith efried: where? that code has changed several times while I was working on this and we don't have any fallback code there
17:42:05 efried dansmith: I think we say that as a best practice or something, but we've got somewhere else that defines the minimum placement microversion for a given release and it's always lower than the maximum for same release. mriedem help me out here.
17:42:11 jaypipes dansmith: example? the code has changed in a backwards *incompatible* way and we don't have fallback code?
17:42:16 efried dansmith: Just look for 406 in report.py
17:42:37 efried cdent culled the stale ones at the end of Queens, so there aren't many left, but still some.
17:42:51 dansmith jaypipes: the last couple of times this exact method has changed
17:43:07 sean-k-mooney efried: the grenagde job in the gate is proably upgradeing placement first so even if it could work its proably not tested today
17:43:13 dansmith I can go actually dig up reviews if you care
17:43:30 edleafe efried: yeah, I didn't see anything either. So would you call this a bug fix or a new feature?
17:43:34 dansmith efried: why are we using different initial versions for any of those calls then?
17:43:53 efried edleafe: needs a bp fo sho
17:44:38 edleafe efried: ok. Is there any discussion I can refer to? I'm not clear on the reasons for doing this
17:44:55 efried dansmith: Because the newer way is usually more efficient or similar; and it gives us an easier delta when we do bump the min
17:44:57 cdent edleafe: needs a spec, is a new feature and an api change
17:45:09 dansmith efried: jaypipes this one is the one I have in my head right now: https://review.openstack.org/#/c/536085/9/nova/scheduler/client/report.py
17:45:26 cdent edleafe: if you want to chat about a bit later this evening I can do a bit of a brain dump on you if you want
17:45:36 cdent but in the midst of something at the moment
17:45:45 edleafe cdent: you mean you haven't dumped it in an email yet? :)
17:46:14 dansmith efried: the problem is that if placement doesn't go first, scheduler depends on it for az calculation, then we have to fail any request that cares about az if placement doesn't support 1.21 _anyway_
17:46:23 cdent no, because I was trying not to own this one. I did have a to do list item which said "write email about consumer uuid" but deleted it when you "got ownershipw of this" :)
17:47:10 edleafe cdent: ah, I was just volunteering my cycles to code it. If I gotta write a spec, I should at least understand the motivation behind it.
17:47:22 edleafe (that usually helps) :)
17:47:36 efried edleafe: I got you, gimme sec
17:47:48 cdent edleafe: details! but yeah, I can dump some brain shortly if efried doesn't beat me to it
17:47:48 jaypipes dansmith: you will note I did not review that.
17:49:00 dansmith jaypipes: so cdent was culling all the microversions when, at the end of each release?
17:49:07 efried dansmith, jaypipes: Agree that one should have had the 406-fallback conditions. I shoulda reviewed it too, but didn't. IIRC that got jammed in really late in Q because we weren't going to make granular happen in time.
17:49:16 dansmith jaypipes: so you want a bunch of microversion handling code for within-release changes?
17:49:17 cdent jaypipes, dansmith: we've been inconsistent about our expectation with nova-side microversion fallback. I've not been able to discern a pattern, mostly because _some_ of the time we say "we expect placement to upgrade first" at which point why bother falling back?
17:49:34 cdent I didn't cull them all, just anything less than 1.14 (I think it was 1.14, would need to check to be sure)
17:49:48 dansmith cdent: yeah I see some places where we do fallback, and some where we don't
17:49:49 dansmith cdent: it was 1.14
17:50:06 jaypipes dansmith: I'm just noting I did not review that patch (didn't know about it at all). I believe I was in Dallas that week.
17:50:46 jaypipes dansmith: I do support the reportclient supporting fallback code (406 handling) for at least a release, yes.
17:50:47 dansmith so what do you people propose? scheduler fails if you request an az and placement is old? fall back but make sure the old filter is enabled otherwise fail?
17:51:08 jaypipes dansmith: yes, that sounds reasonable to me.
17:51:11 dansmith jaypipes: we're not testing nova against an older placement, so it basically doesn't work
17:51:17 dansmith jaypipes: seriously? that was a joke :)
17:51:46 dansmith well, the second part was a joke
17:51:52 dansmith maybe you mean the first part?
17:51:56 dansmith but that doesn't seem very friendly to me
17:51:58 jaypipes dansmith: well, the "make sure the old filter is enabled" was a joke, right?
17:52:05 dansmith yes
17:52:07 sean-k-mooney jaypipes: as other service start using placement it will likely make sense to have placement upgrade early in the sequence. is there a reason you do not want to commit to placement before nova in general?
17:52:12 jaypipes dansmith: the "fail if placement doesn't support the az filter" wasn't.
17:52:58 dansmith jaypipes: I think if we're adding code for all those branches we need a grenade test where we leave placement behind on queens and run master against it
17:53:00 efried dansmith: I think what I'm saying is that nova can support the AZ filter even without support for GET /a_c?member_of=
17:53:00 dansmith like we did for nova-compute
17:53:03 dansmith otherwise it's broken
17:53:18 efried dansmith: And if we need to support N-1 release of placement in nova, we've gotta write that code "the hard way".
17:53:20 dansmith efried: it can if you leave the post-processing filter enabled
17:53:29 mriedem efried: nova-status has a min placement API version check
17:53:29 jaypipes sean-k-mooney: if the change in the placement API is backwards compatible (which a *new* query param is), I don't see a reason to fixate an upgrade to always do placement first.
17:53:32 dansmith efried: yeah, I'm asserting we do not
17:53:59 efried mriedem: So the discussion here is: why isn't that minimum version always the same as the max-in-release?
17:54:23 mriedem because the nova side code might not be using the max yet
17:54:38 dansmith mriedem: in most cases it's not
17:54:41 efried mriedem: Under what circumstances? Haven't we always said you have to upgrade placement first?
17:54:42 mriedem generally it likely is because we added the new placement api versions for things nova needs
17:54:48 sean-k-mooney jaypipes: that is true but it appears we are not quite to that point yet. as dan said we likely need a gate job that leaves placement on N-1 to validate it works if that is the guareentee we want to give
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? :)

Earlier   Later