Delete shim workloads tasks in pod.

cmd/containerd-shim-runhcs-v1/pod.go

@@ -51,6 +51,14 @@ type shimPod interface {
 	// the `shimExecStateRunning, shimExecStateExited` states. If the exec is
 	// not in this state this pod MUST return `errdefs.ErrFailedPrecondition`.
 	KillTask(ctx context.Context, tid, eid string, signal uint32, all bool) error
+	// DeleteTask removes a task from being tracked by this pod, and cleans up
+	// the resources the shim allocated for the task.
+	//
+	// The task's init exec (eid == "") must be either `shimExecStateCreated` or
+	// `shimExecStateExited`.  If the exec is not in this state this pod MUST
+	// return `errdefs.ErrFailedPrecondition`. Deleting the pod's sandbox task
+	// is a no-op.
+	DeleteTask(ctx context.Context, tid string) error
 }
 
 func createPod(ctx context.Context, events publisher, req *task.CreateTaskRequest, s *specs.Spec) (shimPod, error) {
@@ -390,3 +398,30 @@ func (p *pod) KillTask(ctx context.Context, tid, eid string, signal uint32, all
 	})
 	return eg.Wait()
 }
+
+func (p *pod) DeleteTask(ctx context.Context, tid string) error {
+	// Deleting the sandbox task is a no-op, since the service should delete its
+	// reference to the sandbox task or pod, and `p.sandboxTask != nil` is an
+	// invariant that is relied on elsewhere.
+	// However, still get the init exec for all tasks to ensure that they have
+	// been properly stopped.
+
+	t, err := p.GetTask(tid)
+	if err != nil {
+		return errors.Wrap(err, "could not find task to delete")
+	}
+
+	e, err := t.GetExec("")
+	if err != nil {
+		return errors.Wrap(err, "could not get initial exec")
+	}
+	if e.State() == shimExecStateRunning {
+		return errors.Wrap(errdefs.ErrFailedPrecondition, "cannot delete task with running exec")
+	}
+
+	if p.id != tid {
+		p.workloadTasks.Delete(tid)
+	}
+
+	return nil
+}

cmd/containerd-shim-runhcs-v1/pod_test.go

@@ -44,6 +44,28 @@ func (tsp *testShimPod) KillTask(ctx context.Context, tid, eid string, signal ui
 	return s.KillExec(ctx, eid, signal, all)
 }
 
+func (tsp *testShimPod) DeleteTask(ctx context.Context, tid string) error {
+	t, err := tsp.GetTask(tid)
+	if err != nil {
+		return err
+	}
+
+	e, err := t.GetExec("")
+	if err != nil {
+		return err
+	}
+	switch e.State() {
+	case shimExecStateRunning:
+		return errdefs.ErrFailedPrecondition
+	default:
+	}
+
+	if tid != tsp.ID() {
+		tsp.tasks.Delete(tid)
+	}
+	return nil
+}
+
 // Pod tests
 
 func setupTestPodWithFakes(t *testing.T) (*pod, *testShimTask) {
@@ -111,6 +133,8 @@ func Test_pod_GetTask_WorkloadID_Created_Success(t *testing.T) {
 	}
 }
 
+// kill tests
+
 func Test_pod_KillTask_UnknownTaskID_Error(t *testing.T) {
 	p, _ := setupTestPodWithFakes(t)
 	err := p.KillTask(context.TODO(), "thisshouldnotmatch", "", 0xf, false)
@@ -205,3 +229,140 @@ func Test_pod_KillTask_WorkloadID_2ndExecID_All_Error(t *testing.T) {
 		verifyExpectedError(t, nil, err, errdefs.ErrFailedPrecondition)
 	}
 }
