Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 59 additions & 3 deletions libpod/container_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,22 +760,78 @@ func (c *Container) ExecResize(sessionID string, newSize resize.TerminalSize) er
return c.ociRuntime.ExecAttachResize(c, sessionID, newSize)
}

// ExecKill sends a signal to the process group of a running exec session,
// reaching any children it spawned, not just the tracked PID.
func (c *Container) ExecKill(sessionID string, sig uint) error {
if !c.batched {
c.lock.Lock()
defer c.lock.Unlock()

if err := c.syncContainer(); err != nil {
return err
}
}

session, ok := c.state.ExecSessions[sessionID]
if !ok {
return fmt.Errorf("container %s has no exec session with ID %s: %w", c.ID(), sessionID, define.ErrNoSuchExecSession)
}

if session.State != define.ExecStateRunning {
return fmt.Errorf("cannot signal container %s exec session %s as it is not running: %w", c.ID(), session.ID(), define.ErrExecSessionStateInvalid)
}

// Also confirms via pidhandle that this PID is still our session, not
// one the kernel already reused.
running, err := c.ociRuntime.ExecUpdateStatus(c, session.ID())
if err != nil {
return err
}
if !running {
session.State = define.ExecStateStopped

if err := c.save(); err != nil {
logrus.Errorf("Saving state of container %s: %v", c.ID(), err)
}

return fmt.Errorf("cannot signal container %s exec session %s as it has stopped: %w", c.ID(), session.ID(), define.ErrExecSessionStateInvalid)
}

pid := session.PID

// pidhandle can't deliver here, pidfd_send_signal only reaches one
// process, not a group. The check above closes most of the reuse gap.
if err := unix.Kill(-pid, unix.Signal(sig)); err != nil {
if errors.Is(err, unix.ESRCH) {
return nil
}
return fmt.Errorf("killing container %s exec session %s process group %d: %w", c.ID(), session.ID(), pid, err)
}

return nil
}

func (c *Container) healthCheckExec(config *ExecConfig, timeout time.Duration, streams *define.AttachStreams) (int, error) {
return c.execLightweight(config, streams, timeout)
}

func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resize <-chan resize.TerminalSize) (int, error) {
return c.exec(config, streams, resize, false)
// sessionIDCallback, if not nil, fires with the session's ID right after
// creation, before start/attach, for callers that need it early.
func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resize <-chan resize.TerminalSize, sessionIDCallback func(string)) (int, error) {
return c.exec(config, streams, resize, false, sessionIDCallback)
}

