Merge pull request #6228 from puppetlabs:fix-shim-pid-leaking-on-stopped-processes
PiperOrigin-RevId: 381341920
This commit is contained in:
commit
fdf7c49030
|
@ -8,6 +8,7 @@ go_library(
|
|||
"api.go",
|
||||
"debug.go",
|
||||
"epoll.go",
|
||||
"errors.go",
|
||||
"options.go",
|
||||
"service.go",
|
||||
"service_linux.go",
|
||||
|
@ -44,6 +45,8 @@ go_library(
|
|||
"@com_github_gogo_protobuf//types:go_default_library",
|
||||
"@com_github_opencontainers_runtime_spec//specs-go:go_default_library",
|
||||
"@com_github_sirupsen_logrus//:go_default_library",
|
||||
"@org_golang_google_grpc//codes:go_default_library",
|
||||
"@org_golang_google_grpc//status:go_default_library",
|
||||
"@org_golang_x_sys//unix:go_default_library",
|
||||
],
|
||||
)
|
||||
|
@ -51,10 +54,14 @@ go_library(
|
|||
go_test(
|
||||
name = "shim_test",
|
||||
size = "small",
|
||||
srcs = ["service_test.go"],
|
||||
srcs = [
|
||||
"errors_test.go",
|
||||
"service_test.go",
|
||||
],
|
||||
library = ":shim",
|
||||
deps = [
|
||||
"//pkg/shim/utils",
|
||||
"@com_github_containerd_containerd//errdefs:go_default_library",
|
||||
"@com_github_opencontainers_runtime_spec//specs-go:go_default_library",
|
||||
],
|
||||
)
|
||||
|
|
|
@ -0,0 +1,59 @@
|
|||
// Copyright 2021 The gVisor Authors.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// https://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package shim
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
|
||||
"github.com/containerd/containerd/errdefs"
|
||||
"google.golang.org/grpc/codes"
|
||||
"google.golang.org/grpc/status"
|
||||
)
|
||||
|
||||
// errToGRPC wraps containerd's ToGRPC error mapper which depends on
|
||||
// github.com/pkg/errors to work correctly. Once we upgrade to containerd v1.4,
|
||||
// this function can go away and we can use errdefs.ToGRPC directly instead.
|
||||
//
|
||||
// TODO(gvisor.dev/issue/6232): Remove after upgrading to containerd v1.4
|
||||
func errToGRPC(err error) error {
|
||||
if err == nil {
|
||||
return nil
|
||||
}
|
||||
if _, ok := status.FromError(err); ok {
|
||||
return err
|
||||
}
|
||||
|
||||
switch {
|
||||
case errors.Is(err, errdefs.ErrInvalidArgument):
|
||||
return status.Errorf(codes.InvalidArgument, err.Error())
|
||||
case errors.Is(err, errdefs.ErrNotFound):
|
||||
return status.Errorf(codes.NotFound, err.Error())
|
||||
case errors.Is(err, errdefs.ErrAlreadyExists):
|
||||
return status.Errorf(codes.AlreadyExists, err.Error())
|
||||
case errors.Is(err, errdefs.ErrFailedPrecondition):
|
||||
return status.Errorf(codes.FailedPrecondition, err.Error())
|
||||
case errors.Is(err, errdefs.ErrUnavailable):
|
||||
return status.Errorf(codes.Unavailable, err.Error())
|
||||
case errors.Is(err, errdefs.ErrNotImplemented):
|
||||
return status.Errorf(codes.Unimplemented, err.Error())
|
||||
case errors.Is(err, context.Canceled):
|
||||
return status.Errorf(codes.Canceled, err.Error())
|
||||
case errors.Is(err, context.DeadlineExceeded):
|
||||
return status.Errorf(codes.DeadlineExceeded, err.Error())
|
||||
}
|
||||
|
||||
return errdefs.ToGRPC(err)
|
||||
}
|
|
@ -0,0 +1,47 @@
|
|||
// Copyright 2021 The gVisor Authors.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// https://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package shim
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/containerd/containerd/errdefs"
|
||||
)
|
||||
|
||||
func TestGRPCRoundTripsErrors(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
err error
|
||||
test func(err error) bool
|
||||
}{
|
||||
{
|
||||
name: "passthrough",
|
||||
err: errdefs.ErrNotFound,
|
||||
test: errdefs.IsNotFound,
|
||||
},
|
||||
{
|
||||
name: "wrapped",
|
||||
err: fmt.Errorf("oh no: %w", errdefs.ErrNotFound),
|
||||
test: errdefs.IsNotFound,
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if err := errdefs.FromGRPC(errToGRPC(tc.err)); !tc.test(err) {
|
||||
t.Errorf("got %+v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
|
@ -151,8 +151,8 @@ func (s *execStoppedState) Delete(ctx context.Context) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
func (s *execStoppedState) Kill(ctx context.Context, sig uint32, all bool) error {
|
||||
return s.p.kill(ctx, sig, all)
|
||||
func (s *execStoppedState) Kill(_ context.Context, sig uint32, _ bool) error {
|
||||
return handleStoppedKill(sig)
|
||||
}
|
||||
|
||||
func (s *execStoppedState) SetExited(int) {
|
||||
|
|
|
@ -452,10 +452,10 @@ func (s *service) Create(ctx context.Context, r *taskAPI.CreateTaskRequest) (*ta
|
|||
}
|
||||
process, err := newInit(r.Bundle, filepath.Join(r.Bundle, "work"), ns, s.platform, config, &s.opts, st.Rootfs)
|
||||
if err != nil {
|
||||
return nil, errdefs.ToGRPC(err)
|
||||
return nil, errToGRPC(err)
|
||||
}
|
||||
if err := process.Create(ctx, config); err != nil {
|
||||
return nil, errdefs.ToGRPC(err)
|
||||
return nil, errToGRPC(err)
|
||||
}
|
||||
|
||||
// Set up OOM notification on the sandbox's cgroup. This is done on
|
||||
|
@ -544,7 +544,7 @@ func (s *service) Exec(ctx context.Context, r *taskAPI.ExecProcessRequest) (*typ
|
|||
Spec: r.Spec,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, errdefs.ToGRPC(err)
|
||||
return nil, errToGRPC(err)
|
||||
}
|
||||
s.mu.Lock()
|
||||
s.processes[r.ExecID] = process
|
||||
|
@ -565,7 +565,7 @@ func (s *service) ResizePty(ctx context.Context, r *taskAPI.ResizePtyRequest) (*
|
|||
Height: uint16(r.Height),
|
||||
}
|
||||
if err := p.Resize(ws); err != nil {
|
||||
return nil, errdefs.ToGRPC(err)
|
||||
return nil, errToGRPC(err)
|
||||
}
|
||||
return empty, nil
|
||||
}
|
||||
|
@ -648,7 +648,7 @@ func (s *service) Kill(ctx context.Context, r *taskAPI.KillRequest) (*types.Empt
|
|||
}
|
||||
if err := p.Kill(ctx, r.Signal, r.All); err != nil {
|
||||
log.L.Debugf("Kill failed: %v", err)
|
||||
return nil, errdefs.ToGRPC(err)
|
||||
return nil, errToGRPC(err)
|
||||
}
|
||||
log.L.Debugf("Kill succeeded")
|
||||
return empty, nil
|
||||
|
@ -660,7 +660,7 @@ func (s *service) Pids(ctx context.Context, r *taskAPI.PidsRequest) (*taskAPI.Pi
|
|||
|
||||
pids, err := s.getContainerPids(ctx, r.ID)
|
||||
if err != nil {
|
||||
return nil, errdefs.ToGRPC(err)
|
||||
return nil, errToGRPC(err)
|
||||
}
|
||||
var processes []*task.ProcessInfo
|
||||
for _, pid := range pids {
|
||||
|
|
Loading…
Reference in New Issue