+
+// delete tests
+
+func Test_pod_DeleteTask_SandboxID(t *testing.T) {
+	p, st := setupTestPodWithFakes(t)
+
+	err := p.KillTask(context.Background(), st.ID(), "", 0xf, true)
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	err = p.DeleteTask(context.Background(), st.ID())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	// it should not be possible to delete the sandbox task
+	_, err = p.GetTask(t.Name())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+}
+
+func Test_pod_DeleteTask_SandboxID_Running(t *testing.T) {
+	p, st := setupTestPodWithFakes(t)
+
+	// start the task
+	e, err := st.GetExec("")
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+	err = e.Start(context.Background())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	err = p.DeleteTask(context.Background(), st.ID())
+	verifyExpectedError(t, nil, err, errdefs.ErrFailedPrecondition)
+}
+
+func Test_pod_DeleteTask_SandboxID_Repeated(t *testing.T) {
+	p, st := setupTestPodWithFakes(t)
+
+	err := p.KillTask(context.Background(), st.ID(), "", 0xf, true)
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	err = p.DeleteTask(context.Background(), st.ID())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	err = p.DeleteTask(context.Background(), st.ID())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+}
+
+func Test_pod_DeleteTask_TaskID(t *testing.T) {
+	p, _ := setupTestPodWithFakes(t)
+	st := setupTestTaskInPod(t, p)
+
+	err := p.KillTask(context.Background(), st.ID(), "", 0xf, true)
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	err = p.DeleteTask(context.Background(), st.ID())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	_, err = p.GetTask(st.ID())
+	verifyExpectedError(t, nil, err, errdefs.ErrNotFound)
+}
+
+func Test_pod_DeleteTask_TaskID_Running(t *testing.T) {
+	p, _ := setupTestPodWithFakes(t)
+	st := setupTestTaskInPod(t, p)
+
+	// start the task
+	e, err := st.GetExec("")
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+	err = e.Start(context.Background())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	err = p.DeleteTask(context.Background(), st.ID())
+	verifyExpectedError(t, nil, err, errdefs.ErrFailedPrecondition)
+
+	// should not actually delete the sandbox task
+	_, err = p.GetTask(t.Name())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	stp, err := p.GetTask(st.ID())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	if stp != st {
+		t.Fatalf("task should not have changed: %v != %v", st, stp)
+	}
+}
+
+func Test_pod_DeleteTask_TaskID_Repeated(t *testing.T) {
+	p, _ := setupTestPodWithFakes(t)
+	st := setupTestTaskInPod(t, p)
+
+	err := p.KillTask(context.Background(), st.ID(), "", 0xf, true)
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	err = p.DeleteTask(context.Background(), st.ID())
+	if err != nil {
+		t.Fatalf("should not have failed, got: %v", err)
+	}
+
+	err = p.DeleteTask(context.Background(), st.ID())
+	verifyExpectedError(t, nil, err, errdefs.ErrNotFound)
+}
+
+func Test_pod_DeleteTask_TaskID_Not_Created(t *testing.T) {
+	p, _ := setupTestPodWithFakes(t)
+	// Add two workload tasks
+	setupTestTaskInPod(t, p)
+	setupTestTaskInPod(t, p)
+
+	err := p.KillTask(context.Background(), strconv.Itoa(rand.Int()), "", 0xf, true)
+	verifyExpectedError(t, nil, err, errdefs.ErrNotFound)
+}

cmd/containerd-shim-runhcs-v1/service_internal.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -205,17 +206,29 @@ func (s *service) startInternal(ctx context.Context, req *task.StartRequest) (*t
 }
 
 func (s *service) deleteInternal(ctx context.Context, req *task.DeleteRequest) (*task.DeleteResponse, error) {
-	// TODO: JTERRY75 we need to send this to the POD for isSandbox
-
 	t, err := s.getTask(req.ID)
 	if err != nil {
 		return nil, err
 	}
+
 	pid, exitStatus, exitedAt, err := t.DeleteExec(ctx, req.ExecID)
 	if err != nil {
 		return nil, err
 	}
-	// TODO: We should be removing the task after this right?
+
+	// if the delete is for a task and not an exec, remove the pod sandbox's reference to the task
+	if s.isSandbox && req.ExecID == "" {
+		p, err := s.getPod()
+		if err != nil {
+			return nil, errors.Wrapf(err, "could not get pod %q to delete task %q", s.tid, req.ID)
+		}
+		err = p.DeleteTask(ctx, req.ID)
+		if err != nil {
+			return nil, fmt.Errorf("could not delete task %q in pod %q: %w", req.ID, s.tid, err)
+		}
+	}
+	// TODO: check if the pod's workload tasks is empty, and, if so, reset p.taskOrPod to nil
+
 	return &task.DeleteResponse{
 		Pid:        uint32(pid),
 		ExitStatus: exitStatus,