// Exec emulates the old Libpod exec API, providing a single call to create,
// run, and remove an exec session. Returns exit code and error. Exit code is
// not guaranteed to be set sanely if error is not nil.
func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resizeChan <-chan resize.TerminalSize, isHealthcheck bool) (exitCode int, retErr error) {
func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resizeChan <-chan resize.TerminalSize, isHealthcheck bool, sessionIDCallback func(string)) (exitCode int, retErr error) {
sessionID, err := c.ExecCreate(config)
if err != nil {
return -1, err
}
if sessionIDCallback != nil {
sessionIDCallback(sessionID)
}
cleanup := true
defer func() {
if cleanup {
Expand Down
2 changes: 1 addition & 1 deletion libpod/container_top_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func (c *Container) execPSinContainer(args []string) ([]string, error) {
cmd := append([]string{"ps"}, args...)
config := new(ExecConfig)
config.Command = cmd
ec, err := c.Exec(config, streams, nil)
ec, err := c.Exec(config, streams, nil, nil)
wPipe.Close()
if err != nil {
return nil, err
Expand Down
36 changes: 36 additions & 0 deletions pkg/api/handlers/compat/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,42 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) {
logrus.Debugf("Attach for container %s exec session %s completed successfully", sessionCtr.ID(), sessionID)
}

// ExecKillHandler sends a signal to a running exec session. Libpod-only, so
// the signal is a plain integer, not a compat name string.
func ExecKillHandler(w http.ResponseWriter, r *http.Request) {
runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
decoder := utils.GetDecoder(r)

sessionID := mux.Vars(r)["id"]

query := struct {
Signal uint `schema:"signal"`
}{}
if err := decoder.Decode(&query, r.URL.Query()); err != nil {
utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err))
return
}

sessionCtr, err := runtime.GetExecSessionContainer(sessionID)
if err != nil {
utils.Error(w, http.StatusNotFound, err)
return
}

if err := sessionCtr.ExecKill(sessionID, query.Signal); err != nil {
if errors.Is(err, define.ErrNoSuchExecSession) {
utils.Error(w, http.StatusNotFound, err)
return
}
if errors.Is(err, define.ErrExecSessionStateInvalid) {
utils.Error(w, http.StatusConflict, err)
return
}
utils.InternalServerError(w, err)
return
}
}

// ExecRemoveHandler removes a exec session.
func ExecRemoveHandler(w http.ResponseWriter, r *http.Request) {
runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
Expand Down
31 changes: 31 additions & 0 deletions pkg/api/server/register_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,5 +386,36 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error {
// 500:
// $ref: "#/responses/internalError"
r.Handle(VersionedPath("/libpod/exec/{id}/remove"), s.APIHandler(compat.ExecRemoveHandler)).Methods(http.MethodPost)

// swagger:operation POST /libpod/exec/{id}/kill libpod ExecKillLibpod
// ---
// tags:
// - exec
// summary: Signal an exec instance
// description: |
// Send a signal to a running exec instance's process group.
// parameters:
// - in: path
// name: id
// type: string
// required: true
// description: Exec instance ID
// - in: query
// name: signal
// type: integer
// required: true
// description: Signal number to send.
// produces:
// - application/json
// responses:
// 200:
// description: no error
// 404:
// $ref: "#/responses/execSessionNotFound"
// 409:
// description: exec session is not running.
// 500:
// $ref: "#/responses/internalError"
r.Handle(VersionedPath("/libpod/exec/{id}/kill"), s.APIHandler(compat.ExecKillHandler)).Methods(http.MethodPost)
return nil
}
18 changes: 18 additions & 0 deletions pkg/bindings/containers/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,24 @@ func ResizeExecTTY(ctx context.Context, sessionID string, options *ResizeExecTTY
return resizeTTY(ctx, bindings.JoinURL("exec", sessionID, "resize"), options.Height, options.Width)
}

// ExecKill sends a signal to a running exec session's process group.
func ExecKill(ctx context.Context, sessionID string, sig uint) error {
conn, err := bindings.GetClient(ctx)
if err != nil {
return err
}

params := url.Values{}
params.Set("signal", strconv.FormatUint(uint64(sig), 10))
rsp, err := conn.DoRequest(ctx, nil, http.MethodPost, bindings.JoinURL("exec", sessionID, "kill"), params, nil)
if err != nil {
return err
}
defer rsp.Body.Close()

return rsp.Process(nil)
}

// resizeTTY set size of TTY of container
func resizeTTY(ctx context.Context, endpoint string, height *int, width *int) error {
conn, err := bindings.GetClient(ctx)
Expand Down
34 changes: 34 additions & 0 deletions pkg/domain/infra/abi/terminal/sigproxy_commn.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,40 @@ import (
"go.podman.io/podman/v6/pkg/signal"
)

// ProxyExecSignals forwards signals Podman receives to an exec session, via
// Container.ExecKill so delivery goes through libpod's own PID bookkeeping.
func ProxyExecSignals(ctr *libpod.Container, sessionID string) {
// Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going
// to the exec session now.
shutdown.Stop() //nolint: errcheck

sigBuffer := make(chan os.Signal, signal.SignalBufferSize)
signal.CatchAll(sigBuffer)

logrus.Debugf("Enabling signal proxying to exec session %s", sessionID)

go func() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goroutine started by ProxyExecSignals runs forever if no signal is ever sent. In the happy-path case, most exec sessions exit normally. This is fine for a CLI tool that's about to exit.

for s := range sigBuffer {
syscallSignal := s.(syscall.Signal)

if err := ctr.ExecKill(sessionID, uint(syscallSignal)); err != nil {
if !errors.Is(err, define.ErrExecSessionStateInvalid) && !errors.Is(err, define.ErrNoSuchExecSession) {
logrus.Errorf("forwarding signal %d to exec session %s: %v", s, sessionID, err)
continue
}
// Session is gone: send this one to ourselves rather than
// lose it, and let the defaults play out.
logrus.Infof("Ceasing signal forwarding, exec session %s has stopped", sessionID)
signal.StopCatch(sigBuffer)
if err := syscall.Kill(syscall.Getpid(), syscallSignal); err != nil {
logrus.Errorf("Failed to kill pid %d", syscall.Getpid())
}
return
}
}
}()
}

// ProxySignals ...
func ProxySignals(ctr *libpod.Container) {
// Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going
Expand Down
5 changes: 4 additions & 1 deletion pkg/domain/infra/abi/terminal/terminal_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, execConfig *libpo
}
}()
}
return ctr.Exec(execConfig, streams, resizechan)
// Forward our signals on, so killing `podman exec` stops what it started.
return ctr.Exec(execConfig, streams, resizechan, func(sessionID string) {
ProxyExecSignals(ctr, sessionID)
})
}

