feat(exec): Add --no-session flag for improved performance

[ryanmccann1024]

Fixes: #26588

For use cases like HPC, where podman exec is called in rapid succession, the standard exec process can become a bottleneck due to container locking and database I/O for session tracking.

This commit introduces a new --no-session flag to podman exec. When used, this flag invokes a new, lightweight backend implementation (ExecNoSession) that:

• Skips container locking, reducing lock contention.
• Bypasses the creation, tracking, and removal of exec sessions in the database.
• Executes the command directly and retrieves the exit code without persisting session state.

Does this PR introduce a user-facing change?

Added a new `--no-session` flag to `podman exec` to provide a performance-optimized execution path that bypasses container locking and database session tracking. This is ideal for high-concurrency environments like HPC where exec session tracking is not required.

[openshift-ci]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[packit-as-a-service]

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

[ryanmccann1024]

Hello @mheon,

I'm not really sure why some of the pipelines fail, I'm stuck.

[mheon]

Integration tests, you need a SkipIfRemote in your tests. System tests are both flakes.

[Honny1]

Good job, LGTM

The failed Healthcheck seems to be a flake. I tested it locally and the Healthcheck passed.

[mheon]

LGTM

[ryanmccann1024]

I think there's still a pending review @mheon

Or I'm not sure if I should do something else before that.

[mheon]

@containers/podman-maintainers PTAL

[Luap99]

is there a reason to define a new function on the interface like this, it could easily be added in ContainerExec()

In particular this function simply removes several required things that ContainerExec() does.

First this here never calls, getContainers() which means --latest will be broken

Second, it misses the if options.Tty branch to add the TERM env which means you get different behavior fro TERM in session on no session mode which seems very unexpected

Lastly it also doesn't do the tty resize logic that is in ExecAttachCtr() so the terminal is not set into the right state I think

[mheon]

I'd prefer to keep this separate - I'm expecting further changes down the line to diverge from normal Exec()

[Luap99]

different how? we can split in libpod but doing this here on the cli/ContainerEngine just seems to bypass basic code that we must always do as I pointed out above.

We can always do if execNoSession inside ContainerExec() once we looked up the container and configured the basic exec config.

what further changes are expected here where this function makes sense?

[mheon]

Complete removal of the Conmon backend in favor of directly calling OCI runtime exec directly.

[Luap99]

Sure but that can still happen within ContainerExec(), right now we duplicate common lookup logic which I find quite bad due the bugs mentioned, at the end of ContainerExec() a simple if options.NoSession would be easier IMO.

[ryanmccann1024]

Was there something I should do related to this?

I'm a little unsure, will resolve it in the meantime!

[giuseppe]

Do we have any numbers on what's the improvement is?

Can you run it under hyperfine and check what's the difference with a regular exec?

[giuseppe]

Do we have any numbers on what's the improvement is?

Can you run it under hyperfine and check what's the difference with a regular exec?

I did the test on my machine and the results are very good:

➜ hyperfine 'bin/podman exec foo true' 'bin/podman exec --no-session foo true'
Benchmark 1: bin/podman exec foo true
  Time (mean ± σ):      80.2 ms ±   2.9 ms    [User: 22.6 ms, System: 14.6 ms]
  Range (min … max):    74.9 ms …  86.5 ms    34 runs

Benchmark 2: bin/podman exec --no-session foo true
  Time (mean ± σ):      29.9 ms ±   6.8 ms    [User: 20.1 ms, System: 11.9 ms]
  Range (min … max):    22.3 ms …  54.1 ms    97 runs

Summary
  bin/podman exec --no-session foo true ran
    2.68 ± 0.62 times faster than bin/podman exec foo true

[Luap99]

Got it!

I think I'm all set to keep working on this although I'm not fully clear on the last suggestion made by @Luap99 (I'm a newbie).

There is this in makeExecConfig() which sets a exit command on conmon which we don't want.

	// TODO: Add some ability to toggle syslog
	exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), false, false, true)
	if err != nil {
		return nil, fmt.Errorf("constructing exit command for exec session: %w", err)
	}
	execConfig.ExitCommand = exitCommandArgs

So in theory all you need to do is to make sure the command is nil for you exec config then it should work I think.

[mheon]

I think the problem here is that this is running serially - maybe put the execSession bit in a goroutine so they run concurrent? Though you'd need a sleep to sequence them, give time for the first exec to start (our CI is really slow)

[ryanmccann1024]

Any suggestions on how to fix the failure on that pipeline due to the merge?

I think there's also a linting error from upstream?

[Honny1]

Linting error fix: #27234

[Honny1]

@ryanmccann1024 Could you please rebase on main?

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Honny1, ryanmccann1024

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [Honny1]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ryanmccann1024]

Hello,

Should I do anything else for this PR?

[Honny1]

Code LGTM. Please just rebase onto the latest main branch, as there were changes to the linter. This will verify that the new linter rules are satisfied.

cc @Luap99

[mheon]

Restarted 3 flakes

[mheon]

/lgtm

[mheon]

I don't know why it's not picking up the release note. Removed the label.

[Luap99]

Personally I am still not happy with the duplication between ContainerExecNoSession() and ContainerExec(), people will forget to update them in sync if new option get added and it seems really unnecessary to me to define more of these "engine" interface functions when there is exactly one call only different.

But since others want to merge this then I won't object further