Improve object deletion. (#7)

Previously, we would delete tracked objects, but made no representations
about when the deletion would be completed. Update `DeleteAll` to keep
deleting everything until we receive API server confirmation that the
objects we are tracking are all gone.

This improves test isolation since we have better guarantees that
objects don't leak between runs.

Signed-off-by: James Peach <[email protected]>

pkg/driver/object.go

@@ -346,7 +346,14 @@ func (o *objectDriver) Delete(obj *unstructured.Unstructured) (*OperationResult,
 	}
 	o.objectLock.Unlock()
 
-	opts := utils.ImmediateDeletionOptions()
+	opts := utils.ImmediateDeletionOptions(metav1.DeletePropagationForeground)
+
+	// Services need to be deleted in the background, see
+	//	https://github.com/kubernetes/kubernetes/issues/87603
+	//	https://github.com/kubernetes/kubernetes/issues/90512
+	if obj.GetKind() == "Service" {
+		opts = utils.ImmediateDeletionOptions(metav1.DeletePropagationBackground)
+	}
 
 	if isNamespaced {
 		err = o.kube.Dynamic.Resource(gvr).Namespace(obj.GetNamespace()).Delete(obj.GetName(), opts)
@@ -405,38 +412,51 @@ func (o *objectDriver) Adopt(obj *unstructured.Unstructured) error {
 }
 
 func (o *objectDriver) DeleteAll() error {
-	targets := make([]*unstructured.Unstructured, 0, len(o.objectPool))
-	var errs []error
-
-	o.objectLock.Lock()
-	for _, u := range o.objectPool {
-		targets = append(targets, u.DeepCopy())
-	}
-	o.objectLock.Unlock()
+	for {
+		var errs []error
+		targets := make([]*unstructured.Unstructured, 0, len(o.objectPool))
 
-	for _, u := range targets {
-		result, err := o.Delete(u)
+		o.objectLock.Lock()
+		for _, u := range o.objectPool {
+			targets = append(targets, u.DeepCopy())
+		}
+		o.objectLock.Unlock()
 
-		if err != nil {
-			errs = append(errs, err)
-			continue
+		if len(targets) == 0 {
+			return nil
 		}
 
-		if result.Error != nil {
-			// Re-wrap the error that we unwrapped for status!
-			errs = append(errs, &apierrors.StatusError{
-				ErrStatus: *result.Error,
-			})
+		for _, u := range targets {
+			result, err := o.Delete(u)
 
-			continue
-		}
-	}
+			if err != nil {
+				errs = append(errs, err)
+				continue
+			}
 
-	if len(errs) == 0 {
-		return nil
-	}
+			if result.Error != nil {
+				switch result.Error.Reason {
+				case metav1.StatusReasonNotFound, metav1.StatusReasonGone:
+					// If the deletion failed because the target wasn't there, then the object
+					// pool won't get updated by the informer callback. We have to update it here.
+					o.objectLock.Lock()
+					delete(o.objectPool, u.GetUID())
+					o.objectLock.Unlock()
+				default:
+					// Re-wrap the error that we unwrapped for status!
+					errs = append(errs, &apierrors.StatusError{
+						ErrStatus: *result.Error,
+					})
+					continue
+				}
+			}
+		}
 
-	errs = append([]error{errors.New("failed to delete all objects")}, errs...)
+		if len(errs) != 0 {
+			errs = append([]error{errors.New("failed to delete all objects")}, errs...)
+			return utils.ChainErrors(errs...)
+		}
 
-	return utils.ChainErrors(errs...)
+		time.Sleep(time.Second)
+	}
 }

pkg/test/runner.go

@@ -195,6 +195,9 @@ func Run(testDoc *doc.Document, opts ...RunOpt) error {
 	tc.regoDriver.StoreItem("/test/params/run-id", tc.envDriver.UniqueID())
 
 	step(tc.recorder, "compiling test document", func() {
+		tc.recorder.Update(
+			result.Infof("test run ID is %s", tc.envDriver.UniqueID()))
+
 		compiler, err = compileDocument(testDoc, tc.policyModules)
 		if err != nil {
 			tc.recorder.Update(result.Fatalf("%s", err.Error()))
@@ -394,8 +397,14 @@ func Run(testDoc *doc.Document, opts ...RunOpt) error {
 		}
 	}
 
-	if !tc.preserve {
-		must.Must(tc.objectDriver.DeleteAll())
+	if tc.preserve {
+		step(tc.recorder, "preserving test objects", func() {})
+	} else {
+		step(tc.recorder, "deleting test objects", func() {
+			if err := tc.objectDriver.DeleteAll(); err != nil {
+				tc.recorder.Update(result.Fatalf("object deletion failed: %s", err))
+			}
+		})
 	}
 
 	// TODO(jpeach): return a structured test result object.

pkg/utils/kubernetes.go

@@ -26,12 +26,10 @@ import (
 
 // ImmediateDeletionOptions returns metav1.DeleteOptions specifying
 // that the caller requires immediate foreground deletion semantics.
-func ImmediateDeletionOptions() *metav1.DeleteOptions {
-	fg := metav1.DeletePropagationForeground
-
+func ImmediateDeletionOptions(propagation metav1.DeletionPropagation) *metav1.DeleteOptions {
 	return &metav1.DeleteOptions{
 		GracePeriodSeconds: pointer.Int64Ptr(0),
-		PropagationPolicy:  &fg,
+		PropagationPolicy:  &propagation,
 	}
 }