// StartAttachCtr starts and (if required) attaches to a container
Expand Down
10 changes: 10 additions & 0 deletions pkg/domain/infra/tunnel/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"go.podman.io/podman/v6/pkg/domain/entities"
"go.podman.io/podman/v6/pkg/domain/entities/reports"
"go.podman.io/podman/v6/pkg/errorhandling"
"go.podman.io/podman/v6/pkg/signal"
"go.podman.io/podman/v6/pkg/specgen"
"go.podman.io/podman/v6/pkg/util"
"go.podman.io/storage/types"
Expand Down Expand Up @@ -635,6 +636,15 @@ func (ic *ContainerEngine) ContainerExec(_ context.Context, nameOrID string, opt
if err != nil {
return 125, err
}
// Forward our signals on, so killing `podman exec` stops what it started,
// same as the local path does via ProxyExecSignals.
remoteProxySignals(sessionID, func(sigName string) error {
sig, err := signal.ParseSignalNameOrNumber(sigName)
if err != nil {
return err
}
return containers.ExecKill(ic.ClientCtx, sessionID, uint(sig))
})
defer func() {
if err := containers.ExecRemove(ic.ClientCtx, sessionID, nil); err != nil {
apiErr := new(bindings.APIVersionError)
Expand Down
50 changes: 50 additions & 0 deletions test/system/032-sig-proxy.bats
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,54 @@ load helpers.sig-proxy
_test_sigproxy c_attach $kidpid
}

@test "podman sigproxy test: exec" {
local cname=c-exec-$(safename)
run_podman run -d --name $cname $IMAGE top

# sleep 97 is spawned by the exec'd shell, so both share a process group
# and signalling it must take down both.
# See above comments regarding $PODMAN and backgrounding.
# SIGTERM, not SIGINT: shells ignore SIGINT in background jobs.
"${PODMAN_CMD[@]}" exec $cname sh -c 'sleep 97 & sleep 98' &
local kidpid=$!

# Wait for both to come up
local timeout=10
while :;do
sleep 0.5
run_podman top $cname args
if [[ "$output" =~ "sleep 97" ]] && [[ "$output" =~ "sleep 98" ]]; then
break
fi
timeout=$((timeout - 1))
if [[ $timeout -eq 0 ]]; then
die "Timed out waiting for exec'd processes to start"
fi
done

kill -TERM $kidpid

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe wait after sending signal?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it.

local exec_status=0
wait $kidpid || exec_status=$?
# 128 + SIGTERM(15): the exec'd shell died from the forwarded signal,
# not from some other error.
is "$exec_status" "143" "podman exec exit status reflects SIGTERM"

# Neither may outlive the exec. A dead child lingers as "[sleep]", so
# match full command lines.
timeout=20
while :;do
sleep 0.5
run_podman top $cname args
if [[ ! "$output" =~ "sleep 9" ]]; then
break
fi
timeout=$((timeout - 1))
if [[ $timeout -eq 0 ]]; then
die "Timed out waiting for exec'd processes to be signalled"
fi
done

run_podman rm -f -t0 $cname
}

# vim: filetype=sh