Merge pull request #4084 from crosbymichael/kill-all-check
[carry] Check error return from json.Unmarshal
This commit is contained in:
		| @@ -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) { | ||||
|   | ||||
| @@ -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 | ||||
| } | ||||
|   | ||||
| @@ -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 { | ||||
|   | ||||
| @@ -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 { | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Phil Estes
					Phil Estes