Don't delete container with task

Make sure we don't delete a container with a live task

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

checkpoint_test.go

@@ -163,11 +163,11 @@ func TestCheckpointRestoreNewContainer(t *testing.T) {
 
 	<-statusC
 
-	if err := container.Delete(ctx); err != nil {
+	if _, err := task.Delete(ctx); err != nil {
 		t.Error(err)
 		return
 	}
-	if _, err := task.Delete(ctx); err != nil {
+	if err := container.Delete(ctx); err != nil {
 		t.Error(err)
 		return
 	}

container.go

@@ -17,8 +17,9 @@ import (
 )
 
 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")
 )
 
 type Container interface {
@@ -45,7 +46,6 @@ type container struct {
 
 	client *Client
 	c      containers.Container
-	task   *task
 }
 
 // ID returns the container's unique id
@@ -69,6 +69,9 @@ func (c *container) Spec() (*specs.Spec, error) {
 // Delete deletes an existing container
 // an error is returned if the container has running tasks
 func (c *container) Delete(ctx context.Context) (err error) {
+	if _, err := c.Task(ctx, nil); err == nil {
+		return ErrDeleteRunningTask
+	}
 	// TODO: should the client be the one removing resources attached
 	// to the container at the moment before we have GC?
 	if c.c.RootFS != "" {
@@ -83,16 +86,7 @@ func (c *container) Delete(ctx context.Context) (err error) {
 }
 
 func (c *container) Task(ctx context.Context, attach IOAttach) (Task, error) {
-	c.mu.Lock()
+	return c.loadTask(ctx, attach)
-	defer c.mu.Unlock()
-	if c.task == nil {
-		t, err := c.loadTask(ctx, attach)
-		if err != nil {
-			return nil, err
-		}
-		c.task = t.(*task)
-	}
-	return c.task, nil
 }
 
 // Image returns the image that the container is based on
@@ -163,7 +157,6 @@ func (c *container) NewTask(ctx context.Context, ioCreate IOCreation, opts ...Ne
 		t.pid = response.Pid
 		close(t.pidSync)
 	}
-	c.task = t
 	return t, nil
 }
 
@@ -206,7 +199,6 @@ func (c *container) loadTask(ctx context.Context, ioAttach IOAttach) (Task, erro
 		pid:         response.Task.Pid,
 		pidSync:     ps,
 	}
-	c.task = t
 	return t, nil
 }
 

container_test.go

@@ -597,3 +597,71 @@ func TestContainerAttach(t *testing.T) {
 		t.Errorf("expected output %q but received %q", expected, output)
 	}
 }
+
+func TestDeleteRunningContainer(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("sleep", "100"))
+	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, empty())
+	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
+	}
+
+	err = container.Delete(ctx)
+	if err == nil {
+		t.Error("delete did not error with running task")
+	}
+	if err != ErrDeleteRunningTask {
+		t.Errorf("expected error %q but received %q", ErrDeleteRunningTask, err)
+	}
+	if err := task.Kill(ctx, syscall.SIGKILL); err != nil {
+		t.Error(err)
+		return
+	}
+	<-statusC
+}