| opendevreview | Steve Baker proposed openstack/ironic master: Memoize calls to bcrypt.checkpw https://review.opendev.org/c/openstack/ironic/+/958130 | 00:05 |
|---|---|---|
| stevebaker[m] | TheJulia: is there an obvious way of seeing the metal3 regression and to see if my change helps? | 00:06 |
| TheJulia | not really, unfortunately. Metal3-integration was just fine, it was a job which uses an ironic-image for the standalone operator dmitry has in metal3 which blew up when when tests went too long | 00:08 |
| TheJulia | Testing | 00:10 |
| TheJulia | I can tell you already that just "baremetal node show <random name>" is faster, already below a second | 00:10 |
| TheJulia | That looks *way* faster | 00:13 |
| TheJulia | like, absurdly faster | 00:16 |
| stevebaker[m] | cool cool | 00:53 |
| JayF | -1 for a release note (required to backport). I'd also prefer we not land this until at least a workday has passed so other cores can chime in. I asked fungi to take a look to make sure there's not some security implication here we haven't spotted. | 01:01 |
| opendevreview | Steve Baker proposed openstack/ironic master: Memoize calls to bcrypt.checkpw https://review.opendev.org/c/openstack/ironic/+/958130 | 01:25 |
| opendevreview | OpenStack Proposal Bot proposed openstack/ironic-ui master: Imported Translations from Zanata https://review.opendev.org/c/openstack/ironic-ui/+/957829 | 02:36 |
| opendevreview | Kaifeng Wang proposed openstack/ironic master: Direct return of vmedia action during in power failure https://review.opendev.org/c/openstack/ironic/+/958147 | 06:42 |
| rpittau | good morning ironic! o/ | 07:05 |
| FreemanBoss[m] | Good morning rpittau: | 07:05 |
| abongale | gm folks | 08:10 |
| janders | what's the best way to check the state of Ironic CI? I'd like to recheck a change which passed reviews and previously passed CI but now is permafailing tempest | 08:15 |
| janders | I don't want to waste cycles blindly running recheck | 08:16 |
| frickler | janders: I'd usually look at the history of the failing job in order to see whether it only fails for a single change or always (for the last 20 runs or so) | 08:18 |
| frickler | janders: if you point to a specific change, I could also try to look at the logs and possibly give some further hints | 08:20 |
| janders | frickler thank you. I am looking at https://review.opendev.org/c/openstack/ironic/+/956972 | 08:20 |
| janders | it's failing ironic-tempest-uefi-redfish-vmedia-4k, it seems this job had 1 pass in ~25 attempts over 2 days | 08:23 |
| rpittau | I think there's a request to remove it | 08:23 |
| janders | rpittau ACK | 08:23 |
| rpittau | https://review.opendev.org/c/openstack/ironic/+/957962 | 08:23 |
| rpittau | I've recheked the patch | 08:24 |
| rpittau | just ignore it for now | 08:24 |
| janders | rpittau thank you | 08:24 |
| janders | not great CI is having issues, but I am glad I learned more how to investigate upstream issues | 08:25 |
| janders | thanks folks! | 08:25 |
| janders | will keep it in mind for the future | 08:25 |
| frickler | yes, iiuc centos published broken images that are used in that particular job | 08:32 |
| *** srelf_ is now known as Continuity | 08:49 | |
| Continuity | Morning Ironic | 08:49 |
| Continuity | Does anyone make use of DPUs as part of an ironic deployment, and if so any blogs or additional reading, im struggling to find anything in the docs about support or use cases | 08:49 |
| opendevreview | Baptiste Jonglez proposed openstack/networking-generic-switch master: tox: exclude all venv dirs from pep8 https://review.opendev.org/c/openstack/networking-generic-switch/+/958152 | 09:25 |
| opendevreview | Baptiste Jonglez proposed openstack/networking-generic-switch master: tox: Enable codespell by default and limit it to relevant files https://review.opendev.org/c/openstack/networking-generic-switch/+/958154 | 09:30 |
| opendevreview | Baptiste Jonglez proposed openstack/networking-generic-switch master: Cast boolean Netmiko kwargs to native types https://review.opendev.org/c/openstack/networking-generic-switch/+/958079 | 09:38 |
| dtantsur | TheJulia: updated re slowness. A new test I've added to https://github.com/metal3-io/ironic-standalone-operator/pull/349 proofs (IMO) that sqlite is not the issue. I'm running a non-HA Ironic with a real database, and it still takes 4 minutes to create and delete nodes. | 11:38 |
| dtantsur | The work around works, and we're unblocked. I'll create a bug to track the slow down. | 11:38 |
| dtantsur | here goes https://bugs.launchpad.net/ironic/+bug/2121143 | 11:46 |
| dtantsur | Continuity: TheJulia has the most context about DPU's than any of us, I suspect. | 11:51 |
| dtantsur | stevebaker[m]: the bug is https://bugs.launchpad.net/ironic/+bug/2121143. I'll try applying your patch but note that the normal JSON RPC case is not affected by the slow-down, only the local RPC one. | 11:54 |
| opendevreview | Merged openstack/ironic master: Make ironic-tempest-uefi-redfish-vmedia-4k non-voting https://review.opendev.org/c/openstack/ironic/+/957962 | 12:26 |
| TheJulia | dtantsur: still invokes that code path, fwiw. I found it using the tox -elocal-ironic-dev command which invokes a very similar configuration | 13:12 |
| kubajj | Hello, in https://opendev.org/openstack/ironic-python-agent/commit/2db123d318e481e4c59e305ca52e4eeb526b7659 Arne limits the number of logical disks in the target raid config to 2. | 13:29 |
| kubajj | Our question is: does anybody have a clue why he set this limit? | 13:29 |
| kubajj | Is there a possibility that this will change, or can we just assume that this is the max? | 13:29 |
| TheJulia | dtantsur: your test completed | 13:30 |
| TheJulia | kubajj: for a mirror set for the root os? | 13:30 |
| kubajj | TheJulia: sorry, should have sent the line https://opendev.org/openstack/ironic-python-agent/src/branch/master/ironic_python_agent/hardware.py#L3357 | 13:31 |
| *** dmellado62 is now known as dmellado | 13:32 | |
| TheJulia | The only thing I can think of is it is to enforce the mirror set usage for the root os | 13:32 |
| kubajj | I am not sure if I understood correctly, my question is directed to if we could support more logical disks (3+) | 13:34 |
| kubajj | TheJulia: if I understood correctly what you mean by mirror set for root os, that's exactly what we do - we have a mirrored root partition and a RAID array with the rest of the disk next to it | 13:35 |
| TheJulia | Nobody has asked. I think most would prefer to prevent folks from trying to make raid5's bootable. I mean it is *doable* but difficult. | 13:38 |
| TheJulia | JayF: I think we should get your request logging change landed today if at all possible, to at least provide clear insight into the wall time on the outer edge of the api. | 13:39 |
| dtantsur | TheJulia: yeah, unfortunately it does not have the required verbosity level, I'm fixing it :( | 13:44 |
| TheJulia | dtantsur: what doesn't? | 13:44 |
| dtantsur | I assumed you meant https://github.com/metal3-io/ironic-image/pull/742 for Steve's patch | 13:44 |
| TheJulia | you mean the job logs? | 13:45 |
| dtantsur | yeah, there is no timing | 13:46 |
| TheJulia | for the job steps? | 13:47 |
| dtantsur | Will run 19 of 19 specs | 13:47 |
| dtantsur | •••SSSSSSSSSS•••••• | 13:47 |
| dtantsur | Ran 9 of 19 Specs in 971.579 seconds | 13:47 |
| dtantsur | This does not give me much to work with :) | 13:48 |
| TheJulia | yeah, that made me raise an eyebrow | 13:48 |
| TheJulia | ok | 13:48 |
| dtantsur | just outdated configuration, fixing in https://github.com/metal3-io/ironic-image/pull/744 | 13:48 |
| TheJulia | FWIW, I looked at a local process rpc single job and it shows no sign of the huge lag, at least in what gets logged. We still need logging like JayF's patch on the API side to trace it end to end. | 13:49 |
| dtantsur | After seeing Steve's patch, I'm also wondering if TLS is making any difference | 13:51 |
| TheJulia | 13:11:08.725: rpc client logs its going to create a ndoe. 13:11:09.735 -> completed via client logging | 13:51 |
| dtantsur | Although, realistically, it should not take seconds | 13:51 |
| TheJulia | no, I was able to pin basic auth's authenticate() method down to our big pain point at least locally | 13:51 |
| TheJulia | 300ms per invocation... | 13:52 |
| kubajj | TheJulia: ok, thanks, we will take it as a fixed limit for now | 13:52 |
| dtantsur | TheJulia: so, you assume it's bcrypt combined with indirection? | 13:52 |
| TheJulia | kubajj: that *should only be* the root volume | 13:52 |
| TheJulia | dtantsur: I can confirm that is the case | 13:52 |
| dtantsur | Gotcha, thanks for looking into it | 13:53 |
| TheJulia | no problem, I still want to dig some more, but at least with the logging we have in place it sure looks like the bcrypt invocation was the huge pain point | 13:53 |
| dtantsur | Since it's local, I wonder if we can support a much more lightweight hash | 13:53 |
| TheJulia | I mean... its likely over optimization, its only 300ms once on that credential | 13:53 |
| TheJulia | afterwards it wallclocked at like 0.6ms | 13:54 |
| TheJulia | Anyway, I have 2.5 hours of meetings starting in 6 minutes, my brain is about to shift focus to that :( | 13:54 |
| dtantsur | ohhh, good luck | 13:55 |
| TheJulia | The indirection actually seems quite sporty without the extra overhead, but until I have JayF's patch in place its a little more difficult to see if we're doing other silly things which are just sinking time. | 13:56 |
| TheJulia | Other changes around verbose logging likely still make a lot of sense to change, just for human sanity as well. | 13:57 |
| dtantsur | Good reminder, left some comments on the JayF's patch | 14:07 |
| opendevreview | Baptiste Jonglez proposed openstack/networking-generic-switch master: Detect device configuration errors on Dell Force10 and OS10 https://review.opendev.org/c/openstack/networking-generic-switch/+/868318 | 14:33 |
| opendevreview | Verification of a change to openstack/ironic master failed: Fix the ability to escape service fail https://review.opendev.org/c/openstack/ironic/+/956972 | 14:44 |
| dtantsur | TheJulia: we're down to less than 20 seconds with stevebaker[m]'s patch \o/ | 14:46 |
| TheJulia | for the hundred node add/delete | 14:57 |
| dtantsur | yep | 14:58 |
| TheJulia | nice | 15:13 |
| TheJulia | sounds like it is faster than it was previously | 15:13 |
| dtantsur | It is, yes. I have a feeling that the HA case also sped up slightly. | 15:15 |
| dtantsur | Now, I have one concern with the implementation but it should not change the performance profile in the normal case. | 15:15 |
| TheJulia | which is? | 15:16 |
| dtantsur | Cache growth, see inline | 15:18 |
| TheJulia | which cache? | 15:18 |
| TheJulia | (keep in mind, on endless call of doom) | 15:18 |
| dtantsur | Sorry :) Steve is adding a cache for bcrypt comparison results, both positive and negative. This allows an attacker to grow the cache infinitely. I'm suggesting using the Python's LRU cache instead. | 15:19 |
| TheJulia | oh, yeah, oh.. heh, yeah | 15:20 |
| TheJulia | I was thinking recording should hav ebeen later, but didn't raise it yesterday due to out of spoons | 15:27 |
| JayF | good catch on that auth catching change dtantsur | 15:36 |
| JayF | I had a nagging feeling I was missing something and I'm going to proclaim that as "it" :D | 15:36 |
| TheJulia | dtantsur: is the 20 seconds on par with what it was before we removed eventlet and went to indirection? | 15:47 |
| opendevreview | Habeeb Babasulaiman proposed openstack/bifrost master: bug: drop --develop flag from testenv usage https://review.opendev.org/c/openstack/bifrost/+/946724 | 15:52 |
| dtantsur | TheJulia: unfortunately, I don't have this stats | 15:56 |
| TheJulia | pity :( | 15:56 |
| TheJulia | I'm fairly sure its likely a little faster with Steve's patch if the basic auth is enforced by Ironic for the api, if not then... likely not | 15:56 |
| TheJulia | since it is any basic auth invocation which is impacted | 15:57 |
| dtantsur | Ah, well. Before the eventlet change, we did not use RPC for the non-HA case. | 15:58 |
| TheJulia | yes | 15:58 |
| dtantsur | I can try to find some job that happened between the local RPC change and the eventlet migration.. | 15:58 |
| TheJulia | but if you were doing basic auth on the front end, you were always paying ~300ms | 15:58 |
| TheJulia | with the rpc changes, that went to 600ms | 15:58 |
| TheJulia | now, that should be functionally wiped out | 15:59 |
| dtantsur | I *think* we terminate basic auth on httpd, but not for the RPC. | 15:59 |
| TheJulia | ok | 15:59 |
| TheJulia | fair enough | 15:59 |
| dtantsur | (I wish our RPC stack adequately supported unix sockets) | 16:00 |
| TheJulia | no auth then? | 16:00 |
| dtantsur | yeah, just use ACL | 16:00 |
| dtantsur | TheJulia: https://github.com/metal3-io/ironic-standalone-operator/actions/runs/17045058098/job/48318568516 is the job I was looking for: with local RPC and eventlet. Around ~ 1.5 minutes for the node test. | 16:04 |
| dtantsur | So, quite a difference with 4 minutes. BUT! It's also before the indirection change. | 16:05 |
| TheJulia | WOW | 16:06 |
| TheJulia | WOW | 16:06 |
| TheJulia | #goodnews | 16:06 |
| dtantsur | TheJulia: now, when you have some brain space, there is another interesting aspect. I see a lot of RPC calls related to traits. We don't even use traits in Metal3. I wonder if they can be avoided. | 16:09 |
| dtantsur | Even node deletion seems to call TraitList.get_trait_names. Creation too. | 16:10 |
| dtantsur | Creation also calls TraitList.get_by_node_id | 16:10 |
| dtantsur | Okay, the latter is caused by node_convert_with_links, which probably should not expect node.get('traits') to be None (I assume it's an empty list instead) | 16:12 |
| dtantsur | hmm, no, it does not make much sense... | 16:14 |
| dtantsur | Oh. RPC create_node returns traits as a full TraitList object. And then we fetch it again. | 16:15 |
| TheJulia | on a call, but basically it all has to do with the traits flow/logic | 16:20 |
| dtantsur | I'll cook a patch soon, and you'll see if you need to yell at me for not understanding indirection :) | 16:20 |
| TheJulia | you can't nest other objects in indirection, fwiw | 16:21 |
| dtantsur | TheJulia: I may not fully understand what you mean, but RPC create_node returns a Node where node.traits is a TraitsList object | 16:22 |
| TheJulia | previously, we nested node.traits as a nested object | 16:25 |
| TheJulia | which was traits | 16:25 |
| opendevreview | Dmitry Tantsur proposed openstack/ironic master: WIP Reduce the number of RPC calls to traits API https://review.opendev.org/c/openstack/ironic/+/958226 | 16:25 |
| dtantsur | TheJulia: it still is a nested object (assuming it deserializes correctly) | 16:26 |
| TheJulia | we can't do that with an OVO nested object through indirection | 16:26 |
| dtantsur | RPC create_node returned {'ironic_object.name': 'Node', 'ironic_object.namespace': 'ironic', 'ironic_object.version': '1.42', 'ironic_object.data': {'id': 56, 'uuid': 'f96d65f2-39f3-4e5c-b360-e918a050bcec', 'name': 'node-55', 'chassis_id': None, 'instance_uuid': None, 'driver': 'redfish', 'driver_info': {}, 'driver_internal_info': {}, 'clean_step': {}, 'deploy_step': {}, 'service_step': {}, 'raid_config': {}, | 16:27 |
| dtantsur | 'target_raid_config': {}, ' | 16:27 |
| dtantsur | instance_info': {}, 'properties': {}, 'reservation': None, 'conductor_affinity': None, 'conductor_group': '', 'power_state': None, 'target_power_state': None, 'provision_state': 'enroll', 'provision_updated_at': None, 'target_provision_state': None, 'maintenance': False, 'maintenance_reason': None, 'fault': None, 'console_enabled': False, 'last_error': None, 'resource_class': None, 'inspection_finished_at': None, | 16:27 |
| dtantsur | 'inspection_started_at' | 16:27 |
| dtantsur | : None, 'extra': {}, 'automated_clean': None, 'protected': False, 'protected_reason': None, 'allocation_id': None, 'bios_interface': 'redfish', 'boot_interface': 'ipxe', 'console_interface': 'no-console', 'deploy_interface': 'direct', 'firmware_interface': 'redfish', 'inspect_interface': 'agent', 'management_interface': 'redfish', 'network_interface': 'noop', 'power_interface': 'redfish', 'raid_interface': 'redfish', | 16:27 |
| dtantsur | 'rescue_interface': | 16:27 |
| dtantsur | 'no-rescue', 'storage_interface': 'noop', 'vendor_interface': 'redfish', 'traits': {'ironic_object.name': 'TraitList', 'ironic_object.namespace': 'ironic', 'ironic_object.version': '1.1', 'ironic_object.data': {'objects': []}}, 'owner': None, 'lessee': None, 'description': None, 'retired': False, 'retired_reason': None, 'network_data': {}, 'boot_mode': None, 'secure_boot': None, 'shard': None, 'parent_node': None, | 16:27 |
| dtantsur | 'disable_power_off': | 16:27 |
| dtantsur | False, 'created_at': '2025-08-20T16:42:54Z', 'updated_at': None}} | 16:27 |
| dtantsur | sorry, too much :( | 16:27 |
| TheJulia | dude, paste ;) | 16:27 |
| dtantsur | An easy mistake: if it's small on my screen, it's small everywhere | 16:27 |
| TheJulia | no worries | 16:28 |
| dtantsur | TheJulia: see traits https://paste.opendev.org/raw/bu58ghGmyDyFb3qODQIy/ | 16:28 |
| TheJulia | Yeah, your turning off indirection on the method so it would try to read locally and then skip indirection | 16:30 |
| TheJulia | The method *must* be decorated for OVO to try and route it over | 16:30 |
| TheJulia | otherwise its doing half and half | 16:30 |
| dtantsur | Well, get_trait_names should not be remotable | 16:31 |
| dtantsur | It operates fully on a local state | 16:31 |
| TheJulia | ohh, yeah, your right! | 16:31 |
| TheJulia | hmmm! | 16:31 |
| dtantsur | This all is another cause of slowness. Any Node API call results in 1-2 RPC calls to TraitsList, regardless of whether traits are even used. | 16:32 |
| dtantsur | I hope my patch fixes that - it will be easy to see in the bifrost logs. | 16:32 |
| TheJulia | yeah, that should be fine, I think | 16:33 |
| TheJulia | but yeah | 16:33 |
| dtantsur | I think it works, next time I'll check if there are any other redundant calls | 17:14 |
| opendevreview | Dmitry Tantsur proposed openstack/ironic master: Reduce the number of RPC calls to traits API https://review.opendev.org/c/openstack/ironic/+/958226 | 17:18 |
| opendevreview | Baptiste Jonglez proposed openstack/networking-generic-switch master: Aruba: Add error message patterns to detect errors https://review.opendev.org/c/openstack/networking-generic-switch/+/958231 | 17:41 |
| opendevreview | Baptiste Jonglez proposed openstack/networking-generic-switch master: Fix support for Aruba AOS CX devices https://review.opendev.org/c/openstack/networking-generic-switch/+/958234 | 17:54 |
| *** keekz_ is now known as keekz | 20:39 | |
| *** logan_ is now known as Guest24660 | 20:52 | |
| *** keekz_ is now known as keekz | 20:55 | |
| *** naskio_ is now known as naskio | 20:55 | |
| *** abongale_ is now known as abongale | 20:55 | |
| *** jungleboyj_ is now known as jungleboyj | 20:55 | |
| *** rpittau_ is now known as rpittau | 20:55 | |
| *** tonyb_ is now known as tonyb | 20:55 | |
| *** gmaan_ is now known as gmaan | 20:55 | |
| JayF | local-ironic-dev is like, loop-restarting for me | 22:04 |
| JayF | I wonder if it's my local code | 22:04 |
| JayF | 2025-08-21 15:05:38.195 54447 INFO cotyledon._service_manager [-] Child 54521 killed by signal -11 | 22:06 |
| TheJulia | I'm sort of suspecting the single process we might need a larger stacks size | 22:08 |
| JayF | yeah looping with that on master | 22:09 |
| JayF | how do you do that? | 22:09 |
| JayF | is there a quick and dirty thing I can do to get this working to test my req logging | 22:09 |
| JayF | if not I can spin a devstack but I'm running outta time | 22:09 |
| JayF | oh, ulimit in the shell beforehand | 22:10 |
| JayF | I can try that | 22:10 |
| JayF | I wonder if tox mucks with limits, too | 22:10 |
| JayF | yeah ulimit -s unlimited in shell beforehand no dice | 22:10 |
| JayF | IRONIC_THREAD_STACK_SIZE | 22:11 |
| JayF | thanks, past julia | 22:11 |
| JayF | os.environ.get('IRONIC_THREAD_STACK_SIZE', 65536)) => TypeError: 'str' object cannot be interpreted as an integer | 22:13 |
| JayF | I'll fix it locally and push something once I sus it out | 22:13 |
| JayF | tox was eating the environment | 22:14 |
| JayF | and I had to wrap that in an int() | 22:14 |
| JayF | I'll update my logging patch first but then will followup to fix both that bug and local-ironic-dev config | 22:14 |
| TheJulia | ugh, yeah | 22:14 |
| TheJulia | makes sense, I didn't try it with the env name | 22:14 |
| JayF | I'm just glad I got to find something broken too | 22:15 |
| JayF | I was feeling left out ;) | 22:15 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: Add request logging middleware for API requests https://review.opendev.org/c/openstack/ironic/+/958103 | 22:20 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: Fix setting IRONIC_THREAD_STACK_SIZE https://review.opendev.org/c/openstack/ironic/+/958249 | 22:25 |
| opendevreview | Jay Faulkner proposed openstack/ironic master: Fix local-ironic-dev by setting stack size https://review.opendev.org/c/openstack/ironic/+/958250 | 22:25 |
| opendevreview | Steve Baker proposed openstack/ironic master: Memoize calls to bcrypt.checkpw https://review.opendev.org/c/openstack/ironic/+/958130 | 22:58 |
| opendevreview | Merged openstack/ironic master: Fix the ability to escape service fail https://review.opendev.org/c/openstack/ironic/+/956972 | 23:39 |
| janders | ^^ \o/ | 23:39 |
| janders | TheJulia if you are still around and have time, what are your general thoughts about https://review.opendev.org/c/openstack/ironic/+/957189 ? | 23:40 |
| janders | if there is any rework needed I'm happy to put some time into it today | 23:40 |
| TheJulia | Seems reasonable to me | 23:43 |
| TheJulia | Just need to get ci to be green | 23:43 |
| janders | ++ | 23:43 |
| janders | thank you Julia | 23:43 |
| janders | doco-wise, do you find https://review.opendev.org/c/openstack/ironic/+/957825 reasonable? | 23:46 |
| janders | I can work with Iury and Riccardo and get these all finished off and merged upstream and downstream today | 23:46 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!