Use watch after deleting objects in tests (GoogleContainerTools#583)

Author: karlkfi

e2e/nomostest/clean.go

@@ -160,7 +160,7 @@ func deleteTestObjectsAndWait(nt *NT) error {
 		}
 	}
 	// Delete all objects in serial and wait for not found in parallel.
-	return deleteObjectsAndWait(nt, objs...)
+	return DeleteObjectsAndWait(nt, objs...)
 }
 
 // deleteManagedNamespacesAndWait deletes all the namespaces managed by Config Sync.
@@ -229,7 +229,7 @@ func deleteNamespacesAndWait(nt *NT, nsList []corev1.Namespace) error {
 	for i := range nsList {
 		objs = append(objs, &nsList[i])
 	}
-	return deleteObjectsAndWait(nt, objs...)
+	return DeleteObjectsAndWait(nt, objs...)
 }
 
 func filterMutableListTypes(nt *NT) map[schema.GroupVersionKind]reflect.Type {
@@ -395,7 +395,7 @@ func deleteKubevirt(nt *NT) error {
 	objs = append(objs, obj)
 
 	// Delete specified KubeVirt objects in parallel and wait for NotFound
-	if err := deleteObjectsAndWait(nt, objs...); err != nil {
+	if err := DeleteObjectsAndWait(nt, objs...); err != nil {
 		return err
 	}
 
@@ -490,9 +490,9 @@ func deleteObject(nt *NT, obj client.Object) error {
 	return nil
 }
 
-// deleteObjectsAndWait deletes zero or more objects in serial and waits for not found in parallel.
+// DeleteObjectsAndWait deletes zero or more objects in serial and waits for not found in parallel.
 // NOTE: Deleting in parallel might be faster, but the log would be harder to debug.
-func deleteObjectsAndWait(nt *NT, objs ...client.Object) error {
+func DeleteObjectsAndWait(nt *NT, objs ...client.Object) error {
 	tg := taskgroup.New()
 	for _, obj := range objs {
 		nn := client.ObjectKeyFromObject(obj)

e2e/nomostest/config_sync.go

@@ -30,7 +30,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
@@ -178,14 +177,7 @@ func uninstallConfigSync(nt *NT) error {
 	if err != nil {
 		return err
 	}
-	for _, o := range objs {
-		if err := nt.KubeClient.Delete(o); err != nil {
-			if !apierrors.IsNotFound(err) && !meta.IsNoMatchError(err) {
-				return err
-			}
-		}
-	}
-	return nil
+	return DeleteObjectsAndWait(nt, objs...)
 }
 
 func isPSPCluster() bool {

e2e/nomostest/reset.go

@@ -298,7 +298,7 @@ func ResetRepoSyncs(nt *NT, rsList []v1beta1.RepoSync) error {
 	if err != nil {
 		return err
 	}
-	if err := batchDeleteAndWait(nt, rbs...); err != nil {
+	if err := DeleteObjectsAndWait(nt, rbs...); err != nil {
 		return err
 	}
 
@@ -316,7 +316,7 @@ func ResetRepoSyncs(nt *NT, rsList []v1beta1.RepoSync) error {
 	if err != nil {
 		return err
 	}
-	if err := batchDeleteAndWait(nt, crbs...); err != nil {
+	if err := DeleteObjectsAndWait(nt, crbs...); err != nil {
 		return err
 	}
 
@@ -361,7 +361,7 @@ func ResetNamespaces(nt *NT, nsList []corev1.Namespace) error {
 // reconcilers, if it exists.
 func deleteRepoSyncClusterRole(nt *NT) error {
 	nt.T.Log("[RESET] Deleting RepoSync ClusterRole")
-	return batchDeleteAndWait(nt, nt.RepoSyncClusterRole())
+	return DeleteObjectsAndWait(nt, nt.RepoSyncClusterRole())
 }
 
 func findUnmanaged(nt *NT, objs ...client.Object) ([]client.Object, error) {
@@ -378,28 +378,6 @@ func findUnmanaged(nt *NT, objs ...client.Object) ([]client.Object, error) {
 	return unmanaged, nil
 }
 
-func batchDeleteAndWait(nt *NT, objs ...client.Object) error {
-	for _, obj := range objs {
-		if err := nt.KubeClient.Delete(obj); err != nil {
-			if !apierrors.IsNotFound(err) {
-				return err
-			}
-		}
-	}
-	tg := taskgroup.New()
-	for _, obj := range objs {
-		gvk, err := kinds.Lookup(obj, nt.Scheme)
-		if err != nil {
-			return err
-		}
-		nn := client.ObjectKeyFromObject(obj)
-		tg.Go(func() error {
-			return nt.Watcher.WatchForNotFound(gvk, nn.Name, nn.Namespace)
-		})
-	}
-	return tg.Wait()
-}
-
 func listRootSyncs(nt *NT, opts ...client.ListOption) (*v1beta1.RootSyncList, error) {
 	rsList := &v1beta1.RootSyncList{}
 	opts = append(opts, withLabelListOption(testkubeclient.TestLabel, testkubeclient.TestLabelValue))

e2e/nomostest/wait_for_sync.go

@@ -106,7 +106,7 @@ func syncDirectory(syncDirectoryMap map[types.NamespacedName]string, nn types.Na
 // WatchForAllSyncs calls WatchForSync on all Syncs in nt.RootRepos & nt.NonRootRepos.
 //
 // If you want to validate specific fields of a Sync object, use
-// nomostest.WatchObject() instead.
+// nt.Watcher.WatchObject() instead.
 func (nt *NT) WatchForAllSyncs(options ...WatchForAllSyncsOptions) error {
 	waitForRepoSyncsOptions := watchForAllSyncsOptions{
 		timeout:            nt.DefaultWaitTimeout,

e2e/testcases/basic_test.go

@@ -178,7 +178,8 @@ func manageNamespace(nt *nomostest.NT, namespace string) {
 	}
 
 	nt.T.Cleanup(func() {
-		if err := nt.KubeClient.Delete(fake.ServiceObject(core.Name("some-other-service"), core.Namespace(namespace))); err != nil {
+		svcObj := fake.ServiceObject(core.Name("some-other-service"), core.Namespace(namespace))
+		if err := nomostest.DeleteObjectsAndWait(nt, svcObj); err != nil {
 			nt.T.Fatal(err)
 		}
 	})

e2e/testcases/kcc_test.go

@@ -18,16 +18,14 @@ import (
 	"fmt"
 	"testing"
 
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"kpt.dev/configsync/e2e/nomostest"
 	"kpt.dev/configsync/e2e/nomostest/ntopts"
+	"kpt.dev/configsync/e2e/nomostest/taskgroup"
 	nomostesting "kpt.dev/configsync/e2e/nomostest/testing"
-	"kpt.dev/configsync/e2e/nomostest/testpredicates"
 	"kpt.dev/configsync/pkg/api/configsync"
 	"kpt.dev/configsync/pkg/testing/fake"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // This file includes tests for KCC resources from a cloud source repository.
@@ -72,10 +70,24 @@ func TestKCCResourcesOnCSR(t *testing.T) {
 		Version: "v1beta1",
 		Kind:    "IAMPolicyMember",
 	}
-	validateKCCResourceReady(nt, gvkPubSubTopic, "test-cs", "foo")
-	validateKCCResourceReady(nt, gvkPubSubSubscription, "test-cs-read", "foo")
-	validateKCCResourceReady(nt, gvkServiceAccount, "pubsub-app", "foo")
-	validateKCCResourceReady(nt, gvkPolicyMember, "policy-member-binding", "foo")
+
+	// Wait until all objects are reconciled
+	tg := taskgroup.New()
+	tg.Go(func() error {
+		return nt.Watcher.WatchForCurrentStatus(gvkPubSubTopic, "test-cs", "foo")
+	})
+	tg.Go(func() error {
+		return nt.Watcher.WatchForCurrentStatus(gvkPubSubSubscription, "test-cs-read", "foo")
+	})
+	tg.Go(func() error {
+		return nt.Watcher.WatchForCurrentStatus(gvkServiceAccount, "pubsub-app", "foo")
+	})
+	tg.Go(func() error {
+		return nt.Watcher.WatchForCurrentStatus(gvkPolicyMember, "policy-member-binding", "foo")
+	})
+	if err := tg.Wait(); err != nil {
+		nt.T.Fatal(err)
+	}
 
 	// Remove the kcc resources
 	nt.T.Log("sync to an empty directory from a CSR repo")
@@ -89,43 +101,21 @@ func TestKCCResourcesOnCSR(t *testing.T) {
 		nt.T.Fatal(err)
 	}
 
-	// Verify that the GCP resources are removed.
-	validateKCCResourceNotFound(nt, gvkPubSubTopic, "test-cs", "foo")
-	validateKCCResourceNotFound(nt, gvkPubSubSubscription, "test-cs-read", "foo")
-	validateKCCResourceNotFound(nt, gvkServiceAccount, "pubsub-app", "foo")
-	validateKCCResourceNotFound(nt, gvkPolicyMember, "policy-member-binding", "foo")
-}
-
-func validateKCCResourceReady(nt *nomostest.NT, gvk schema.GroupVersionKind, name, namespace string) {
-	nomostest.Wait(nt.T, fmt.Sprintf("wait for kcc resources %q %v to be ready", name, gvk),
-		nt.DefaultWaitTimeout, func() error {
-			u := &unstructured.Unstructured{}
-			u.SetGroupVersionKind(gvk)
-			return nt.Validate(name, namespace, u, kccResourceReady)
-		})
-}
-
-func kccResourceReady(o client.Object) error {
-	if o == nil {
-		return testpredicates.ErrObjectNotFound
-	}
-	u := o.(*unstructured.Unstructured)
-	conditions, found, err := unstructured.NestedSlice(u.Object, "status", "conditions")
-	if err != nil || !found || len(conditions) == 0 {
-		return fmt.Errorf(".status.conditions not found %v", err)
-	}
-	condition := (conditions[0]).(map[string]interface{})
-	if condition["type"] != "Ready" || condition["status"] != "True" {
-		return fmt.Errorf("resource is not ready %v", condition)
+	// Wait until all objects are not found
+	tg = taskgroup.New()
+	tg.Go(func() error {
+		return nt.Watcher.WatchForNotFound(gvkPubSubTopic, "test-cs", "foo")
+	})
+	tg.Go(func() error {
+		return nt.Watcher.WatchForNotFound(gvkPubSubSubscription, "test-cs-read", "foo")
+	})
+	tg.Go(func() error {
+		return nt.Watcher.WatchForNotFound(gvkServiceAccount, "pubsub-app", "foo")
+	})
+	tg.Go(func() error {
+		return nt.Watcher.WatchForNotFound(gvkPolicyMember, "policy-member-binding", "foo")
+	})
+	if err := tg.Wait(); err != nil {
+		nt.T.Fatal(err)
 	}
-	return nil
-}
-
-func validateKCCResourceNotFound(nt *nomostest.NT, gvk schema.GroupVersionKind, name, namespace string) {
-	nomostest.Wait(nt.T, fmt.Sprintf("wait for %q %v to terminate", name, gvk),
-		nt.DefaultWaitTimeout, func() error {
-			u := &unstructured.Unstructured{}
-			u.SetGroupVersionKind(gvk)
-			return nt.ValidateNotFound(name, namespace, u)
-		})
 }

e2e/testcases/multi_sync_test.go

@@ -773,7 +773,7 @@ func TestControllerValidationErrors(t *testing.T) {
 		nt.T.Fatal(err)
 	}
 	t.Cleanup(func() {
-		if err := nt.KubeClient.Delete(testNamespace); err != nil {
+		if err := nomostest.DeleteObjectsAndWait(nt, testNamespace); err != nil {
 			nt.T.Fatal(err)
 		}
 	})
@@ -794,7 +794,7 @@ func TestControllerValidationErrors(t *testing.T) {
 	}
 	nt.WaitForRootSyncStalledError(rootSync.Namespace, rootSync.Name, "Validation", "RootSync objects are only allowed in the config-management-system namespace, not in test-ns")
 	t.Cleanup(func() {
-		if err := nt.KubeClient.Delete(rootSync); err != nil {
+		if err := nomostest.DeleteObjectsAndWait(nt, rootSync); err != nil {
 			nt.T.Fatal(err)
 		}
 	})
@@ -805,7 +805,7 @@ func TestControllerValidationErrors(t *testing.T) {
 		nt.T.Fatal(err)
 	}
 	nt.WaitForRepoSyncStalledError(rs.Namespace, rs.Name, "Validation", "RepoSync objects are not allowed in the config-management-system namespace")
-	if err := nt.KubeClient.Delete(rs); err != nil {
+	if err := nomostest.DeleteObjectsAndWait(nt, rs); err != nil {
 		nt.T.Fatal(err)
 	}
 
@@ -823,7 +823,7 @@ func TestControllerValidationErrors(t *testing.T) {
 		fmt.Sprintf(`Invalid reconciler name "ns-reconciler-%s-%s-%d": must be no more than %d characters.`,
 			testNs, veryLongName, len(veryLongName), validation.DNS1123SubdomainMaxLength))
 	t.Cleanup(func() {
-		if err := nt.KubeClient.Delete(rs); err != nil {
+		if err := nomostest.DeleteObjectsAndWait(nt, rs); err != nil {
 			nt.T.Fatal(err)
 		}
 	})
@@ -838,7 +838,7 @@ func TestControllerValidationErrors(t *testing.T) {
 		fmt.Sprintf(`The managed secret name "ns-reconciler-%s-%s-%d-%s" is invalid: must be no more than %d characters. To fix it, update '.spec.git.secretRef.name'`,
 			testNs, rsInvalidSecretRef.Name, len(rsInvalidSecretRef.Name), v1beta1.GetSecretName(rsInvalidSecretRef.Spec.SecretRef), validation.DNS1123SubdomainMaxLength))
 	t.Cleanup(func() {
-		if err := nt.KubeClient.Delete(rsInvalidSecretRef); err != nil {
+		if err := nomostest.DeleteObjectsAndWait(nt, rsInvalidSecretRef); err != nil {
 			nt.T.Fatal(err)
 		}
 	})

e2e/testcases/namespace_repo_test.go

@@ -193,10 +193,8 @@ func TestDeleteRepoSync_Delegated_AndRepoSyncV1Alpha1(t *testing.T) {
 	}
 	secretNames := getNsReconcilerSecrets(nt, bsNamespace)
 
-	// Delete RepoSync custom resource from the cluster.
-	err := nt.KubeClient.Delete(&rs)
-	if err != nil {
-		nt.T.Fatalf("RepoSync delete failed: %v", err)
+	if err := nomostest.DeleteObjectsAndWait(nt, &rs); err != nil {
+		nt.T.Fatal(err)
 	}
 
 	checkRepoSyncResourcesNotPresent(nt, bsNamespace, secretNames)

e2e/testcases/namespaces_test.go

@@ -820,10 +820,8 @@ func checkpointProtectedNamespace(nt *nomostest.NT, namespace string) {
 		if apierrors.IsNotFound(err) {
 			nt.T.Cleanup(func() {
 				// Revert to initial state (not found).
-				if err := nt.KubeClient.Delete(nsObj); err != nil {
-					if !apierrors.IsNotFound(err) {
-						nt.T.Errorf("Failed to revert %q namespace: %v", namespace, err)
-					}
+				if err := nomostest.DeleteObjectsAndWait(nt, nsObj); err != nil {
+					nt.T.Fatal(err)
 				}
 			})
 			return

e2e/testcases/oci_sync_test.go

@@ -325,6 +325,19 @@ func TestSwitchFromGitToOci(t *testing.T) {
 		nt.T.Fatal(err)
 	}
 
+	// To facilitate cleanup, add the implicit namespace explicitly.
+	// Otherwise, the reconciler finalizer will skip deleting the implicit
+	// namespace, because it has the PreventDeletion annotation.
+	t.Cleanup(func() {
+		implictNs := &corev1.Namespace{}
+		implictNs.Name = namespace
+		nt.Must(nt.RootRepos[configsync.RootSyncName].Add(repoResourcePath, implictNs))
+		nt.Must(nt.RootRepos[configsync.RootSyncName].CommitAndPush("add implicit namespace explicitly"))
+		if err := nt.WatchForAllSyncs(); err != nil {
+			nt.T.Fatal(err)
+		}
+	})
+
 	// Verify the central controlled configuration: switch from Git to OCI
 	// Backward compatibility check. Previously managed RepoSync objects without sourceType should still work.
 	nt.T.Log("Add the RepoSync object to the Root Repo")
@@ -457,14 +470,6 @@ func TestSwitchFromGitToOci(t *testing.T) {
 	nt.T.Log("Manually patch RepoSync object to miss OCI spec when sourceType is oci")
 	nt.MustMergePatch(rs, `{"spec":{"sourceType":"oci"}}`)
 	nt.WaitForRepoSyncStalledError(namespace, configsync.RepoSyncName, "Validation", `KNV1061: RepoSyncs must specify spec.oci when spec.sourceType is "oci"`)
-
-	// Stop the Config Sync webhook to delete the implicit namespace manually
-	nomostest.StopWebhook(nt)
-	if err := nt.KubeClient.Delete(implictNs); err != nil {
-		{
-			nt.T.Error(err)
-		}
-	}
 }
 
 func parseObjectFromFile(nt *nomostest.NT, absPath string) (client.Object, error) {

e2e/testcases/root_sync_test.go

@@ -48,20 +48,13 @@ import (
 func TestDeleteRootSyncAndRootSyncV1Alpha1(t *testing.T) {
 	nt := nomostest.New(t, nomostesting.ACMController)
 
-	var rs v1beta1.RootSync
-	err := nt.Validate(configsync.RootSyncName, v1.NSConfigManagementSystem, &rs)
+	rs := &v1beta1.RootSync{}
+	err := nt.Validate(configsync.RootSyncName, v1.NSConfigManagementSystem, rs)
 	if err != nil {
 		nt.T.Fatal(err)
 	}
 
-	// Delete RootSync custom resource from the cluster.
-	err = nt.KubeClient.Delete(&rs)
-	if err != nil {
-		nt.T.Fatalf("deleting RootSync: %v", err)
-	}
-
-	// Verify RootSync no longer present.
-	if err := nt.Watcher.WatchForNotFound(kinds.RootSyncV1Beta1(), rs.GetName(), rs.GetNamespace()); err != nil {
+	if err := nomostest.DeleteObjectsAndWait(nt, rs); err != nil {
 		nt.T.Fatal(err)
 	}