diff --git a/e2e/testcases/lifecycle_directives_test.go b/e2e/testcases/lifecycle_directives_test.go index cb5c5b41f..dbf3c8da4 100644 --- a/e2e/testcases/lifecycle_directives_test.go +++ b/e2e/testcases/lifecycle_directives_test.go @@ -26,9 +26,11 @@ import ( nomostesting "kpt.dev/configsync/e2e/nomostest/testing" "kpt.dev/configsync/e2e/nomostest/testpredicates" "kpt.dev/configsync/pkg/api/configsync" + "kpt.dev/configsync/pkg/api/configsync/v1beta1" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/core/k8sobjects" "kpt.dev/configsync/pkg/kinds" + "kpt.dev/configsync/pkg/metadata" "kpt.dev/configsync/pkg/syncer/differ" "kpt.dev/configsync/pkg/util" "sigs.k8s.io/cli-utils/pkg/common" @@ -42,10 +44,7 @@ func TestPreventDeletionNamespace(t *testing.T) { rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID) // Ensure the Namespace doesn't already exist. - err := nt.ValidateNotFound("shipping", "", &corev1.Namespace{}) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.ValidateNotFound("shipping", "", &corev1.Namespace{})) role := k8sobjects.RoleObject(core.Name("shipping-admin")) role.Rules = []rbacv1.PolicyRule{{ @@ -61,23 +60,22 @@ func TestPreventDeletionNamespace(t *testing.T) { nt.Must(rootSyncGitRepo.CommitAndPush("declare Namespace with prevent deletion lifecycle annotation")) nt.Must(nt.WatchForAllSyncs()) - err = nt.Validate("shipping", "", &corev1.Namespace{}, + rsObj := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncNN.Name, rootSyncNN.Namespace, rsObj)) + applySetID := core.GetLabel(rsObj, metadata.ApplySetParentIDLabel) + + nt.Must(nt.Validate("shipping", "", &corev1.Namespace{}, testpredicates.NotPendingDeletion, testpredicates.HasAnnotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), - testpredicates.HasAllNomosMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, nsObj) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, role) - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) // Delete the declaration and ensure the Namespace isn't deleted. nt.Must(rootSyncGitRepo.Remove("acme/namespaces/shipping/ns.yaml")) @@ -86,28 +84,21 @@ func TestPreventDeletionNamespace(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) // Ensure we kept the undeclared Namespace that had the "deletion: prevent" annotation. - err = nt.Validate("shipping", "", &corev1.Namespace{}, + nt.Must(nt.Validate("shipping", "", &corev1.Namespace{}, testpredicates.NotPendingDeletion, testpredicates.HasAnnotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), - testpredicates.NoConfigSyncMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) + // Ensure we deleted the undeclared Role that doesn't have the annotation. - err = nt.ValidateNotFound("shipping-admin", "shipping", &rbacv1.Role{}) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.ValidateNotFound("shipping-admin", "shipping", &rbacv1.Role{})) nt.MetricsExpectations.RemoveObject(configsync.RootSyncKind, rootSyncNN, nsObj) nt.MetricsExpectations.AddObjectDelete(configsync.RootSyncKind, rootSyncNN, role) - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) // Remove the lifecycle annotation from the namespace so that the namespace can be deleted after the test case. nsObj = k8sobjects.NamespaceObject("shipping") @@ -118,12 +109,9 @@ func TestPreventDeletionNamespace(t *testing.T) { nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, nsObj) nt.MetricsExpectations.RemoveObject(configsync.RootSyncKind, rootSyncNN, role) - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) } func TestPreventDeletionRole(t *testing.T) { @@ -132,10 +120,7 @@ func TestPreventDeletionRole(t *testing.T) { rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID) // Ensure the Namespace doesn't already exist. - err := nt.ValidateNotFound("shipping-admin", "shipping", &rbacv1.Role{}) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.ValidateNotFound("shipping-admin", "shipping", &rbacv1.Role{})) // Declare the Role with the lifecycle annotation, and ensure it is created. role := k8sobjects.RoleObject(core.Name("shipping-admin"), preventDeletion) @@ -150,53 +135,47 @@ func TestPreventDeletionRole(t *testing.T) { nt.Must(rootSyncGitRepo.CommitAndPush("declare Role with prevent deletion lifecycle annotation")) nt.Must(nt.WatchForAllSyncs()) + rsObj := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncNN.Name, rootSyncNN.Namespace, rsObj)) + applySetID := core.GetLabel(rsObj, metadata.ApplySetParentIDLabel) + // ensure that the Role is created with the preventDeletion annotation - err = nt.Validate("shipping-admin", "shipping", &rbacv1.Role{}, + nt.Must(nt.Validate("shipping-admin", "shipping", &rbacv1.Role{}, testpredicates.NotPendingDeletion, testpredicates.HasAnnotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), - testpredicates.HasAllNomosMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, nsObj) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, role) // Validate metrics. - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) // Delete the declaration and ensure the Namespace isn't deleted. nt.Must(rootSyncGitRepo.Remove("acme/namespaces/shipping/role.yaml")) nt.Must(rootSyncGitRepo.CommitAndPush("remove Role declaration")) nt.Must(nt.WatchForAllSyncs()) - err = nt.Validate("shipping-admin", "shipping", &rbacv1.Role{}, + nt.Must(nt.Validate("shipping-admin", "shipping", &rbacv1.Role{}, testpredicates.NotPendingDeletion, testpredicates.HasAnnotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), - testpredicates.NoConfigSyncMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) nt.MetricsExpectations.RemoveObject(configsync.RootSyncKind, rootSyncNN, role) // Validate metrics. - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, // Adjust operations for this edge case. // Deletion is prevented, but management annotations/labels are removed. Operations: []metrics.ObjectOperation{ {Operation: metrics.UpdateOperation, Count: 1}, // Role }, - }) - if err != nil { - nt.T.Fatal(err) - } + })) // Remove the lifecycle annotation from the role so that the role can be deleted after the test case. delete(role.Annotations, common.LifecycleDeleteAnnotation) @@ -204,23 +183,18 @@ func TestPreventDeletionRole(t *testing.T) { nt.Must(rootSyncGitRepo.CommitAndPush("remove the lifecycle annotation from Role")) nt.Must(nt.WatchForAllSyncs()) - err = nt.Validate("shipping-admin", "shipping", &rbacv1.Role{}, + nt.Must(nt.Validate("shipping-admin", "shipping", &rbacv1.Role{}, testpredicates.NotPendingDeletion, testpredicates.MissingAnnotation(common.LifecycleDeleteAnnotation), - testpredicates.HasAllNomosMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, role) // Validate metrics. - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) } func TestPreventDeletionClusterRole(t *testing.T) { @@ -229,10 +203,7 @@ func TestPreventDeletionClusterRole(t *testing.T) { rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID) // Ensure the ClusterRole doesn't already exist. - err := nt.ValidateNotFound("test-admin", "", &rbacv1.ClusterRole{}) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.ValidateNotFound("test-admin", "", &rbacv1.ClusterRole{})) // Declare the ClusterRole with the lifecycle annotation, and ensure it is created. clusterRole := k8sobjects.ClusterRoleObject(core.Name("test-admin"), preventDeletion) @@ -245,26 +216,26 @@ func TestPreventDeletionClusterRole(t *testing.T) { nt.Must(rootSyncGitRepo.CommitAndPush("declare ClusterRole with prevent deletion lifecycle annotation")) nt.Must(nt.WatchForAllSyncs()) - err = nt.Validate("test-admin", "", &rbacv1.ClusterRole{}, + rsObj := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncNN.Name, rootSyncNN.Namespace, rsObj)) + applySetID := core.GetLabel(rsObj, metadata.ApplySetParentIDLabel) + + nt.Must(nt.Validate("test-admin", "", &rbacv1.ClusterRole{}, testpredicates.NotPendingDeletion, testpredicates.HasAnnotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), - testpredicates.HasAllNomosMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) // Delete the declaration and ensure the ClusterRole isn't deleted. nt.Must(rootSyncGitRepo.Remove("acme/cluster/cr.yaml")) nt.Must(rootSyncGitRepo.CommitAndPush("remove ClusterRole bar declaration")) nt.Must(nt.WatchForAllSyncs()) - err = nt.Validate("test-admin", "", &rbacv1.ClusterRole{}, + nt.Must(nt.Validate("test-admin", "", &rbacv1.ClusterRole{}, testpredicates.NotPendingDeletion, testpredicates.HasAnnotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), - testpredicates.NoConfigSyncMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) // Remove the lifecycle annotation from the cluster-role so that it can be deleted after the test case. delete(clusterRole.Annotations, common.LifecycleDeleteAnnotation) @@ -272,23 +243,18 @@ func TestPreventDeletionClusterRole(t *testing.T) { nt.Must(rootSyncGitRepo.CommitAndPush("remove the lifecycle annotation from ClusterRole")) nt.Must(nt.WatchForAllSyncs()) - err = nt.Validate("test-admin", "", &rbacv1.ClusterRole{}, + nt.Must(nt.Validate("test-admin", "", &rbacv1.ClusterRole{}, testpredicates.NotPendingDeletion, testpredicates.MissingAnnotation(common.LifecycleDeleteAnnotation), - testpredicates.HasAllNomosMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, clusterRole) // Validate metrics. - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: rootSyncNN, - }) - if err != nil { - nt.T.Fatal(err) - } + })) } func skipAutopilotManagedNamespace(nt *nomostest.NT, ns string) bool { @@ -297,9 +263,10 @@ func skipAutopilotManagedNamespace(nt *nomostest.NT, ns string) bool { } func TestPreventDeletionSpecialNamespaces(t *testing.T) { + rootSyncID := nomostest.DefaultRootSyncID nt := nomostest.New(t, nomostesting.Lifecycle, - ntopts.SyncWithGitSource(nomostest.DefaultRootSyncID, ntopts.Unstructured)) - rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID) + ntopts.SyncWithGitSource(rootSyncID, ntopts.Unstructured)) + rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(rootSyncID) // Build list of special namespaces to test. // Skip namespaces managed by GKE Autopilot, if on an Autopilot cluster @@ -319,24 +286,25 @@ func TestPreventDeletionSpecialNamespaces(t *testing.T) { nt.Must(rootSyncGitRepo.CommitAndPush("Add special namespaces and one non-special namespace")) nt.Must(nt.WatchForAllSyncs()) + rsObj := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncID.Name, rootSyncID.Namespace, rsObj)) + applySetID := core.GetLabel(rsObj, metadata.ApplySetParentIDLabel) + // Verify that the special namespaces have the `client.lifecycle.config.k8s.io/deletion: detach` annotation. for ns := range specialNamespaces { - if err := nt.Validate(ns, "", &corev1.Namespace{}, + nt.Must(nt.Validate(ns, "", &corev1.Namespace{}, testpredicates.NotPendingDeletion, testpredicates.HasAnnotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), - testpredicates.HasAllNomosMetadata()); err != nil { - nt.T.Fatal(err) - } + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) } // Verify that the bookstore namespace does not have the `client.lifecycle.config.k8s.io/deletion: detach` annotation. - err := nt.Validate(bookstoreNS.Name, bookstoreNS.Namespace, &corev1.Namespace{}, + nt.Must(nt.Validate(bookstoreNS.Name, bookstoreNS.Namespace, &corev1.Namespace{}, testpredicates.NotPendingDeletion, testpredicates.MissingAnnotation(common.LifecycleDeleteAnnotation), - testpredicates.HasAllNomosMetadata()) - if err != nil { - nt.T.Fatal(err) - } + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) for ns := range specialNamespaces { nt.Must(rootSyncGitRepo.Remove(fmt.Sprintf("acme/ns-%s.yaml", ns))) @@ -347,20 +315,16 @@ func TestPreventDeletionSpecialNamespaces(t *testing.T) { // Verify that the special namespaces still exist and have the `client.lifecycle.config.k8s.io/deletion: detach` annotation. for ns := range specialNamespaces { - if err := nt.Validate(ns, "", &corev1.Namespace{}, + nt.Must(nt.Validate(ns, "", &corev1.Namespace{}, testpredicates.NotPendingDeletion, testpredicates.HasAnnotation(common.LifecycleDeleteAnnotation, common.PreventDeletion), - testpredicates.NoConfigSyncMetadata()); err != nil { - nt.T.Fatal(err) - } + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) } // Verify that the bookstore namespace is removed. // Namespace should be marked as deleted, but may not be NotFound yet, // because its finalizer will block until all objects in that namespace are // deleted. - err = nt.Watcher.WatchForNotFound(kinds.Namespace(), bookstoreNS.Name, bookstoreNS.Namespace) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Watcher.WatchForNotFound(kinds.Namespace(), bookstoreNS.Name, bookstoreNS.Namespace)) } diff --git a/e2e/testcases/namespaces_test.go b/e2e/testcases/namespaces_test.go index 9185b6e36..fc59a4050 100644 --- a/e2e/testcases/namespaces_test.go +++ b/e2e/testcases/namespaces_test.go @@ -31,6 +31,7 @@ import ( "kpt.dev/configsync/e2e/nomostest/testpredicates" "kpt.dev/configsync/e2e/nomostest/testwatcher" "kpt.dev/configsync/pkg/api/configsync" + "kpt.dev/configsync/pkg/api/configsync/v1beta1" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/core/k8sobjects" "kpt.dev/configsync/pkg/kinds" @@ -232,6 +233,10 @@ func TestManagementDisabledNamespace(t *testing.T) { rootSyncNN := nomostest.RootSyncNN(configsync.RootSyncName) + rsObj := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncNN.Name, rootSyncNN.Namespace, rsObj)) + applySetID := core.GetLabel(rsObj, metadata.ApplySetParentIDLabel) + namespacesToTest := []string{"foo", metav1.NamespaceDefault} for _, nsName := range namespacesToTest { // Create nsObj. @@ -243,26 +248,21 @@ func TestManagementDisabledNamespace(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) // Test that the namespace exists with expected config management labels and annotations. - err := nt.Validate(nsObj.Name, "", &corev1.Namespace{}, testpredicates.HasAllNomosMetadata()) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(nsObj.Name, "", &corev1.Namespace{}, + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) // Test that the configmap exists with expected config management labels and annotations. - err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, testpredicates.HasAllNomosMetadata()) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, nsObj) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, cm1) - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: nomostest.RootSyncNN(configsync.RootSyncName), - }) - if err != nil { - nt.T.Fatal(err) - } + })) // Update the namespace and the configmap to be no longer be managed nsObj.Annotations[metadata.ResourceManagementKey] = metadata.ResourceManagementDisabled @@ -273,26 +273,21 @@ func TestManagementDisabledNamespace(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) // Test that the now unmanaged namespace does not contain any config management labels or annotations - err = nt.Validate(nsObj.Name, "", &corev1.Namespace{}, testpredicates.NoConfigSyncMetadata()) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(nsObj.Name, "", &corev1.Namespace{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) // Test that the now unmanaged configmap does not contain any config management labels or annotations - err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, testpredicates.NoConfigSyncMetadata()) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, nsObj) nt.MetricsExpectations.AddObjectApply(configsync.RootSyncKind, rootSyncNN, cm1) - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: nomostest.RootSyncNN(configsync.RootSyncName), - }) - if err != nil { - nt.T.Fatal(err) - } + })) // Remove the namspace and the configmap from the repository nt.Must(rootSyncGitRepo.Remove(fmt.Sprintf("acme/namespaces/%s", nsName))) @@ -300,26 +295,21 @@ func TestManagementDisabledNamespace(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) // Test that the namespace still exists on the cluster, and does not contain any config management labels or annotations - err = nt.Validate(nsObj.Name, "", &corev1.Namespace{}, testpredicates.NoConfigSyncMetadata()) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(nsObj.Name, "", &corev1.Namespace{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) // Test that the configmap still exists on the cluster, and does not contain any config management labels or annotations - err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, testpredicates.NoConfigSyncMetadata()) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) nt.MetricsExpectations.RemoveObject(configsync.RootSyncKind, rootSyncNN, nsObj) nt.MetricsExpectations.RemoveObject(configsync.RootSyncKind, rootSyncNN, cm1) - err = nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ + nt.Must(nomostest.ValidateStandardMetricsForRootSync(nt, metrics.Summary{ Sync: nomostest.RootSyncNN(configsync.RootSyncName), - }) - if err != nil { - nt.T.Fatal(err) - } + })) } } @@ -343,14 +333,22 @@ func TestManagementDisabledConfigMap(t *testing.T) { })) rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID) + rsObj := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncNN.Name, rootSyncNN.Namespace, rsObj)) + applySetID := core.GetLabel(rsObj, metadata.ApplySetParentIDLabel) + // Test that the namespace exists with expected config management labels and annotations. - err := nt.Validate(fooNamespace.Name, "", &corev1.Namespace{}, testpredicates.HasAllNomosMetadata()) + err := nt.Validate(fooNamespace.Name, "", &corev1.Namespace{}, + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID)) if err != nil { nt.T.Error(err) } // Test that cm1 exists with expected config management labels and annotations. - err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, testpredicates.HasAllNomosMetadata()) + err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID)) if err != nil { nt.T.Error(err) } @@ -362,7 +360,9 @@ func TestManagementDisabledConfigMap(t *testing.T) { } // Test that cm3 exists with expected config management labels and annotations. - err = nt.Validate(cm3.Name, cm3.Namespace, &corev1.ConfigMap{}, testpredicates.HasAllNomosMetadata()) + err = nt.Validate(cm3.Name, cm3.Namespace, &corev1.ConfigMap{}, + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID)) if err != nil { nt.T.Error(err) } @@ -389,7 +389,9 @@ func TestManagementDisabledConfigMap(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) // Test that the now unmanaged configmap does not contain any config management labels or annotations - err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, testpredicates.NoConfigSyncMetadata()) + err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel)) if err != nil { nt.T.Error(err) } @@ -421,7 +423,9 @@ func TestManagementDisabledConfigMap(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) // Test that the configmap still exists on the cluster, and does not contain any config management labels or annotations - err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, testpredicates.NoConfigSyncMetadata()) + err = nt.Validate(cm1.Name, cm1.Namespace, &corev1.ConfigMap{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel)) if err != nil { nt.T.Error(err) } @@ -491,7 +495,8 @@ func TestSyncLabelsAndAnnotationsOnKubeSystem(t *testing.T) { // Test that the kube-system namespace exists without the label and annotation. err = nt.Validate(kubeSystemNamespace.Name, "", &corev1.Namespace{}, - testpredicates.MissingLabel("test-corp.com/awesome-controller-flavour"), testpredicates.MissingAnnotation("test-corp.com/awesome-controller-mixin")) + testpredicates.MissingLabel("test-corp.com/awesome-controller-flavour"), + testpredicates.MissingAnnotation("test-corp.com/awesome-controller-mixin")) if err != nil { nt.T.Error(err) } @@ -512,7 +517,9 @@ func TestSyncLabelsAndAnnotationsOnKubeSystem(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) // Test that the now unmanaged kube-system namespace does not contain any config management labels or annotations. - err = nt.Validate(kubeSystemNamespace.Name, "", &corev1.Namespace{}, testpredicates.NoConfigSyncMetadata()) + err = nt.Validate(kubeSystemNamespace.Name, "", &corev1.Namespace{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel)) if err != nil { nt.T.Error(err) } diff --git a/e2e/testcases/reconciler_finalizer_test.go b/e2e/testcases/reconciler_finalizer_test.go index bb090c2ea..b6a415639 100644 --- a/e2e/testcases/reconciler_finalizer_test.go +++ b/e2e/testcases/reconciler_finalizer_test.go @@ -80,9 +80,7 @@ func TestReconcilerFinalizer_Orphan(t *testing.T) { nt.Must(rootSyncGitRepo.Add(deployment1Path, deployment1)) nt.Must(rootSyncGitRepo.CommitAndPush("Adding deployment helloworld-1 to RootSync")) nt.Must(nt.WatchForAllSyncs()) - if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), deployment1.Name, deployment1.Namespace); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), deployment1.Name, deployment1.Namespace)) // Tail reconciler logs and print if there's an error. // This is necessary because if the RootSync are deleted, the @@ -93,16 +91,12 @@ func TestReconcilerFinalizer_Orphan(t *testing.T) { go nomostest.TailReconcilerLogs(ctx, nt, nomostest.RootReconcilerObjectKey(rootSyncKey.Name)) nt.T.Log("Disabling RootSync deletion propagation") - rootSync := k8sobjects.RootSyncObjectV1Beta1(rootSyncKey.Name) - err := nt.KubeClient.Get(rootSync.GetName(), rootSync.GetNamespace(), rootSync) - if err != nil { - nt.T.Fatal(err) - } + rootSync := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncID.Name, rootSyncID.Namespace, rootSync)) + applySetID := core.GetLabel(rootSync, metadata.ApplySetParentIDLabel) + if nomostest.SetDeletionPropagationPolicy(rootSync, metadata.DeletionPropagationPolicyOrphan) { - err = nt.KubeClient.Update(rootSync) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Update(rootSync)) } nt.Must(nt.Watcher.WatchObject(kinds.RootSyncV1Beta1(), rootSync.GetName(), rootSync.GetNamespace(), testwatcher.WatchPredicates( @@ -111,26 +105,27 @@ func TestReconcilerFinalizer_Orphan(t *testing.T) { ))) // Delete the RootSync - err = nt.KubeClient.Delete(rootSync) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Delete(rootSync)) // The RootSync should skip finalizing and be deleted immediately - err = nt.Watcher.WatchForNotFound(kinds.RootSyncV1Beta1(), rootSync.GetName(), rootSync.GetNamespace()) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Watcher.WatchForNotFound(kinds.RootSyncV1Beta1(), rootSync.GetName(), rootSync.GetNamespace())) + tg := taskgroup.New() tg.Go(func() error { // Namespace1 should NOT have been deleted, because it was orphaned by the RootSync. return nt.Watcher.WatchObject(kinds.Namespace(), namespace1.GetName(), namespace1.GetNamespace(), - testwatcher.WatchPredicates(testpredicates.HasAllNomosMetadata())) // metadata NOT removed when orphaned + testwatcher.WatchPredicates( + testpredicates.HasAllNomosMetadata(), // metadata NOT removed when orphaned + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID), + )) }) tg.Go(func() error { // Deployment1 should NOT have been deleted, because it was orphaned by the RootSync. return nt.Watcher.WatchObject(kinds.Deployment(), deployment1.GetName(), deployment1.GetNamespace(), - testwatcher.WatchPredicates(testpredicates.HasAllNomosMetadata())) // metadata NOT removed when orphaned + testwatcher.WatchPredicates( + testpredicates.HasAllNomosMetadata(), // metadata NOT removed when orphaned + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID), + )) }) if err := tg.Wait(); err != nil { nt.T.Fatal(err) @@ -383,9 +378,7 @@ func TestReconcilerFinalizer_MultiLevelMixed(t *testing.T) { nt.Must(rootSyncGitRepo.Add(deployment1Path, deployment1)) nt.Must(rootSyncGitRepo.CommitAndPush("Adding deployment helloworld-1 to RootSync")) nt.Must(nt.WatchForAllSyncs()) - if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), deployment1.Name, deployment1.Namespace); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), deployment1.Name, deployment1.Namespace)) // Add deployment-helloworld-2 to RepoSync deployment2Path := nomostest.StructuredNSPath(deployment2NN.Namespace, "deployment-helloworld-2") @@ -395,9 +388,7 @@ func TestReconcilerFinalizer_MultiLevelMixed(t *testing.T) { nt.Must(repoSyncGitRepo.Add(deployment2Path, deployment2)) nt.Must(repoSyncGitRepo.CommitAndPush("Adding deployment helloworld-2 to RepoSync")) nt.Must(nt.WatchForAllSyncs()) - if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), deployment2.Name, deployment2.Namespace); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), deployment2.Name, deployment2.Namespace)) // Tail reconciler logs and print if there's an error. // This is necessary because if the RootSync/RepoSync are deleted, the @@ -409,16 +400,10 @@ func TestReconcilerFinalizer_MultiLevelMixed(t *testing.T) { go nomostest.TailReconcilerLogs(ctx, nt, nomostest.NsReconcilerObjectKey(repoSyncKey.Namespace, repoSyncKey.Name)) nt.T.Log("Enabling RootSync deletion propagation") - rootSync := k8sobjects.RootSyncObjectV1Beta1(rootSyncKey.Name) - err := nt.KubeClient.Get(rootSync.Name, rootSync.Namespace, rootSync) - if err != nil { - nt.T.Fatal(err) - } + rootSync := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncID.Name, rootSyncID.Namespace, rootSync)) if nomostest.SetDeletionPropagationPolicy(rootSync, metadata.DeletionPropagationPolicyForeground) { - err = nt.KubeClient.Update(rootSync) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Update(rootSync)) } nt.Must(nt.Watcher.WatchObject(kinds.RootSyncV1Beta1(), rootSync.GetName(), rootSync.GetNamespace(), testwatcher.WatchPredicates( @@ -439,6 +424,10 @@ func TestReconcilerFinalizer_MultiLevelMixed(t *testing.T) { testpredicates.MissingFinalizer(metadata.ReconcilerFinalizer), ))) + repoSync = &v1beta1.RepoSync{} + nt.Must(nt.KubeClient.Get(repoSyncID.Name, repoSyncID.Namespace, repoSync)) + repoSyncApplySetID := core.GetLabel(repoSync, metadata.ApplySetParentIDLabel) + // Abandon the test namespace, otherwise it will block the finalizer namespace1Path := nomostest.StructuredNSPath(namespace1NN.Name, namespace1NN.Name) namespace1 := rootSyncGitRepo.MustGet(nt.T, namespace1Path) @@ -448,10 +437,7 @@ func TestReconcilerFinalizer_MultiLevelMixed(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) // Delete the RootSync - err = nt.KubeClient.Delete(rootSync) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Delete(rootSync)) tg := taskgroup.New() tg.Go(func() error { @@ -466,24 +452,26 @@ func TestReconcilerFinalizer_MultiLevelMixed(t *testing.T) { // After RepoSync and Deployment1 are deleted, the RootSync should have its finalizer removed and be garbage collected return nt.Watcher.WatchForNotFound(kinds.RootSyncV1Beta1(), rootSync.GetName(), rootSync.GetNamespace()) }) - if err := tg.Wait(); err != nil { - nt.T.Fatal(err) - } + nt.Must(tg.Wait()) + tg = taskgroup.New() tg.Go(func() error { // Namespace1 should NOT have been deleted, because it was abandoned by the RootSync. - // TODO: Use NoConfigSyncMetadata predicate once metadata removal is fixed (b/256043590) return nt.Watcher.WatchObject(kinds.Namespace(), namespace1.GetName(), namespace1.GetNamespace(), - testwatcher.WatchPredicates()) + testwatcher.WatchPredicates( + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel), + )) }) tg.Go(func() error { // Deployment2 should NOT have been deleted, because it was orphaned by the RepoSync. return nt.Watcher.WatchObject(kinds.Deployment(), deployment2.GetName(), deployment2.GetNamespace(), - testwatcher.WatchPredicates(testpredicates.HasAllNomosMetadata())) + testwatcher.WatchPredicates( + testpredicates.HasAllNomosMetadata(), // metadata NOT removed when orphaned + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, repoSyncApplySetID), + )) }) - if err := tg.Wait(); err != nil { - nt.T.Fatal(err) - } + nt.Must(tg.Wait()) } // TestReconcileFinalizerReconcileTimeout verifies that the reconciler finalizer diff --git a/e2e/testcases/sync_ordering_test.go b/e2e/testcases/sync_ordering_test.go index 641506a53..510b8913e 100644 --- a/e2e/testcases/sync_ordering_test.go +++ b/e2e/testcases/sync_ordering_test.go @@ -31,6 +31,7 @@ import ( "kpt.dev/configsync/e2e/nomostest/testpredicates" "kpt.dev/configsync/e2e/nomostest/testwatcher" "kpt.dev/configsync/pkg/api/configsync" + "kpt.dev/configsync/pkg/api/configsync/v1beta1" "kpt.dev/configsync/pkg/applier" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/core/k8sobjects" @@ -45,9 +46,10 @@ import ( // The sync ordering feature is only supported in the multi-repo mode. func TestMultiDependencies(t *testing.T) { + rootSyncID := nomostest.DefaultRootSyncID nt := nomostest.New(t, nomostesting.Lifecycle, - ntopts.SyncWithGitSource(nomostest.DefaultRootSyncID, ntopts.Unstructured)) - rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID) + ntopts.SyncWithGitSource(rootSyncID, ntopts.Unstructured)) + rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(rootSyncID) namespaceName := "bookstore" nt.T.Logf("Remove the namespace %q if it already exists", namespaceName) @@ -67,19 +69,13 @@ func TestMultiDependencies(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) ns := &corev1.Namespace{} - if err := nt.KubeClient.Get(namespaceName, "", ns); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Get(namespaceName, "", ns)) cm1 := &corev1.ConfigMap{} - if err := nt.KubeClient.Get(cm1Name, namespaceName, cm1); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Get(cm1Name, namespaceName, cm1)) cm2 := &corev1.ConfigMap{} - if err := nt.KubeClient.Get(cm2Name, namespaceName, cm2); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Get(cm2Name, namespaceName, cm2)) nt.T.Logf("Verify that the namespace is created before the configmaps in it") if cm1.CreationTimestamp.Before(&ns.CreationTimestamp) { @@ -96,9 +92,8 @@ func TestMultiDependencies(t *testing.T) { } nt.T.Logf("Verify that cm2 has the dependsOn annotation") - if err := nt.Validate(cm2Name, namespaceName, &corev1.ConfigMap{}, testpredicates.HasAnnotation(dependson.Annotation, "/namespaces/bookstore/ConfigMap/cm1")); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm2Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.HasAnnotation(dependson.Annotation, "/namespaces/bookstore/ConfigMap/cm1"))) // There are 2 configmaps in the namespace at this point: cm1, cm2. // The dependency graph is: @@ -114,14 +109,10 @@ func TestMultiDependencies(t *testing.T) { nt.T.Logf("Verify that cm1 is created before cm3") cm1 = &corev1.ConfigMap{} - if err := nt.KubeClient.Get(cm1Name, namespaceName, cm1); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Get(cm1Name, namespaceName, cm1)) cm3 := &corev1.ConfigMap{} - if err := nt.KubeClient.Get(cm3Name, namespaceName, cm3); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Get(cm3Name, namespaceName, cm3)) if cm3.CreationTimestamp.Before(&cm1.CreationTimestamp) { nt.T.Fatalf("an object (%s) should be created after its dependency (%s)", core.GKNN(cm3), core.GKNN(cm1)) } @@ -143,14 +134,10 @@ func TestMultiDependencies(t *testing.T) { nt.T.Log("Verify that cm1 is created before cm0") cm1 = &corev1.ConfigMap{} - if err := nt.KubeClient.Get(cm1Name, namespaceName, cm1); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Get(cm1Name, namespaceName, cm1)) cm0 := &corev1.ConfigMap{} - if err := nt.KubeClient.Get(cm3Name, namespaceName, cm0); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.KubeClient.Get(cm3Name, namespaceName, cm0)) if cm0.CreationTimestamp.Before(&cm1.CreationTimestamp) { nt.T.Fatalf("Declaring the dependency of an existing object (%s) on a non-existing object (%s) should not cause the existing object to be recreated", core.GKNN(cm1), core.GKNN(cm0)) } @@ -169,9 +156,8 @@ func TestMultiDependencies(t *testing.T) { nt.WaitForRootSyncSyncError(configsync.RootSyncName, applier.ApplierErrorCode, "cyclic dependency", nil) nt.T.Log("Verify that cm0 does not have the dependsOn annotation") - if err := nt.Validate(cm0Name, namespaceName, &corev1.ConfigMap{}, testpredicates.MissingAnnotation(dependson.Annotation)); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm0Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.MissingAnnotation(dependson.Annotation))) nt.T.Log("Remove the cyclic dependency from the Git repo") nt.Must(rootSyncGitRepo.Add("acme/cm0.yaml", k8sobjects.ConfigMapObject(core.Name(cm0Name), core.Namespace(namespaceName)))) @@ -191,15 +177,10 @@ func TestMultiDependencies(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) nt.T.Log("Verify that cm3 is removed") - err := nt.Watcher.WatchForNotFound(kinds.ConfigMap(), cm3Name, namespaceName) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Watcher.WatchForNotFound(kinds.ConfigMap(), cm3Name, namespaceName)) nt.T.Log("Verify that cm1 is still on the cluster") - if err := nt.Validate(cm1Name, namespaceName, &corev1.ConfigMap{}); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm1Name, namespaceName, &corev1.ConfigMap{})) // There are 3 configmaps in the namespace at this point: cm0, cm1, cm2. // The dependency graph is: @@ -214,16 +195,10 @@ func TestMultiDependencies(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) nt.T.Log("Verify that cm1 is removed") - err = nt.Watcher.WatchForNotFound(kinds.ConfigMap(), cm1Name, namespaceName) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Watcher.WatchForNotFound(kinds.ConfigMap(), cm1Name, namespaceName)) nt.T.Log("Verify that cm2 is removed") - err = nt.Watcher.WatchForNotFound(kinds.ConfigMap(), cm2Name, namespaceName) - if err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Watcher.WatchForNotFound(kinds.ConfigMap(), cm2Name, namespaceName)) // There are 1 configmap in the namespace at this point: cm0. @@ -238,19 +213,16 @@ func TestMultiDependencies(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) nt.T.Logf("Verify that cm1 has the dependsOn annotation, and depends on cm0") - if err := nt.Validate(cm1Name, namespaceName, &corev1.ConfigMap{}, testpredicates.HasAnnotation(dependson.Annotation, "/namespaces/bookstore/ConfigMap/cm0")); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm1Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.HasAnnotation(dependson.Annotation, "/namespaces/bookstore/ConfigMap/cm0"))) nt.T.Logf("Verify that cm2 has the dependsOn annotation, and depends on cm0") - if err := nt.Validate(cm2Name, namespaceName, &corev1.ConfigMap{}, testpredicates.HasAnnotation(dependson.Annotation, "/namespaces/bookstore/ConfigMap/cm0")); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm2Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.HasAnnotation(dependson.Annotation, "/namespaces/bookstore/ConfigMap/cm0"))) nt.T.Logf("Verify that cm3 has the dependsOn annotation, and depends on cm0") - if err := nt.Validate(cm3Name, namespaceName, &corev1.ConfigMap{}, testpredicates.HasAnnotation(dependson.Annotation, "/namespaces/bookstore/ConfigMap/cm0")); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm3Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.HasAnnotation(dependson.Annotation, "/namespaces/bookstore/ConfigMap/cm0"))) // There are 4 configmaps in the namespace at this point: cm0, cm1, cm2 and cm3. // The dependency graph is: @@ -266,15 +238,19 @@ func TestMultiDependencies(t *testing.T) { nt.Must(rootSyncGitRepo.CommitAndPush("Disable cm3 by adding the `configmanagement.gke.io/managed: disabled` annotation")) nt.Must(nt.WatchForAllSyncs()) + rsObj := &v1beta1.RootSync{} + nt.Must(nt.KubeClient.Get(rootSyncID.Name, rootSyncID.Namespace, rsObj)) + applySetID := core.GetLabel(rsObj, metadata.ApplySetParentIDLabel) + nt.T.Log("Verify that cm3 no longer has the CS metadata") - if err := nt.Validate(cm3Name, namespaceName, &corev1.ConfigMap{}, testpredicates.NoConfigSyncMetadata()); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm3Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) nt.T.Log("Verify that cm0 still has the CS metadata") - if err := nt.Validate(cm0Name, namespaceName, &corev1.ConfigMap{}, testpredicates.HasAllNomosMetadata()); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm0Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.HasAllNomosMetadata(), + testpredicates.HasLabel(metadata.ApplySetPartOfLabel, applySetID))) // There are 4 configmaps in the namespace at this point: cm0, cm1, cm2 and cm3. // The inventory tracks 3 configmaps: cm0, cm1, cm2. The dependency graph is: @@ -288,9 +264,8 @@ func TestMultiDependencies(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) nt.T.Log("Verify that cm2 no longer has the dependsOn annotation") - if err := nt.Validate(cm2Name, namespaceName, &corev1.ConfigMap{}, testpredicates.MissingAnnotation(dependson.Annotation)); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm2Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.MissingAnnotation(dependson.Annotation))) // There are 4 configmaps in the namespace at this point: cm0, cm1, cm2 and cm3. // The inventory tracks 3 configmaps: cm0, cm1, cm2. The dependency graph is: @@ -307,14 +282,14 @@ func TestMultiDependencies(t *testing.T) { nt.Must(nt.WatchForAllSyncs()) nt.T.Log("Verify that cm1 no longer has the CS metadata") - if err := nt.Validate(cm1Name, namespaceName, &corev1.ConfigMap{}, testpredicates.NoConfigSyncMetadata()); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm1Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) nt.T.Log("Verify that cm0 no longer has the CS metadata") - if err := nt.Validate(cm0Name, namespaceName, &corev1.ConfigMap{}, testpredicates.NoConfigSyncMetadata()); err != nil { - nt.T.Fatal(err) - } + nt.Must(nt.Validate(cm0Name, namespaceName, &corev1.ConfigMap{}, + testpredicates.NoConfigSyncMetadata(), + testpredicates.MissingLabel(metadata.ApplySetPartOfLabel))) } diff --git a/pkg/applier/applier.go b/pkg/applier/applier.go index f4d300d33..5c6cff354 100644 --- a/pkg/applier/applier.go +++ b/pkg/applier/applier.go @@ -829,8 +829,9 @@ func (s *supervisor) abandonObject(ctx context.Context, obj client.Object) error fromObj.SetLabels(uObj.GetLabels()) toObj := fromObj.DeepCopy() - updated := metadata.RemoveConfigSyncMetadata(toObj) - if !updated { + updated1 := metadata.RemoveConfigSyncMetadata(toObj) + updated2 := metadata.RemoveApplySetPartOfLabel(toObj, s.clientSet.ApplySetID) + if !updated1 && !updated2 { return nil } // Use merge-patch instead of server-side-apply, because we don't have diff --git a/pkg/applier/clientset.go b/pkg/applier/clientset.go index 796f042ab..757673921 100644 --- a/pkg/applier/clientset.go +++ b/pkg/applier/clientset.go @@ -53,6 +53,7 @@ type ClientSet struct { Client client.Client Mapper meta.RESTMapper StatusMode string + ApplySetID string } // NewClientSet constructs a new ClientSet. @@ -113,5 +114,6 @@ func NewClientSet(c client.Client, configFlags *genericclioptions.ConfigFlags, s Client: c, Mapper: mapper, StatusMode: statusMode, + ApplySetID: applySetID, }, nil } diff --git a/pkg/metadata/metadata.go b/pkg/metadata/metadata.go index e738705a1..210becb39 100644 --- a/pkg/metadata/metadata.go +++ b/pkg/metadata/metadata.go @@ -137,3 +137,20 @@ func RemoveConfigSyncMetadata(obj client.Object) bool { after := len(obj.GetAnnotations()) + len(obj.GetLabels()) return before != after } + +// RemoveApplySetPartOfLabel removes the ApplySet part-of label IFF the value +// matches the specified applySetID. +// The resource is modified in place. Returns true if the object was modified. +func RemoveApplySetPartOfLabel(obj client.Object, applySetID string) bool { + labels := obj.GetLabels() + if labels == nil { + return false + } + v, found := labels[ApplySetPartOfLabel] + if !found || v != applySetID { + return false + } + delete(labels, ApplySetPartOfLabel) + obj.SetLabels(labels) + return true +} diff --git a/pkg/metadata/metadata_test.go b/pkg/metadata/metadata_test.go index 9a99c2040..8a8467c34 100644 --- a/pkg/metadata/metadata_test.go +++ b/pkg/metadata/metadata_test.go @@ -19,12 +19,15 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "kpt.dev/configsync/pkg/api/configmanagement" "kpt.dev/configsync/pkg/core" "kpt.dev/configsync/pkg/core/k8sobjects" "kpt.dev/configsync/pkg/kinds" "kpt.dev/configsync/pkg/metadata" "kpt.dev/configsync/pkg/syncer/syncertest" + "sigs.k8s.io/cli-utils/pkg/testutil" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -148,3 +151,76 @@ func TestRemoveConfigSyncMetadata(t *testing.T) { t.Errorf("the labels and annotations shouldn't be updated in this case") } } + +func TestRemoveApplySetPartOfLabel(t *testing.T) { + tests := []struct { + name string + obj client.Object + applySetID string + wantUpdated bool + wantObj client.Object + }{ + { + name: "noop no labels", + obj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Annotation(metadata.OwningInventoryKey, "random-value")), + applySetID: "example", + wantUpdated: false, + wantObj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Annotation(metadata.OwningInventoryKey, "random-value")), + }, + { + name: "noop no key match", + obj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Annotation(metadata.OwningInventoryKey, "random-value"), + core.Label(metadata.SystemLabel, "random-value")), + applySetID: "example", + wantUpdated: false, + wantObj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Annotation(metadata.OwningInventoryKey, "random-value"), + core.Label(metadata.SystemLabel, "random-value")), + }, + { + name: "noop no value match", + obj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Annotation(metadata.OwningInventoryKey, "random-value"), + core.Label(metadata.SystemLabel, "random-value"), + core.Label(metadata.ApplySetPartOfLabel, "example")), + applySetID: "example-2", + wantUpdated: false, + wantObj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Annotation(metadata.OwningInventoryKey, "random-value"), + core.Label(metadata.SystemLabel, "random-value"), + core.Label(metadata.ApplySetPartOfLabel, "example")), + }, + { + name: "removal", + obj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Annotation(metadata.OwningInventoryKey, "random-value"), + core.Label(metadata.SystemLabel, "random-value"), + core.Label(metadata.ApplySetPartOfLabel, "example")), + applySetID: "example", + wantUpdated: true, + wantObj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Annotation(metadata.OwningInventoryKey, "random-value"), + core.Label(metadata.SystemLabel, "random-value")), + }, + { + name: "removal to empty", + obj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy"), + core.Label(metadata.ApplySetPartOfLabel, "example")), + applySetID: "example", + wantUpdated: true, + wantObj: k8sobjects.UnstructuredObject(kinds.Deployment(), core.Name("deploy")), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj, err := kinds.ObjectAsClientObject(tt.obj.DeepCopyObject()) + require.NoError(t, err) + updated := metadata.RemoveApplySetPartOfLabel(obj, tt.applySetID) + assert.Equal(t, tt.wantUpdated, updated) + testutil.AssertEqual(t, tt.wantObj, obj) + }) + } +} diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 9cd75e426..e94083085 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -185,8 +185,9 @@ func Run(opts Options) { } // Configure the Applier. + applySetID := applyset.IDFromSync(opts.SyncName, opts.ReconcilerScope) genericClient := syncerclient.New(cl, metrics.APICallDuration) - baseApplier, err := reconcile.NewApplierForMultiRepo(cfg, genericClient) + baseApplier, err := reconcile.NewApplierForMultiRepo(cfg, genericClient, applySetID) if err != nil { klog.Fatalf("Instantiating Applier: %v", err) } @@ -198,7 +199,6 @@ func Run(opts Options) { if reconcileTimeout < 0 { klog.Fatalf("Invalid reconcileTimeout: %v, timeout should not be negative", reconcileTimeout) } - applySetID := applyset.IDFromSync(opts.SyncName, opts.ReconcilerScope) clientSet, err := applier.NewClientSet(cl, configFlags, opts.StatusMode, applySetID) if err != nil { klog.Fatalf("Error creating clients: %v", err) diff --git a/pkg/syncer/reconcile/apply.go b/pkg/syncer/reconcile/apply.go index 0f864e1e9..8a4dd0827 100644 --- a/pkg/syncer/reconcile/apply.go +++ b/pkg/syncer/reconcile/apply.go @@ -69,16 +69,17 @@ type clientApplier struct { openAPIResources openapi.Resources client *syncerclient.Client fights fight.Detector + applySetID string } var _ Applier = &clientApplier{} // NewApplierForMultiRepo returns a new clientApplier for callers with multi repo feature enabled. -func NewApplierForMultiRepo(cfg *rest.Config, client *syncerclient.Client) (Applier, error) { - return newApplier(cfg, client) +func NewApplierForMultiRepo(cfg *rest.Config, client *syncerclient.Client, applySetID string) (Applier, error) { + return newApplier(cfg, client, applySetID) } -func newApplier(cfg *rest.Config, client *syncerclient.Client) (Applier, error) { +func newApplier(cfg *rest.Config, client *syncerclient.Client, applySetID string) (Applier, error) { c, err := dynamic.NewForConfig(cfg) if err != nil { return nil, err @@ -100,6 +101,7 @@ func newApplier(cfg *rest.Config, client *syncerclient.Client) (Applier, error) openAPIResources: oa, client: client, fights: fight.NewDetector(), + applySetID: applySetID, }, nil } @@ -175,7 +177,9 @@ func (c *clientApplier) Update(ctx context.Context, intendedState, currentState func (c *clientApplier) RemoveNomosMeta(ctx context.Context, u *unstructured.Unstructured, controller string) status.Error { var changed bool _, err := c.client.Apply(ctx, u, func(obj client.Object) (client.Object, error) { - changed = metadata.RemoveConfigSyncMetadata(obj) + changed1 := metadata.RemoveConfigSyncMetadata(obj) + changed2 := metadata.RemoveApplySetPartOfLabel(obj, c.applySetID) + changed := changed1 || changed2 if !changed { return obj, syncerclient.NoUpdateNeeded() } diff --git a/pkg/syncer/syncertest/fake/applier.go b/pkg/syncer/syncertest/fake/applier.go index 04ad91426..91a1d091e 100644 --- a/pkg/syncer/syncertest/fake/applier.go +++ b/pkg/syncer/syncertest/fake/applier.go @@ -33,6 +33,7 @@ type Applier struct { CreateError status.Error UpdateError status.Error DeleteError status.Error + ApplySetID string } var _ reconcile.Applier = &Applier{} @@ -63,8 +64,10 @@ func (a *Applier) Update(ctx context.Context, intendedState, _ *unstructured.Uns // RemoveNomosMeta implements reconcile.Applier. func (a *Applier) RemoveNomosMeta(ctx context.Context, intent *unstructured.Unstructured, _ string) status.Error { - updated := metadata.RemoveConfigSyncMetadata(intent) - if !updated { + changed1 := metadata.RemoveConfigSyncMetadata(intent) + changed2 := metadata.RemoveApplySetPartOfLabel(intent, a.ApplySetID) + changed := changed1 || changed2 + if !changed { return nil }