Fix error on doulbe Kill calls

This returns a typed error for calls to Kill when the process has
already finished.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
This commit is contained in:
Michael Crosby 2017-06-14 16:11:51 -07:00
parent 8d9ccd646b
commit 3b9d9dfa3e
8 changed files with 112 additions and 18 deletions

View File

@ -6,7 +6,7 @@ import (
"text/tabwriter" "text/tabwriter"
"github.com/containerd/containerd" "github.com/containerd/containerd"
"github.com/containerd/containerd/api/services/execution" tasks "github.com/containerd/containerd/api/services/tasks/v1"
tasktypes "github.com/containerd/containerd/api/types/task" tasktypes "github.com/containerd/containerd/api/types/task"
"github.com/urfave/cli" "github.com/urfave/cli"
) )
@ -67,9 +67,9 @@ func taskListFn(context *cli.Context) error {
return err return err
} }
tasks := client.TaskService() s := client.TaskService()
tasksResponse, err := tasks.List(ctx, &execution.ListRequest{}) tasksResponse, err := s.List(ctx, &tasks.ListTasksRequest{})
if err != nil { if err != nil {
return err return err
} }

View File

@ -20,6 +20,7 @@ var (
ErrNoImage = errors.New("container does not have an image") ErrNoImage = errors.New("container does not have an image")
ErrNoRunningTask = errors.New("no running task") ErrNoRunningTask = errors.New("no running task")
ErrDeleteRunningTask = errors.New("cannot delete container with running task") ErrDeleteRunningTask = errors.New("cannot delete container with running task")
ErrProcessExited = errors.New("process already exited")
) )
type Container interface { type Container interface {

View File

@ -665,3 +665,72 @@ func TestDeleteRunningContainer(t *testing.T) {
} }
<-statusC <-statusC
} }
func TestContainerKill(t *testing.T) {
if testing.Short() {
t.Skip()
}
client, err := New(address)
if err != nil {
t.Fatal(err)
}
defer client.Close()
var (
ctx, cancel = testContext()
id = t.Name()
)
defer cancel()
image, err := client.GetImage(ctx, testImage)
if err != nil {
t.Error(err)
return
}
spec, err := GenerateSpec(WithImageConfig(ctx, image), WithProcessArgs("sh", "-c", "cat"))
if err != nil {
t.Error(err)
return
}
container, err := client.NewContainer(ctx, id, WithSpec(spec), WithImage(image), WithNewRootFS(id, image))
if err != nil {
t.Error(err)
return
}
defer container.Delete(ctx)
task, err := container.NewTask(ctx, Stdio)
if err != nil {
t.Error(err)
return
}
defer task.Delete(ctx)
statusC := make(chan uint32, 1)
go func() {
status, err := task.Wait(ctx)
if err != nil {
t.Error(err)
}
statusC <- status
}()
if err := task.Start(ctx); err != nil {
t.Error(err)
return
}
if err := task.Kill(ctx, syscall.SIGKILL); err != nil {
t.Error(err)
return
}
<-statusC
err = task.Kill(ctx, syscall.SIGTERM)
if err == nil {
t.Error("second call to kill should return an error")
return
}
if err != ErrProcessExited {
t.Errorf("expected error %q but received %q", ErrProcessExited, err)
}
}

View File

