Fix error handling for delete pod (#685)

* Fix error handling for delete pod

- Error handling was looking for a k8s error from the provider, but
  providers should be using errdefs.
- Error handling was returning early if pod was not found and deleting
  from k8s in all other cases.

* Don't run unit tests twice

node/pod_test.go

@@ -19,11 +19,14 @@ import (
 	"path"
 	"testing"
 
+	pkgerrors "github.com/pkg/errors"
 	"github.com/virtual-kubelet/virtual-kubelet/errdefs"
 	testutil "github.com/virtual-kubelet/virtual-kubelet/internal/test/util"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
 )
 
@@ -33,6 +36,8 @@ type mockProvider struct {
 	creates int
 	updates int
 	deletes int
+
+	errorOnDelete error
 }
 
 func (m *mockProvider) CreatePod(ctx context.Context, pod *corev1.Pod) error {
@@ -64,6 +69,9 @@ func (m *mockProvider) GetPodStatus(ctx context.Context, namespace, name string)
 }
 
 func (m *mockProvider) DeletePod(ctx context.Context, p *corev1.Pod) error {
+	if m.errorOnDelete != nil {
+		return m.errorOnDelete
+	}
 	delete(m.pods, path.Join(p.GetNamespace(), p.GetName()))
 	m.deletes++
 	return nil
@@ -293,3 +301,63 @@ func TestPodNoSpecChange(t *testing.T) {
 	assert.Check(t, is.Equal(svr.mock.creates, 1))
 	assert.Check(t, is.Equal(svr.mock.updates, 0))
 }
+
+func TestPodDelete(t *testing.T) {
+	type testCase struct {
+		desc   string
+		delErr error
+	}
+
+	cases := []testCase{
+		{desc: "no error on delete", delErr: nil},
+		{desc: "not found error on delete", delErr: errdefs.NotFound("not found")},
+		{desc: "unknown error on delete", delErr: pkgerrors.New("random error")},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			c := newTestController()
+			c.mock.errorOnDelete = tc.delErr
+
+			pod := &corev1.Pod{}
+			pod.ObjectMeta.Namespace = "default"
+			pod.ObjectMeta.Name = "nginx"
+			pod.Spec = corev1.PodSpec{
+				Containers: []corev1.Container{
+					corev1.Container{
+						Name:  "nginx",
+						Image: "nginx:1.15.12",
+					},
+				},
+			}
+
+			pc := c.client.CoreV1().Pods("default")
+
+			p, err := pc.Create(pod)
+			assert.NilError(t, err)
+
+			ctx := context.Background()
+			err = c.createOrUpdatePod(ctx, p) // make sure it's actually created
+			assert.NilError(t, err)
+			assert.Check(t, is.Equal(c.mock.creates, 1))
+
+			err = c.deletePod(ctx, pod.Namespace, pod.Name)
+			assert.Equal(t, pkgerrors.Cause(err), err)
+
+			var expectDeletes int
+			if tc.delErr == nil {
+				expectDeletes = 1
+			}
+			assert.Check(t, is.Equal(c.mock.deletes, expectDeletes))
+
+			expectDeleted := tc.delErr == nil || errdefs.IsNotFound(tc.delErr)
+
+			_, err = pc.Get(pod.Name, metav1.GetOptions{})
+			if expectDeleted {
+				assert.Assert(t, errors.IsNotFound(err))
+			} else {
+				assert.NilError(t, err)
+			}
+		})
+	}
+}