Skip to content

Commit c32b5dd

Browse files
committed
Fix change in runc kill all behavior
`runc kill -all` ignores `libcontainer.ErrNotRunning`, but `runc kill` does not. Since we used to equate `SIGTERM` and `SIGKILL` with `-all`, restore old behavior by ignoring `"container not running"` runc errors if those signals are sent. See: opencontainers/runc@f8ad20f#diff-ade6035c3e554d7627cdc368b27f475fc0dad83e02382a1dea9cae9b75871087 Additionally, switch to using error strings directly from runc code in `internal\guest\runtime\runc\utils.go`: they have been available since runc v1.1.0-rc.1. Also, add logic to match on container not/still running error strings. See: opencontainers/runc#3033 Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
1 parent 31ea91d commit c32b5dd

File tree

2 files changed

+13
-10
lines changed

2 files changed

+13
-10
lines changed

internal/guest/runtime/runc/container.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,10 @@ func (c *container) Kill(signal syscall.Signal) error {
8383
out, err := cmd.CombinedOutput()
8484
if err != nil {
8585
runcErr := parseRuncError(string(out))
86-
return errors.Wrapf(runcErr, "unknown runc error after kill %v: %s", err, string(out))
86+
if (signal == syscall.SIGTERM || signal == syscall.SIGKILL) && errors.Is(runcErr, runtime.ErrContainerNotRunning) {
87+
return nil
88+
}
89+
return errors.Wrapf(runcErr, "runc kill failed with %v: %s", err, string(out))
8790
}
8891
return nil
8992
}

internal/guest/runtime/runc/utils.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"strings"
1313
"syscall"
1414

15+
"github.com/opencontainers/runc/libcontainer"
1516
"github.com/pkg/errors"
1617
"github.com/sirupsen/logrus"
1718

@@ -124,20 +125,19 @@ func (l *standardLogEntry) asError() (err error) {
124125
}
125126

126127
func parseRuncError(s string) (err error) {
127-
// TODO (helsaawy): match with errors from
128-
// https://github.com/opencontainers/runc/blob/master/libcontainer/error.go
129128
if strings.HasPrefix(s, "container") && strings.HasSuffix(s, "does not exist") {
130-
// currently: "container <container id> does not exist"
129+
// match "container %q does not exist" and [libcontainer.ErrNotExist]
131130
err = runtime.ErrContainerDoesNotExist
132-
} else if strings.Contains(s, "container with id exists") ||
133-
strings.Contains(s, "container with given ID already exists") {
131+
} else if strings.Contains(s, "container with id exists") || strings.Contains(s, libcontainer.ErrExist.Error()) {
134132
err = runtime.ErrContainerAlreadyExists
135-
} else if strings.Contains(s, "invalid id format") ||
136-
strings.Contains(s, "invalid container ID format") {
133+
} else if strings.Contains(s, "invalid id format") || strings.Contains(s, libcontainer.ErrInvalidID.Error()) {
137134
err = runtime.ErrInvalidContainerID
138-
} else if strings.Contains(s, "container") &&
139-
strings.Contains(s, "that is not stopped") {
135+
} else if strings.Contains(s, "container") && strings.Contains(s, "that is not stopped") {
140136
err = runtime.ErrContainerNotStopped
137+
} else if strings.Contains(s, libcontainer.ErrRunning.Error()) {
138+
err = runtime.ErrContainerStillRunning
139+
} else if strings.Contains(s, libcontainer.ErrNotRunning.Error()) {
140+
err = runtime.ErrContainerNotRunning
141141
} else {
142142
err = errors.New(s)
143143
}

0 commit comments

Comments
 (0)