@ -167,7 +167,10 @@ func (e *execProcess) Resize(ws console.WinSize) error {
} }
func (e *execProcess) Signal(sig int) error { func (e *execProcess) Signal(sig int) error {
return unix.Kill(e.pid, syscall.Signal(sig)) if err := unix.Kill(e.pid, syscall.Signal(sig)); err != nil {
return checkKillError(err)
}
return nil
} }
func (e *execProcess) Stdin() io.Closer { func (e *execProcess) Stdin() io.Closer {

View File

@ -20,6 +20,7 @@ import (
shimapi "github.com/containerd/containerd/api/services/shim/v1" shimapi "github.com/containerd/containerd/api/services/shim/v1"
"github.com/containerd/containerd/log" "github.com/containerd/containerd/log"
"github.com/containerd/containerd/mount" "github.com/containerd/containerd/mount"
"github.com/containerd/containerd/plugin"
"github.com/containerd/fifo" "github.com/containerd/fifo"
runc "github.com/containerd/go-runc" runc "github.com/containerd/go-runc"
"github.com/pkg/errors" "github.com/pkg/errors"
@ -234,7 +235,7 @@ func (p *initProcess) Kill(context context.Context, signal uint32, all bool) err
err := p.runc.Kill(context, p.id, int(signal), &runc.KillOpts{ err := p.runc.Kill(context, p.id, int(signal), &runc.KillOpts{
All: all, All: all,
}) })
return p.runcError(err, "runc kill failed") return checkKillError(err)
} }
func (p *initProcess) killAll(context context.Context) error { func (p *initProcess) killAll(context context.Context) error {
@ -245,7 +246,7 @@ func (p *initProcess) killAll(context context.Context) error {
} }
func (p *initProcess) Signal(sig int) error { func (p *initProcess) Signal(sig int) error {
return unix.Kill(p.pid, syscall.Signal(sig)) return checkKillError(unix.Kill(p.pid, syscall.Signal(sig)))
} }
func (p *initProcess) Stdin() io.Closer { func (p *initProcess) Stdin() io.Closer {
@ -352,3 +353,13 @@ func copyFile(to, from string) error {
_, err = io.Copy(tt, ff) _, err = io.Copy(tt, ff)
return err return err
} }
func checkKillError(err error) error {
if err == nil {
return nil
}
if strings.Contains(err.Error(), "os: process already finished") || err == unix.ESRCH {
return plugin.ErrProcessExited
}
return err
}

View File

@ -274,12 +274,16 @@ func (s *Service) Kill(ctx context.Context, r *shimapi.KillRequest) (*google_pro
} }
return empty, nil return empty, nil
} }
if int(r.Pid) == s.initProcess.pid {
if err := s.initProcess.Kill(ctx, r.Signal, r.All); err != nil {
return nil, err
}
return empty, nil
}
pids, err := s.getContainerPids(ctx, s.initProcess.id) pids, err := s.getContainerPids(ctx, s.initProcess.id)
if err != nil { if err != nil {
return nil, err return nil, err
} }
valid := false valid := false
for _, p := range pids { for _, p := range pids {
if r.Pid == p { if r.Pid == p {
@ -287,15 +291,12 @@ func (s *Service) Kill(ctx context.Context, r *shimapi.KillRequest) (*google_pro
break break
} }
} }
if !valid { if !valid {
return nil, errors.Errorf("process %d does not exist in container", r.Pid) return nil, errors.Errorf("process %d does not exist in container", r.Pid)
} }
if err := unix.Kill(int(r.Pid), syscall.Signal(r.Signal)); err != nil { if err := unix.Kill(int(r.Pid), syscall.Signal(r.Signal)); err != nil {
return nil, err return nil, checkKillError(err)
} }
return empty, nil return empty, nil
} }

View File

@ -3,7 +3,8 @@ package plugin
import "errors" import "errors"
var ( var (
ErrContainerExists = errors.New("container with id already exists") ErrContainerExists = errors.New("runtime: container with id already exists")
ErrContainerNotExist = errors.New("container does not exist") ErrContainerNotExist = errors.New("runtime: container does not exist")
ErrRuntimeNotExist = errors.New("runtime does not exist") ErrRuntimeNotExist = errors.New("runtime: runtime does not exist")
ErrProcessExited = errors.New("runtime: process already exited")
) )

12
task.go
View File

@ -16,9 +16,11 @@ import (
tasktypes "github.com/containerd/containerd/api/types/task" tasktypes "github.com/containerd/containerd/api/types/task"
"github.com/containerd/containerd/content" "github.com/containerd/containerd/content"
"github.com/containerd/containerd/events" "github.com/containerd/containerd/events"
"github.com/containerd/containerd/plugin"
"github.com/containerd/containerd/rootfs" "github.com/containerd/containerd/rootfs"
"github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/image-spec/specs-go/v1"
specs "github.com/opencontainers/runtime-spec/specs-go" specs "github.com/opencontainers/runtime-spec/specs-go"
"google.golang.org/grpc"
) )
const UnknownExitStatus = 255 const UnknownExitStatus = 255
@ -108,11 +110,17 @@ func (t *task) Kill(ctx context.Context, s syscall.Signal) error {
_, err := t.client.TaskService().Kill(ctx, &tasks.KillRequest{ _, err := t.client.TaskService().Kill(ctx, &tasks.KillRequest{
Signal: uint32(s), Signal: uint32(s),
ContainerID: t.containerID, ContainerID: t.containerID,
PidOrAll: &tasks.KillRequest_All{ PidOrAll: &tasks.KillRequest_Pid{
All: true, Pid: t.pid,
}, },
}) })
if err != nil {
if strings.Contains(grpc.ErrorDesc(err), plugin.ErrProcessExited.Error()) {
return ErrProcessExited
}
return err return err
}
return nil
} }
func (t *task) Pause(ctx context.Context) error { func (t *task) Pause(ctx context.Context) error {