Skip to content

Commit 7cd9b81

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #26727 from ryanmccann1024/feature/26588-exec-no-session
feat(exec): Add --no-session flag for improved performance
2 parents dfdd3b5 + 61cbc0c commit 7cd9b81

File tree

8 files changed

+237
-105
lines changed

8 files changed

+237
-105
lines changed

‎cmd/podman/containers/exec.go‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ var (
5151
execOpts entities.ExecOptions
5252
execDetach bool
5353
execCidFile string
54+
execNoSession bool
5455
)
5556

5657
func execFlags(cmd *cobra.Command) {
@@ -100,6 +101,10 @@ func execFlags(cmd *cobra.Command) {
100101
flags.Int32(waitFlagName, 0, "Total seconds to wait for container to start")
101102
_ = flags.MarkHidden(waitFlagName)
102103

104+
if !registry.IsRemote() {
105+
flags.BoolVar(&execNoSession, "no-session", false, "Do not create a database session for the exec process")
106+
}
107+
103108
if registry.IsRemote() {
104109
_ = flags.MarkHidden("preserve-fds")
105110
}
@@ -121,6 +126,12 @@ func init() {
121126
}
122127

123128
func exec(cmd *cobra.Command, args []string) error {
129+
if execNoSession {
130+
if execDetach || cmd.Flags().Changed("detach-keys") {
131+
return errors.New("--no-session cannot be used with --detach or --detach-keys")
132+
}
133+
}
134+
124135
nameOrID, command, err := determineTargetCtrAndCmd(args, execOpts.Latest, execCidFile != "")
125136
if err != nil {
126137
return err
@@ -168,17 +179,21 @@ func exec(cmd *cobra.Command, args []string) error {
168179
}
169180
}
170181

171-
if !execDetach {
172-
streams := define.AttachStreams{}
173-
streams.OutputStream = os.Stdout
174-
streams.ErrorStream = os.Stderr
175-
if execOpts.Interactive {
176-
streams.InputStream = bufio.NewReader(os.Stdin)
177-
streams.AttachInput = true
178-
}
179-
streams.AttachOutput = true
180-
streams.AttachError = true
182+
streams := define.AttachStreams{}
183+
streams.OutputStream = os.Stdout
184+
streams.ErrorStream = os.Stderr
185+
if execOpts.Interactive {
186+
streams.InputStream = bufio.NewReader(os.Stdin)
187+
streams.AttachInput = true
188+
}
189+
streams.AttachOutput = true
190+
streams.AttachError = true
181191

192+
if execNoSession {
193+
exitCode, err := registry.ContainerEngine().ContainerExecNoSession(registry.Context(), nameOrID, execOpts, streams)
194+
registry.SetExitCode(exitCode)
195+
return err
196+
} else if !execDetach {
182197
exitCode, err := registry.ContainerEngine().ContainerExec(registry.Context(), nameOrID, execOpts, streams)
183198
registry.SetExitCode(exitCode)
184199
return err

‎docs/source/markdown/options/no-session.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
####> This option file is used in:
2+
####> podman exec
3+
####> If file is edited, make sure the changes
4+
####> are applicable to all of those.
5+
#### **--no-session**
6+
7+
Do not create a database session for the exec process. This can improve performance but the exec session will not be visible to other podman commands.

‎docs/source/markdown/podman-exec.1.md.in‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ Start the exec session, but do not attach to it. The command runs in the backgro
3131

3232
@@option latest
3333

34+
@@option no-session
35+
3436
@@option preserve-fd
3537

3638
@@option preserve-fds

‎libpod/container_exec.go‎

Lines changed: 97 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -822,87 +822,7 @@ func (c *Container) ExecResize(sessionID string, newSize resize.TerminalSize) er
822822
}
823823

824824
func (c *Container) healthCheckExec(config *ExecConfig, timeout time.Duration, streams *define.AttachStreams) (int, error) {
825-
unlock := true
826-
if !c.batched {
827-
c.lock.Lock()
828-
defer func() {
829-
if unlock {
830-
c.lock.Unlock()
831-
}
832-
}()
833-
834-
if err := c.syncContainer(); err != nil {
835-
return -1, err
836-
}
837-
}
838-
839-
if err := c.verifyExecConfig(config); err != nil {
840-
return -1, err
841-
}
842-
843-
if !c.ensureState(define.ContainerStateRunning) {
844-
return -1, fmt.Errorf("can only create exec sessions on running containers: %w", define.ErrCtrStateInvalid)
845-
}
846-
847-
session, err := c.createExecSession(config)
848-
if err != nil {
849-
return -1, err
850-
}
851-
852-
if c.state.ExecSessions == nil {
853-
c.state.ExecSessions = make(map[string]*ExecSession)
854-
}
855-
c.state.ExecSessions[session.ID()] = session
856-
defer delete(c.state.ExecSessions, session.ID())
857-
858-
opts, err := prepareForExec(c, session)
859-
if err != nil {
860-
return -1, err
861-
}
862-
defer func() {
863-
// cleanupExecBundle MUST be called with the parent container locked.
864-
if !unlock && !c.batched {
865-
c.lock.Lock()
866-
unlock = true
867-
868-
if err := c.syncContainer(); err != nil {
869-
logrus.Errorf("Error syncing container %s state: %v", c.ID(), err)
870-
// Normally we'd want to continue here, get rid of the exec directory.
871-
// But the risk of proceeding into a function that can mutate state with a bad state is high.
872-
// Lesser of two evils is to bail and leak a directory.
873-
return
874-
}
875-
}
876-
if err := c.cleanupExecBundle(session.ID()); err != nil {
877-
logrus.Errorf("Container %s light exec session cleanup error: %v", c.ID(), err)
878-
}
879-
}()
880-
881-
pid, attachErrChan, err := c.ociRuntime.ExecContainer(c, session.ID(), opts, streams, nil)
882-
if err != nil {
883-
return -1, err
884-
}
885-
session.PID = pid
886-
session.PIDData = getPidData(pid)
887-
888-
if !c.batched {
889-
c.lock.Unlock()
890-
unlock = false
891-
}
892-
893-
select {
894-
case err = <-attachErrChan:
895-
if err != nil {
896-
return -1, fmt.Errorf("container %s light exec session with pid: %d error: %v", c.ID(), pid, err)
897-
}
898-
case <-time.After(timeout):
899-
if err := c.ociRuntime.ExecStopContainer(c, session.ID(), 0); err != nil {
900-
return -1, err
901-
}
902-
return -1, fmt.Errorf("%v of %s", define.ErrHealthCheckTimeout, c.HealthCheckConfig().Timeout.String())
903-
}
904-
905-
return c.readExecExitCode(session.ID())
825+
return c.execLightweight(config, streams, timeout)
906826
}
907827

908828
func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resize <-chan resize.TerminalSize) (int, error) {
@@ -1321,3 +1241,99 @@ func justWriteExecExitCode(c *Container, sessionID string, exitCode int, emitEve
13211241
// Finally, save our changes.
13221242
return c.save()
13231243
}
1244+
1245+
// execLightweight executes a command in a container without creating a persistent exec session.
1246+
// It is used by both ExecNoSession and healthCheckExec to avoid code duplication.
1247+
func (c *Container) execLightweight(config *ExecConfig, streams *define.AttachStreams, timeout time.Duration) (int, error) {
1248+
if err := c.verifyExecConfig(config); err != nil {
1249+
return -1, err
1250+
}
1251+
1252+
unlock := true
1253+
if !c.batched {
1254+
c.lock.Lock()
1255+
defer func() {
1256+
if unlock {
1257+
c.lock.Unlock()
1258+
}
1259+
}()
1260+
1261+
if err := c.syncContainer(); err != nil {
1262+
return -1, err
1263+
}
1264+
}
1265+
1266+
if !c.ensureState(define.ContainerStateRunning) {
1267+
return -1, fmt.Errorf("can only create exec sessions on running containers: %w", define.ErrCtrStateInvalid)
1268+
}
1269+
1270+
session, err := c.createExecSession(config)
1271+
if err != nil {
1272+
return -1, err
1273+
}
1274+
1275+
if c.state.ExecSessions == nil {
1276+
c.state.ExecSessions = make(map[string]*ExecSession)
1277+
}
1278+
c.state.ExecSessions[session.ID()] = session
1279+
defer delete(c.state.ExecSessions, session.ID())
1280+
1281+
opts, err := prepareForExec(c, session)
1282+
if err != nil {
1283+
return -1, err
1284+
}
1285+
defer func() {
1286+
if err := c.cleanupExecBundle(session.ID()); err != nil {
1287+
logrus.Errorf("Container %s light exec session cleanup error: %v", c.ID(), err)
1288+
}
1289+
}()
1290+
1291+
pid, attachErrChan, err := c.ociRuntime.ExecContainer(c, session.ID(), opts, streams, nil)
1292+
if err != nil {
1293+
// Check if the error is command not found before returning
1294+
if exitCode := define.ExitCode(err); exitCode == define.ExecErrorCodeNotFound {
1295+
return exitCode, err
1296+
}
1297+
return define.ExecErrorCodeGeneric, err
1298+
}
1299+
session.PID = pid
1300+
session.PIDData = getPidData(pid)
1301+
1302+
if !c.batched {
1303+
c.lock.Unlock()
1304+
unlock = false
1305+
}
1306+
1307+
// Handle timeout for health checks
1308+
if timeout > 0 {
1309+
select {
1310+
case err = <-attachErrChan:
1311+
if err != nil {
1312+
return -1, fmt.Errorf("container %s light exec session with pid: %d error: %v", c.ID(), pid, err)
1313+
}
1314+
case <-time.After(timeout):
1315+
if err := c.ociRuntime.ExecStopContainer(c, session.ID(), 0); err != nil {
1316+
return -1, err
1317+
}
1318+
return -1, fmt.Errorf("%v of %s", define.ErrHealthCheckTimeout, timeout.String())
1319+
}
1320+
} else {
1321+
// For no-session exec, wait for completion without timeout
1322+
err = <-attachErrChan
1323+
if err != nil && !errors.Is(err, define.ErrDetach) {
1324+
// Check if the error is command not found
1325+
if exitCode := define.ExitCode(err); exitCode == define.ExecErrorCodeNotFound {
1326+
return exitCode, err
1327+
}
1328+
return define.ExecErrorCodeGeneric, err
1329+
}
1330+
}
1331+
1332+
return c.readExecExitCode(session.ID())
1333+
}
1334+
1335+
// ExecNoSession executes a command in a container without creating a persistent exec session.
1336+
// It skips database operations and minimizes container locking for performance.
1337+
func (c *Container) ExecNoSession(config *ExecConfig, streams *define.AttachStreams, _ <-chan resize.TerminalSize) (int, error) {
1338+
return c.execLightweight(config, streams, 0)
1339+
}

‎pkg/domain/entities/engine_container.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ type ContainerEngine interface { //nolint:interfacebloat
2626
ContainerCopyToArchive(ctx context.Context, nameOrID string, path string, writer io.Writer) (ContainerCopyFunc, error)
2727
ContainerCreate(ctx context.Context, s *specgen.SpecGenerator) (*ContainerCreateReport, error)
2828
ContainerExec(ctx context.Context, nameOrID string, options ExecOptions, streams define.AttachStreams) (int, error)
29+
ContainerExecNoSession(ctx context.Context, nameOrID string, options ExecOptions, streams define.AttachStreams) (int, error)
2930
ContainerExecDetached(ctx context.Context, nameOrID string, options ExecOptions) (string, error)
3031
ContainerExists(ctx context.Context, nameOrID string, options ContainerExistsOptions) (*BoolReport, error)
3132
ContainerExport(ctx context.Context, nameOrID string, options ContainerExportOptions) error

‎pkg/domain/infra/abi/containers.go‎

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -872,7 +872,7 @@ func (ic *ContainerEngine) ContainerAttach(ctx context.Context, nameOrID string,
872872
return nil
873873
}
874874

875-
func makeExecConfig(options entities.ExecOptions, rt *libpod.Runtime) (*libpod.ExecConfig, error) {
875+
func makeExecConfig(options entities.ExecOptions, rt *libpod.Runtime, noSession bool) (*libpod.ExecConfig, error) {
876876
execConfig := new(libpod.ExecConfig)
877877
execConfig.Command = options.Cmd
878878
execConfig.Terminal = options.Tty
@@ -885,18 +885,21 @@ func makeExecConfig(options entities.ExecOptions, rt *libpod.Runtime) (*libpod.E
885885
execConfig.PreserveFD = options.PreserveFD
886886
execConfig.AttachStdin = options.Interactive
887887

888-
// Make an exit command
889-
storageConfig := rt.StorageConfig()
890-
runtimeConfig, err := rt.GetConfig()
891-
if err != nil {
892-
return nil, fmt.Errorf("retrieving Libpod configuration to build exec exit command: %w", err)
893-
}
894-
// TODO: Add some ability to toggle syslog
895-
exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), false, false, true)
896-
if err != nil {
897-
return nil, fmt.Errorf("constructing exit command for exec session: %w", err)
888+
// Only set up exit command for regular exec sessions, not no-session mode
889+
if !noSession {
890+
// Make an exit command
891+
storageConfig := rt.StorageConfig()
892+
runtimeConfig, err := rt.GetConfig()
893+
if err != nil {
894+
return nil, fmt.Errorf("retrieving Libpod configuration to build exec exit command: %w", err)
895+
}
896+
// TODO: Add some ability to toggle syslog
897+
exitCommandArgs, err := specgenutil.CreateExitCommandArgs(storageConfig, runtimeConfig, logrus.IsLevelEnabled(logrus.DebugLevel), false, false, true)
898+
if err != nil {
899+
return nil, fmt.Errorf("constructing exit command for exec session: %w", err)
900+
}
901+
execConfig.ExitCommand = exitCommandArgs
898902
}
899-
execConfig.ExitCommand = exitCommandArgs
900903

901904
return execConfig, nil
902905
}
@@ -946,7 +949,7 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrID string, o
946949
util.ExecAddTERM(ctr.Env(), options.Envs)
947950
}
948951

949-
execConfig, err := makeExecConfig(options, ic.Libpod)
952+
execConfig, err := makeExecConfig(options, ic.Libpod, false)
950953
if err != nil {
951954
return ec, err
952955
}
@@ -955,6 +958,36 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrID string, o
955958
return define.TranslateExecErrorToExitCode(ec, err), err
956959
}
957960

961+
func (ic *ContainerEngine) ContainerExecNoSession(_ context.Context, nameOrID string, options entities.ExecOptions, streams define.AttachStreams) (int, error) {
962+
ec := define.ExecErrorCodeGeneric
963+
err := checkExecPreserveFDs(options)
964+
if err != nil {
965+
return ec, err
966+
}
967+
968+
containers, err := getContainers(ic.Libpod, getContainersOptions{latest: options.Latest, names: []string{nameOrID}})
969+
if err != nil {
970+
return ec, err
971+
}
972+
if len(containers) != 1 {
973+
return ec, fmt.Errorf("%w: expected to find exactly one container but got %d", define.ErrInternal, len(containers))
974+
}
975+
ctr := containers[0]
976+
977+
if options.Tty {
978+
util.ExecAddTERM(ctr.Env(), options.Envs)
979+
}
980+
981+
execConfig, err := makeExecConfig(options, ic.Libpod, true)
982+
if err != nil {
983+
return ec, err
984+
}
985+
986+
ec, err = ctr.ExecNoSession(execConfig, &streams, nil)
987+
// Translate exit codes for consistency with regular exec
988+
return define.TranslateExecErrorToExitCode(ec, err), err
989+
}
990+
958991
func (ic *ContainerEngine) ContainerExecDetached(_ context.Context, nameOrID string, options entities.ExecOptions) (string, error) {
959992
err := checkExecPreserveFDs(options)
960993
if err != nil {
@@ -970,7 +1003,7 @@ func (ic *ContainerEngine) ContainerExecDetached(_ context.Context, nameOrID str
9701003
}
9711004
ctr := containers[0]
9721005

973-
execConfig, err := makeExecConfig(options, ic.Libpod)
1006+
execConfig, err := makeExecConfig(options, ic.Libpod, false)
9741007
if err != nil {
9751008
return "", err
9761009
}

‎pkg/domain/infra/tunnel/containers.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,10 @@ func (ic *ContainerEngine) ContainerExec(_ context.Context, nameOrID string, opt
656656
return inspectOut.ExitCode, nil
657657
}
658658

659+
func (ic *ContainerEngine) ContainerExecNoSession(_ context.Context, _ string, _ entities.ExecOptions, _ define.AttachStreams) (int, error) {
660+
return 0, errors.New("--no-session is not supported for the remote client")
661+
}
662+
659663
func (ic *ContainerEngine) ContainerExecDetached(_ context.Context, nameOrID string, options entities.ExecOptions) (retSessionID string, retErr error) {
660664
createConfig := makeExecConfig(options)
661665

0 commit comments

Comments
 (0)