| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2026-04-02 | |||
| 16:11:25 | sean-k-mooney | or a dnm on that repo with a depens on agains the nova change either way works | |
| 16:13:37 | sean-k-mooney | actully no | |
| 16:13:56 | sean-k-mooney | well. that does not appre to currenlty be runign the test as a sperate test | |
| 16:14:08 | sean-k-mooney | it might be part of ironic_tempest_plugin.tests.scenario.test_baremetal_basic_ops.BaremetalBasicOpsAndRescue | |
| 16:14:13 | sean-k-mooney | as a sideffect | |
| 16:14:32 | sean-k-mooney | ill check but this shoudl be a sperate test so its clearly visabel in the tempest report | |
| 16:15:22 | sean-k-mooney | ok its embded in the largere test not its own thing... | |
| 16:15:36 | sean-k-mooney | which was partly why it was so hard to debug before | |
| 16:22:25 | sean-k-mooney | gibi: on second tought this sound like a next week us problem. i left a -1 on the ironic-tempst-patch since it currenly not actully validating the console works | |
| 16:24:23 | gibi | Thanks. | |
| 16:24:45 | gibi | It is definitely not earlier than next week us problem for me ow | |
| 16:24:46 | gibi | now | |
| 16:25:38 | sean-k-mooney | im checkign ont thing while im at it | |
| 16:25:53 | sean-k-mooney | we had 2 jobs in diffent pipelien one was testign with ironic sharding | |
| 16:26:01 | sean-k-mooney | and the other was testing without it to make sure both work | |
| 16:26:56 | sean-k-mooney | ok this one does not have the shared key defiend so hopefully our other ironic job still does | |
| 16:27:19 | opendevreview | Clif Houck proposed openstack/nova master: [WIP] perf(ironic): eliminate O(N²) ProviderTree deepcopy at startup (B6) https://review.opendev.org/c/openstack/nova/+/980676 | |
| 16:27:28 | sean-k-mooney | ironic-tempest-ipa-wholedisk-bios-agent_ipmitool-tinyipa | |
| 16:28:35 | opendevreview | Clif Houck proposed openstack/nova master: perf(ironic): eliminate O(N²) ProviderTree deepcopy at startup (B6) https://review.opendev.org/c/openstack/nova/+/980676 | |
| 16:29:49 | sean-k-mooney | ah its ironic-tempest-ipa-wholedisk-direct-tinyipa-multinode-shard | |
| 16:29:53 | clif | JayF: sean-k-mooney ^ I cleaned up that commit a bit and now tests pass for me locally. The change looks reasonable to me and I'm somewhat convinced that it provides a speedup | |
| 16:29:55 | sean-k-mooney | and that in the weekly perodic | |
| 16:31:01 | sean-k-mooney | ack so you pulled that out onto master directly cool | |
| 16:31:09 | clif | yep | |
| 16:31:38 | clif | unit tests needed a small change to pass due to new arguments to expected calls | |
| 16:32:04 | clif | my caveat is I'm still very new to the nova codebase in general | |
| 16:32:07 | sean-k-mooney | ah makes sense | |
| 16:32:58 | sean-k-mooney | this likely need more test coverage but did you have a chace to do any manual testing of this with a real or simulated ironic? | |
| 16:33:24 | sean-k-mooney | jay mention there is a db simulator thing in ironic but i didnt have a change to dig into it | |
| 16:33:40 | clif | I haven't done manual testing with it no, I can take a closer look at test coverage | |
| 16:35:39 | sean-k-mooney | we liekly dont need/want the simulator in this change but we might want to add some simpel unit test that create a repesteive provide tyree cache with 1, 10, 100 nodes and do some very simple scale tests | |
| 16:36:26 | JayF | going from "working ironic devstack" -> "working ironic devstack with lots of fake driver nodes" is trivial by hand | |
| 16:36:33 | JayF | I am actually working today on hooking it up via devstack config | |
| 16:36:48 | sean-k-mooney | oh cool | |
| 16:37:35 | sean-k-mooney | so that should be simpler to tst we litclly jsut need to stop and start nova and see how long it take to compelte if so | |
| 16:37:49 | sean-k-mooney | nova -> the nova-compute | |
| 16:37:55 | JayF | yeah | |
| 16:38:06 | JayF | honestly the culture here has always seemed to only count proof if it runs in CI | |
| 16:38:12 | JayF | so I wans't even going to attempt a manual test | |
| 16:38:26 | sean-k-mooney | well yes if its not in ci we assume its broken | |
| 16:38:45 | sean-k-mooney | but i jsut wanted to be sure we saw a speedup before we spent too much time on this | |
| 16:38:54 | sean-k-mooney | i.e. to make sure it really not a test artifact | |
| 16:39:20 | sean-k-mooney | so a one of manual test to confirm build confindence in the aproch | |
| 16:42:01 | clif | I don't mind attempting manual testing, I will say the change logically makes sense to me. Doing less work should *almost* always result in a speed increase | |
| 16:42:48 | sean-k-mooney | clif: yep what im hoping the manaul test will show is how big or small this is to the overall tiem | |
| 16:42:52 | sean-k-mooney | in a real system | |
| 16:43:10 | sean-k-mooney | the next speed up to try woudl be hte mutli threading patch | |
| 16:43:17 | sean-k-mooney | those are where i think most of it will come form | |
| 16:43:26 | sean-k-mooney | out of the inifally few | |
| 16:43:33 | clif | yep, thats the one I want to look at next | |
| 16:43:49 | JayF | my downstream wanted to pull down that multithreading patch yesterday and I told em to wait for clif :D | |
| 16:44:24 | JayF | event-based updates for ironic node provision state is apparently a real need for them; it can take 20-30 minutes post-cleaning for a node to return to inventory | |
| 16:44:59 | clif | yea that's... not ideal to say the least | |
| 16:46:09 | sean-k-mooney | clif: for what its worth we often do add context like this to commti message if we have the data | |
| 16:46:11 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/427145 | |
| 16:46:49 | sean-k-mooney | its not really ment to be a rigorus test jsut indicitive | |
| 16:47:57 | sean-k-mooney | JayF: i need ot understand the delay there more but not today | |
| 16:48:08 | JayF | that periodic has to complete | |
| 16:48:16 | JayF | one of the big ones that are kinda awful on ironic | |
| 16:48:21 | sean-k-mooney | well it shoudl not really depend on that | |
| 16:48:28 | JayF | that's what I wanna fix :D | |
| 16:48:33 | sean-k-mooney | so i want to understand why | |
| 16:48:37 | JayF | same, I haven't gotten there yet | |
| 16:48:43 | JayF | and mostly am asking clif to get there :D | |
| 16:48:55 | sean-k-mooney | becuase when a node is cleanign it placement recouse provider shoudl jsut have reserved=total for the baremetal resocue class | |
| 16:49:23 | sean-k-mooney | but ironic can just set reserved back to 0 directly in placment when its done cleaning and its in the aviable state | |
| 16:49:56 | JayF | yeah, we don't do ANYTHING with placement now | |
| 16:50:00 | sean-k-mooney | so i just want to make sure we are not doing somethign dumb like recreating the RP and deleteing it based on the state | |
| 16:50:05 | sean-k-mooney | oh ok | |
| 16:50:15 | sean-k-mooney | so it relying on nova to poll ironic exclusivly | |
| 16:50:17 | JayF | yeah this is the stuff I was talking about for fixing for Ironic | |
| 16:50:18 | JayF | yep | |
| 16:50:25 | sean-k-mooney | and notice that the state has moved to avaible and we update it | |
| 16:50:30 | JayF | there are lots of opportunities for ironic to push via nova events api and/or talk to placement directly | |
| 16:50:39 | sean-k-mooney | ack ya | |
| 16:50:52 | sean-k-mooney | we can talk about that more whewn we both have more time | |
| 16:51:07 | sean-k-mooney | there defintly ways to short circut the perodic | |
| 16:54:17 | melwitt | dansmith, sean-k-mooney: I dunno if one of yall could help but there are a couple of patches needing a second +2 to start vtpm live migration testing in CI, devstack https://review.opendev.org/c/openstack/devstack/+/958254 and nova https://review.opendev.org/c/openstack/nova/+/978841 | |
| 16:55:41 | melwitt | on the devstack one mdevctl is included and IIRC it was something maybe sean had suggested on IRC to include but I don't think it's needed for vtpm. I can remove it if desired | |
| 16:56:07 | sean-k-mooney | sure i can look at them now | |
| 16:56:28 | sean-k-mooney | im pretty sure i have looked at or asked you about hte devstack one before | |
| 16:57:25 | melwitt | yeah same. thanks | |
| 17:02:33 | sean-k-mooney | do we also need https://review.opendev.org/c/openstack/devstack/+/968114 ? | |
| 17:03:26 | melwitt | oh hm, checking, I forgot about this one | |
| 17:05:08 | sean-k-mooney | https://review.opendev.org/c/openstack/devstack/+/968114/comment/fdbb7370_567faacf/ | |
| 17:05:18 | sean-k-mooney | so that will work but only on single node | |
| 17:05:25 | melwitt | I can't remember why I did that, must have been before I realized it wasn't needed | |
| 17:05:44 | melwitt | I don't find any patches that depends-on this | |
| 17:05:47 | sean-k-mooney | if we want to be able to enable this on a compute (without barbican) we woudl have to have a flag | |
| 17:05:49 | sean-k-mooney | ack | |
| 17:06:06 | sean-k-mooney | so the nova job you worte | |
| 17:06:17 | sean-k-mooney | is using barbican on the contoler | |
| 17:06:26 | sean-k-mooney | but disbaeld on the compute as we would expect | |
| 17:06:49 | sean-k-mooney | i wonder are we using the castalan optiosn or something for the key manager | |
| 17:07:50 | sean-k-mooney | melwitt: meta question is there a reason not to put this into nova-next or nova-alt-config? | |
| 17:07:52 | melwitt | it is using castellan but I don't remember where it's defaulted to that | |
| 17:08:30 | sean-k-mooney | we can move this testing later by the way im jsut confiming | |
| 17:08:54 | melwitt | sean-k-mooney: it requires nested virt in order to work, so I think I was likely thinking to make as small and short a job as possible to not have as much contention | |
| 17:09:33 | sean-k-mooney | ok i was thinking of movign the nova-next job ot use nested virt a while back | |
| 17:09:36 | melwitt | i.e. a longer job that requires nested virt will tie up the limited CI nodes that support nested virt | |