From a3f933d9981929ed074b7c07b231e884a7923c97 Mon Sep 17 00:00:00 2001
From: Sargun Dhillon <sargun@sargun.me>
Date: Fri, 17 May 2019 10:53:24 -0700
Subject: [PATCH] Fix permission denied warnings during tests (#617)

For some reason the kubelet tries to do things that it gets perm
denied. This changes the base.yml in the skaffold file to have
the permissions. In turn, this broke the node deletion test. So,
instead of looking for the deletion event (imperative), we wait
for the node UID to change (declarative).
---
 hack/skaffold/virtual-kubelet/base.yml |  4 ++
 test/e2e/framework/node.go             | 22 ++++++-----
 test/e2e/node_test.go                  | 54 ++++++++++++++++----------
 3 files changed, 49 insertions(+), 31 deletions(-)

diff --git a/hack/skaffold/virtual-kubelet/base.yml b/hack/skaffold/virtual-kubelet/base.yml
index a69970a63..0cbfc14c2 100644
--- a/hack/skaffold/virtual-kubelet/base.yml
+++ b/hack/skaffold/virtual-kubelet/base.yml
@@ -13,6 +13,7 @@ rules:
   resources:
   - configmaps
   - secrets
+  - services
   verbs:
   - get
   - list
@@ -26,6 +27,7 @@ rules:
   - get
   - list
   - watch
+  - patch
 - apiGroups:
   - ""
   resources:
@@ -39,12 +41,14 @@ rules:
   - nodes/status
   verbs:
   - update
+  - patch
 - apiGroups:
   - ""
   resources:
   - pods/status
   verbs:
   - update
+  - patch
 - apiGroups:
   - ""
   resources:
diff --git a/test/e2e/framework/node.go b/test/e2e/framework/node.go
index d0ec060fd..d1fa4892e 100644
--- a/test/e2e/framework/node.go
+++ b/test/e2e/framework/node.go
@@ -43,16 +43,18 @@ func (f *Framework) WaitUntilNodeCondition(fn watch.ConditionFunc) error {
 	return nil
 }
 
-// WaitUntilNodeAdded is a watch condition which waits until the VK node object
-// is added.
-func (f *Framework) WaitUntilNodeAdded(event watchapi.Event) (bool, error) {
-	if event.Type != watchapi.Added {
-		return false, nil
-	}
-	return event.Object.(*corev1.Node).Name == f.NodeName, nil
-}
-
 // DeleteNode deletes the vk node used by the framework
 func (f *Framework) DeleteNode() error {
-	return f.KubeClient.CoreV1().Nodes().Delete(f.NodeName, nil)
+	var gracePeriod int64
+	propagation := metav1.DeletePropagationBackground
+	opts := metav1.DeleteOptions{
+		PropagationPolicy:  &propagation,
+		GracePeriodSeconds: &gracePeriod,
+	}
+	return f.KubeClient.CoreV1().Nodes().Delete(f.NodeName, &opts)
+}
+
+// GetNode gets the vk nodeused by the framework
+func (f *Framework) GetNode() (*corev1.Node, error) {
+	return f.KubeClient.CoreV1().Nodes().Get(f.NodeName, metav1.GetOptions{})
 }
diff --git a/test/e2e/node_test.go b/test/e2e/node_test.go
index 21358555d..ede1810c8 100644
--- a/test/e2e/node_test.go
+++ b/test/e2e/node_test.go
@@ -3,49 +3,61 @@
 package e2e
 
 import (
+	"context"
 	"testing"
 	"time"
 
 	"gotest.tools/assert"
+	is "gotest.tools/assert/cmp"
+	"k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
 	watchapi "k8s.io/apimachinery/pkg/watch"
 )
 
 // TestNodeCreateAfterDelete makes sure that a node is automatically recreated
 // if it is deleted while VK is running.
 func TestNodeCreateAfterDelete(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	podList, err := f.KubeClient.CoreV1().Pods(f.Namespace).List(metav1.ListOptions{
+		FieldSelector: fields.OneTermEqualSelector("spec.nodeName", f.NodeName).String(),
+	})
+
+	assert.NilError(t, err)
+	assert.Assert(t, is.Len(podList.Items, 0), "Kubernetes does not allow node deletion with dependent objects (pods) in existence: %v", podList.Items)
+
 	chErr := make(chan error, 1)
-	chDone := make(chan struct{})
-	defer close(chDone)
+
+	originalNode, err := f.GetNode()
+	assert.NilError(t, err)
+
+	ctx, cancel = context.WithTimeout(ctx, time.Minute)
+	defer cancel()
 
 	go func() {
-		var deleted bool
 		wait := func(e watchapi.Event) (bool, error) {
-			select {
-			case <-chDone:
-				return true, nil
-			default:
+			err = ctx.Err()
+			// Our timeout has expired
+			if err != nil {
+				return true, err
+			}
+			if e.Type == watchapi.Deleted || e.Type == watchapi.Error {
+				return false, nil
 			}
 
-			if deleted {
-				return f.WaitUntilNodeAdded(e)
-			}
-			if e.Type == watchapi.Deleted {
-				deleted = true
-			}
-			return false, nil
+			return originalNode.ObjectMeta.UID != e.Object.(*v1.Node).ObjectMeta.UID, nil
 		}
 		chErr <- f.WaitUntilNodeCondition(wait)
 	}()
 
 	assert.NilError(t, f.DeleteNode())
 
-	timer := time.NewTimer(60 * time.Second)
-	defer timer.Stop()
-
 	select {
-	case <-timer.C:
-		t.Fatal("timeout waiting for node to be recreated")
-	case err := <-chErr:
-		assert.NilError(t, err)
+	case result := <-chErr:
+		assert.NilError(t, result, "Did not observe new node object created after deletion")
+	case <-ctx.Done():
+		t.Fatal("Test timed out while waiting for node object to be deleted / recreated")
 	}
 }