| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-13 | |||
| 16:19:30 | gmann | dansmith: but that will be run after negative evacuate test pass https://github.com/openstack/nova/blob/master/roles/run-evacuate-hook/tasks/main.yaml#L87 | |
| 16:20:21 | dansmith | ah okay, I thought you meant the setup_evacuate_resources.sh which seems to be fine | |
| 16:20:28 | dansmith | but yeah got it | |
| 16:21:04 | gmann | k | |
| 16:21:36 | bauzas | we are one day before RC1 and still a lot of changes in flight mode :/ | |
| 16:21:51 | bauzas | even if the gate is not flipping that much, that's still limbo dance | |
| 16:22:29 | opendevreview | Dan Smith proposed openstack/nova master: Make our nova-ovs-hybrid-plug job omit cinder https://review.opendev.org/c/openstack/nova/+/893540 | |
| 16:22:48 | dansmith | bauzas: things needing review or just waiting to merge? your etherpad seemed like everything was pretty well on its way when I looked | |
| 16:22:59 | sean-k-mooney | dansmith: oh i forgot about that am not today but i can look at it tomorrow i guess | |
| 16:23:06 | dansmith | sean-k-mooney: already on it | |
| 16:23:18 | sean-k-mooney | oh ok thanks | |
| 16:27:08 | bauzas | dansmith: everything is accepted except the prelude (which is still on my laptop atm) so just a gate update | |
| 16:27:17 | dansmith | bacack | |
| 16:27:20 | dansmith | or ack even | |
| 16:27:35 | bauzas | bah ack | |
| 16:27:37 | bauzas | :p | |
| 16:34:31 | opendevreview | Sylvain Bauza proposed openstack/nova master: Add a Bobcat prelude section https://review.opendev.org/c/openstack/nova/+/894940 | |
| 16:36:20 | bauzas | JayF: I'm lost in translation, does https://review.opendev.org/c/openstack/nova/+/894833/2 mean that ironic shards won't work until we deliver an openstacksdk release ? | |
| 16:36:28 | bauzas | if so, /me is a very sad panda | |
| 16:36:38 | gmann | I will also check etherpad if anything new change needed to merge, | |
| 16:39:22 | JayF | bauzas: johnthetubaguy and I are on a call right now trying to find alternatives | |
| 16:39:27 | JayF | bauzas: I assure you I am the saddest panda | |
| 16:40:19 | bauzas | gmann: prelude just arrived, but maybe we should remove the first bullet point items | |
| 16:42:08 | bauzas | JayF: we still have time to remove the shards from the highlights and remove the conf options, if you want MHO | |
| 16:42:23 | bauzas | or just revert the 3rd patch tbc | |
| 16:42:34 | JayF | that is the worst of all options, I think we have a chance of getting a cleaner fix | |
| 16:42:35 | bauzas | I can hold RC1 just for that | |
| 16:42:49 | bauzas | JayF: a clean fix can't be a client change | |
| 16:43:02 | JayF | we are trying to manipulate it to do what we want w/o the client change | |
| 16:43:06 | bauzas | it's a long-term solution, but this can't happen in the timeframe we hav | |
| 16:43:08 | JayF | that patch is up as the known-working example | |
| 16:43:57 | bauzas | well, the latest patchset is just a noop but yeah | |
| 16:44:24 | gmann | bauzas: noted, will wait for that | |
| 16:44:36 | bauzas | the fact is, I'm surprised we're facing this at the very end, but urgency requires me to do the post-mortem after RC1, not now | |
| 16:45:06 | bauzas | but I'd enjoy any very quick fix | |
| 16:45:08 | dansmith | bauzas: I think it makes sense to communicate the "not super tested" nature of it if we're going to leave it in the prelude, personally | |
| 16:45:11 | bauzas | trust me about it | |
| 16:45:25 | dansmith | given what we normally expect for validation of headline features like that | |
| 16:45:41 | bauzas | dansmith: yeah, and trust me, I can even remove it from the prelude | |
| 16:45:59 | bauzas | actually the highlights worry me much | |
| 16:46:12 | bauzas | since they will be used by the marketing folks for the marketing fest | |
| 16:46:32 | bauzas | fortunately, this isn't merged yet, I'm gonna put a -W until we clarify the state | |
| 16:54:10 | bauzas | JayF: I'll drop now for 2.5 hours but I'll come back | |
| 16:54:24 | bauzas | in case you have options | |
| 17:12:41 | sean-k-mooney | JayF: bauzas i think we use ironic client of rthe shard stuff no? | |
| 17:12:50 | JayF | bauzas: I am convinced a revert is the best path. I'd personally prefer even reverting the peer_list deprecation; but if we leave that deprecation in, can I get some assurance that we won't *remove* peer_list until sharding lands? | |
| 17:12:54 | sean-k-mooney | JayF: stephenfin had a seriese ot move ironic to the sdk | |
| 17:12:55 | JayF | sean-k-mooney: we use a mix of client and sdk | |
| 17:13:09 | sean-k-mooney | ok but we didnt land the changes to move to sdk only | |
| 17:13:12 | JayF | sean-k-mooney: which is part of why this was complex to figure out what was going on | |
| 17:13:20 | JayF | https://etherpad.opendev.org/p/nova-sharding-rca I've created this | |
| 17:13:24 | sean-k-mooney | ok | |
| 17:13:27 | JayF | putting notes from our technical research for follow-ups | |
| 17:13:34 | sean-k-mooney | so are we using ironic client for this or not? | |
| 17:13:46 | JayF | all node fetches in Ironic for node listing use sdk | |
| 17:13:50 | JayF | most node updates/writes use client | |
| 17:13:52 | bauzas | JayF: so you basically wanna revert the whole series ? | |
| 17:13:53 | JayF | but that is a very rough line | |
| 17:14:11 | sean-k-mooney | lets not do that just yet | |
| 17:14:13 | JayF | bauzas: I think that's what we have to do. We'd need an SDK change, and there's a sniff of a bug in the Ironic API handling too, which is what pushed me to revert | |
| 17:14:23 | sean-k-mooney | if the specific call we need are functional then i woudl not revert | |
| 17:14:36 | bauzas | sean-k-mooney: we are on the edge of RC1 | |
| 17:14:47 | JayF | the entire basis of ironic/nova node sharding is that we have the ability to query nodes limited by shard | |
| 17:14:51 | bauzas | and we highlighted to the marketing about the ironic features | |
| 17:14:54 | JayF | on ironic side we index on shard to make that a fast query | |
| 17:15:10 | JayF | and so if SDK can't add that shard query, doing late filtering or anything like that basically guts the purpose of the change | |
| 17:16:05 | dansmith | if there's any question, we should revert for sure | |
| 17:16:07 | sean-k-mooney | ok its using the sdk | |
| 17:16:09 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L616 | |
| 17:16:23 | JayF | Yeah I've been on a zoom w/ johnthetubaguy troubleshooting this since like 7am local time, 3 hours | |
| 17:16:25 | JayF | and julia in here too | |
| 17:16:29 | dansmith | we should leave the deprecation in place but we probably need to adjust the text that says to use the other thing | |
| 17:16:46 | JayF | dansmith: please please lets not remove peer_list until shard lands though :/ | |
| 17:16:49 | bauzas | dansmith: jay is even proposing to revert the whole stack | |
| 17:17:05 | JayF | bauzas: my care about peer_list is this: we need a single slurp release with peer_list + sharding for migration | |
| 17:17:06 | sean-k-mooney | we are not removing peer_list until D | |
| 17:17:08 | dansmith | bauzas: right, which is probably best | |
| 17:17:13 | JayF | I have no concerns with deprecating peer_list now, but if we remove it in D | |
| 17:17:19 | JayF | then SLURP users don't get a migratory release | |
| 17:17:40 | sean-k-mooney | yes they do the migratiory release is C | |
| 17:17:58 | bauzas | okay, then I'm preparing the reverts | |
| 17:18:08 | sean-k-mooney | it will be deprected in B and C and then removed in D C will have shard supprot and peer_list support | |
| 17:18:25 | JayF | bauzas: I'm extremely sorry for this, we should've had devstack testing lined up as a prereq | |
| 17:19:14 | bauzas | JayF: this is not about being sorry, I prefer we found the bug before we deliver | |
| 17:19:22 | bauzas | so thanks for having spotted it | |
| 17:19:24 | sean-k-mooney | i would prefer to just using update the docs hostely rather then revert | |
| 17:19:34 | JayF | bauzas: that's why I made darn sure I was hooking up devstack before release time | |
| 17:19:57 | dansmith | nova should have expected a devstack job showing it working before merging, so the fault lies with us all | |
| 17:20:16 | sean-k-mooney | dansmith: also true ... | |
| 17:20:27 | JayF | dansmith: I literally just said that on the zoom call I'm still on | |
| 17:20:57 | dansmith | yup, I didn't review the later patches so I didn't even know we were missing that | |
| 17:21:18 | dansmith | but this is a good demonstration of why we should (and usually do) require such things | |
| 17:21:29 | bauzas | agreed | |
| 17:21:32 | dansmith | at least we found it before release | |
| 17:22:16 | sean-k-mooney | JayF: i assume laning the requried change in sdk was aslo rejected right | |
| 17:22:23 | JayF | it's already landed in master | |
| 17:22:31 | JayF | but we saw concerning behavior from the /v1/nodes?shard=blah Ironic API endpoing | |
| 17:22:37 | JayF | which is why I hit the revert button | |
| 17:22:47 | sean-k-mooney | ok but it has not release yet right | |
| 17:22:53 | JayF | I want it to land; but I don't want to break people and my confidence in this feature functioning is extremely low | |