diff --git a/runtime/v1/shim/service.go b/runtime/v1/shim/service.go index f3e1f4b7c..c0ec03931 100644 --- a/runtime/v1/shim/service.go +++ b/runtime/v1/shim/service.go @@ -521,13 +521,8 @@ func (s *Service) checkProcesses(e runc.Exit) { } if ip, ok := p.(*process.Init); ok { - shouldKillAll, err := shouldKillAllOnExit(s.bundle) - if err != nil { - log.G(s.context).WithError(err).Error("failed to check shouldKillAll") - } - // Ensure all children are killed - if shouldKillAll { + if shouldKillAllOnExit(s.context, s.bundle) { if err := ip.KillAll(s.context); err != nil { log.G(s.context).WithError(err).WithField("id", ip.ID()). Error("failed to kill init's children") @@ -547,23 +542,25 @@ func (s *Service) checkProcesses(e runc.Exit) { } } -func shouldKillAllOnExit(bundlePath string) (bool, error) { +func shouldKillAllOnExit(ctx context.Context, bundlePath string) bool { var bundleSpec specs.Spec bundleConfigContents, err := ioutil.ReadFile(filepath.Join(bundlePath, "config.json")) if err != nil { - return false, err + log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to read config.json") + return true + } + if err := json.Unmarshal(bundleConfigContents, &bundleSpec); err != nil { + log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to unmarshal bundle json") + return true } - json.Unmarshal(bundleConfigContents, &bundleSpec) - if bundleSpec.Linux != nil { for _, ns := range bundleSpec.Linux.Namespaces { if ns.Type == specs.PIDNamespace && ns.Path == "" { - return false, nil + return false } } } - - return true, nil + return true } func (s *Service) getContainerPids(ctx context.Context, id string) ([]uint32, error) { diff --git a/runtime/v2/runc/util.go b/runtime/v2/runc/util.go index 51ca04864..166597d49 100644 --- a/runtime/v2/runc/util.go +++ b/runtime/v2/runc/util.go @@ -19,8 +19,15 @@ package runc import ( + "context" + "encoding/json" + "io/ioutil" + "path/filepath" + "github.com/containerd/containerd/api/events" + "github.com/containerd/containerd/log" "github.com/containerd/containerd/runtime" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" ) @@ -53,3 +60,26 @@ func GetTopic(e interface{}) string { } return runtime.TaskUnknownTopic } + +// ShouldKillAllOnExit reads the bundle's OCI spec and returns true if +// there is an error reading the spec or if the container has a private PID namespace +func ShouldKillAllOnExit(ctx context.Context, bundlePath string) bool { + var bundleSpec specs.Spec + bundleConfigContents, err := ioutil.ReadFile(filepath.Join(bundlePath, "config.json")) + if err != nil { + log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to read config.json") + return true + } + if err := json.Unmarshal(bundleConfigContents, &bundleSpec); err != nil { + log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to unmarshal bundle json") + return true + } + if bundleSpec.Linux != nil { + for _, ns := range bundleSpec.Linux.Namespaces { + if ns.Type == specs.PIDNamespace && ns.Path == "" { + return false + } + } + } + return true +} diff --git a/runtime/v2/runc/v1/service.go b/runtime/v2/runc/v1/service.go index d6c537768..558a08dbe 100644 --- a/runtime/v2/runc/v1/service.go +++ b/runtime/v2/runc/v1/service.go @@ -20,7 +20,6 @@ package v1 import ( "context" - "encoding/json" "io/ioutil" "os" "os/exec" @@ -33,7 +32,6 @@ import ( eventstypes "github.com/containerd/containerd/api/events" "github.com/containerd/containerd/api/types/task" "github.com/containerd/containerd/errdefs" - "github.com/containerd/containerd/log" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/pkg/oom" @@ -49,7 +47,6 @@ import ( "github.com/gogo/protobuf/proto" "github.com/gogo/protobuf/types" ptypes "github.com/gogo/protobuf/types" - specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" @@ -595,14 +592,9 @@ func (s *service) checkProcesses(e runcC.Exit) { return } - shouldKillAll, err := shouldKillAllOnExit(container.Bundle) - if err != nil { - log.G(s.context).WithError(err).Error("failed to check shouldKillAll") - } - for _, p := range container.All() { if p.Pid() == e.Pid { - if shouldKillAll { + if runc.ShouldKillAllOnExit(s.context, container.Bundle) { if ip, ok := p.(*process.Init); ok { // Ensure all children are killed if err := ip.KillAll(s.context); err != nil { @@ -624,25 +616,6 @@ func (s *service) checkProcesses(e runcC.Exit) { } } -func shouldKillAllOnExit(bundlePath string) (bool, error) { - var bundleSpec specs.Spec - bundleConfigContents, err := ioutil.ReadFile(filepath.Join(bundlePath, "config.json")) - if err != nil { - return false, err - } - json.Unmarshal(bundleConfigContents, &bundleSpec) - - if bundleSpec.Linux != nil { - for _, ns := range bundleSpec.Linux.Namespaces { - if ns.Type == specs.PIDNamespace && ns.Path == "" { - return false, nil - } - } - } - - return true, nil -} - func (s *service) getContainerPids(ctx context.Context, id string) ([]uint32, error) { p, err := s.container.Process("") if err != nil { diff --git a/runtime/v2/runc/v2/service.go b/runtime/v2/runc/v2/service.go index 80fbd7c7f..03a0db5cc 100644 --- a/runtime/v2/runc/v2/service.go +++ b/runtime/v2/runc/v2/service.go @@ -35,7 +35,6 @@ import ( eventstypes "github.com/containerd/containerd/api/events" "github.com/containerd/containerd/api/types/task" "github.com/containerd/containerd/errdefs" - "github.com/containerd/containerd/log" "github.com/containerd/containerd/mount" "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/pkg/oom" @@ -51,7 +50,6 @@ import ( "github.com/gogo/protobuf/proto" "github.com/gogo/protobuf/types" ptypes "github.com/gogo/protobuf/types" - specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" @@ -701,13 +699,8 @@ func (s *service) checkProcesses(e runcC.Exit) { } if ip, ok := p.(*process.Init); ok { - shouldKillAll, err := shouldKillAllOnExit(container.Bundle) - if err != nil { - log.G(s.context).WithError(err).Error("failed to check shouldKillAll") - } - // Ensure all children are killed - if shouldKillAll { + if runc.ShouldKillAllOnExit(s.context, container.Bundle) { if err := ip.KillAll(s.context); err != nil { logrus.WithError(err).WithField("id", ip.ID()). Error("failed to kill init's children") @@ -729,25 +722,6 @@ func (s *service) checkProcesses(e runcC.Exit) { } } -func shouldKillAllOnExit(bundlePath string) (bool, error) { - var bundleSpec specs.Spec - bundleConfigContents, err := ioutil.ReadFile(filepath.Join(bundlePath, "config.json")) - if err != nil { - return false, err - } - json.Unmarshal(bundleConfigContents, &bundleSpec) - - if bundleSpec.Linux != nil { - for _, ns := range bundleSpec.Linux.Namespaces { - if ns.Type == specs.PIDNamespace && ns.Path == "" { - return false, nil - } - } - } - - return true, nil -} - func (s *service) getContainerPids(ctx context.Context, id string) ([]uint32, error) { container, err := s.getContainer(id) if err != nil {