From 8ca6d4a6f285eaf19d715cb5501091a9a44c853b Mon Sep 17 00:00:00 2001 From: Youn Jae Kim Date: Thu, 13 Oct 2022 01:07:28 +0900 Subject: [PATCH 1/3] Added new test --- test/e2e/work_api_e2e_test.go | 237 +++++++++++++++++++++++++++++++++- 1 file changed, 236 insertions(+), 1 deletion(-) diff --git a/test/e2e/work_api_e2e_test.go b/test/e2e/work_api_e2e_test.go index ef11354c3..b6a85faa8 100644 --- a/test/e2e/work_api_e2e_test.go +++ b/test/e2e/work_api_e2e_test.go @@ -44,7 +44,8 @@ var _ = Describe("Work API Controller test", func() { "CreationTimestamp", "Annotations", "OwnerReferences", - "ManagedFields"), + "ManagedFields", + "Labels"), } appliedWorkCmpOptions = append(cmpOptions, cmpopts.IgnoreFields(workapi.AppliedResourceMeta{}, "UID")) @@ -556,6 +557,240 @@ var _ = Describe("Work API Controller test", func() { Expect(customResource.GetAnnotations()[specHashAnnotation]).ToNot(BeEmpty(), "There is no spec annotation on the custom resource %s", customResource.GetName()) }) + + It("Manifests with dependencies within different work objects should successfully apply", func() { + + workNameForNamespace := testutils.RandomWorkName(5) + workNameForServiceAccount := testutils.RandomWorkName(6) + + testNamespace := corev1.Namespace{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Namespace", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + }, + } + // Ns abbreviated to avoid duplicate wording + nsNamespaceType := types.NamespacedName{ + Name: testNamespace.Name, + } + + testServiceAccount := corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ServiceAccount", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service-account", + Namespace: testNamespace.Name, + }, + } + serviceAccountNamespaceType := types.NamespacedName{ + Name: testServiceAccount.Name, + Namespace: testNamespace.Name, + } + + namespaceTypeForNamespace := types.NamespacedName{Name: workNameForNamespace, Namespace: workNamespace.Name} + namespaceTypeForServiceAccount := types.NamespacedName{Name: workNameForServiceAccount, Namespace: workNamespace.Name} + + manifestNamespace := testutils.AddManifests([]runtime.Object{&testNamespace}, []workapi.Manifest{}) + manifestServiceAccount := testutils.AddManifests([]runtime.Object{&testServiceAccount}, []workapi.Manifest{}) + + workForNamespace := testutils.CreateWork(ctx, *HubCluster, workNameForNamespace, workNamespace.Name, manifestNamespace) + workForServiceAccount := testutils.CreateWork(ctx, *HubCluster, workNameForServiceAccount, workNamespace.Name, manifestServiceAccount) + + By(fmt.Sprintf("Applied Condition should be set to True for Work %s and %s", namespaceTypeForNamespace, namespaceTypeForServiceAccount)) + + wantAppliedCondition := []metav1.Condition{ + { + Type: conditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: "appliedWorkComplete", + }, + } + + receivedWorkForNamespace := workapi.Work{} + receivedWorkForServiceAccount := workapi.Work{} + + Eventually(func() string { + if err := HubCluster.KubeClient.Get(ctx, namespaceTypeForNamespace, &receivedWorkForNamespace); err != nil { + return err.Error() + } + + return cmp.Diff(wantAppliedCondition, receivedWorkForNamespace.Status.Conditions, cmpOptions...) + }, testutils.PollTimeout, testutils.PollInterval).Should(BeEmpty(), + "Validate WorkStatus for work %s mismatch (-want, +got):", workForNamespace) + + Eventually(func() string { + if err := HubCluster.KubeClient.Get(ctx, namespaceTypeForServiceAccount, &receivedWorkForServiceAccount); err != nil { + return err.Error() + } + + return cmp.Diff(wantAppliedCondition, receivedWorkForServiceAccount.Status.Conditions, cmpOptions...) + }, testutils.PollTimeout, testutils.PollInterval).Should(BeEmpty(), + "Validate WorkStatus for work %s mismatch (-want, +got):", workForServiceAccount) + + By(fmt.Sprintf("Manifest Condiitons on Work Objects %s and %s should be applied", namespaceTypeForNamespace, namespaceTypeForServiceAccount)) + wantNamespaceManifestCondition := []workapi.ManifestCondition{ + { + Conditions: []metav1.Condition{ + { + Type: conditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: "appliedManifestUpdated", + }, + }, + Identifier: workapi.ResourceIdentifier{ + Group: testNamespace.GroupVersionKind().Group, + Version: testNamespace.GroupVersionKind().Version, + Kind: testNamespace.GroupVersionKind().Kind, + Namespace: testNamespace.Namespace, + Name: testNamespace.Name, + Resource: "namespaces", + }, + }, + } + wantServiceAccountManifestCondition := []workapi.ManifestCondition{ + { + Conditions: []metav1.Condition{ + { + Type: conditionTypeApplied, + Status: metav1.ConditionTrue, + Reason: "appliedManifestUpdated", + }, + }, + Identifier: workapi.ResourceIdentifier{ + Group: testServiceAccount.GroupVersionKind().Group, + Version: testServiceAccount.GroupVersionKind().Version, + Kind: testServiceAccount.GroupVersionKind().Kind, + Namespace: testServiceAccount.Namespace, + Name: testServiceAccount.Name, + Resource: "serviceaccounts", + }, + }, + } + + Expect(cmp.Diff(wantNamespaceManifestCondition, receivedWorkForNamespace.Status.ManifestConditions, cmpOptions...)).Should(BeEmpty(), + "Manifest Condition not matching for work %s (-want, +got):", namespaceTypeForNamespace) + + Expect(cmp.Diff(wantServiceAccountManifestCondition, receivedWorkForServiceAccount.Status.ManifestConditions, cmpOptions...)).Should(BeEmpty(), + "Manifest Condition not matching for work %s (-want, +got):", namespaceTypeForServiceAccount) + + By(fmt.Sprintf("AppliedWorkStatus should contain the meta for the resource %s and %s", testNamespace.Name, testServiceAccount.Name)) + appliedWorkForNamespace := workapi.AppliedWork{} + Expect(MemberCluster.KubeClient.Get(ctx, namespaceTypeForNamespace, &appliedWorkForNamespace)).Should(Succeed(), + "Retrieving AppliedWork %s failed", workNameForNamespace) + + wantAppliedWorkConditionNamespace := workapi.AppliedtWorkStatus{ + AppliedResources: []workapi.AppliedResourceMeta{ + { + ResourceIdentifier: workapi.ResourceIdentifier{ + Group: testNamespace.GroupVersionKind().Group, + Version: testNamespace.GroupVersionKind().Version, + Kind: testNamespace.GroupVersionKind().Kind, + Namespace: testNamespace.Namespace, + Name: testNamespace.Name, + Resource: "namespaces", + }, + }, + }, + } + + Expect(cmp.Diff(wantAppliedWorkConditionNamespace, appliedWorkForNamespace.Status, appliedWorkCmpOptions...)).Should(BeEmpty(), + "Validate AppliedResourceMeta mismatch for appliedWork %s (-want, +got):", appliedWorkForNamespace.Name) + + appliedWorkForServiceAccount := workapi.AppliedWork{} + Expect(MemberCluster.KubeClient.Get(ctx, namespaceTypeForServiceAccount, &appliedWorkForServiceAccount)).Should(Succeed(), + "Retrieving AppliedWork %s failed", workNameForServiceAccount) + + wantAppliedConditionServiceAccount := workapi.AppliedtWorkStatus{ + AppliedResources: []workapi.AppliedResourceMeta{ + { + ResourceIdentifier: workapi.ResourceIdentifier{ + Group: testServiceAccount.GroupVersionKind().Group, + Version: testServiceAccount.GroupVersionKind().Version, + Kind: testServiceAccount.GroupVersionKind().Kind, + Namespace: testServiceAccount.Namespace, + Name: testServiceAccount.Name, + Resource: "serviceaccounts", + }, + }, + }, + } + + Expect(cmp.Diff(wantAppliedConditionServiceAccount, appliedWorkForServiceAccount.Status, appliedWorkCmpOptions...)).Should(BeEmpty(), + "Validate AppliedResourceMeta mismatch for appliedWork %s (-want, +got):", appliedWorkForServiceAccount.Name) + + By(fmt.Sprintf("The resources %s and %s should both be created in the member cluster.", testNamespace.Name, testServiceAccount.Name)) + retrievedNamespace := corev1.Namespace{} + Expect(MemberCluster.KubeClient.Get(ctx, nsNamespaceType, &retrievedNamespace)).Should(Succeed(), + "Failed in retrieving resource %s", testNamespace) + wantNamespace := corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + }, + } + + namespaceCmpOptions := append(cmpOptions, + cmpopts.IgnoreFields(corev1.Namespace{}, "Spec", "Status")) + + Expect(cmp.Diff(wantNamespace, retrievedNamespace, namespaceCmpOptions...)).Should(BeEmpty(), + "Validate Namespace %s mismatch (-want, +got):", wantNamespace.Name) + + retrievedServiceAccount := corev1.ServiceAccount{} + Expect(MemberCluster.KubeClient.Get(ctx, serviceAccountNamespaceType, &retrievedServiceAccount)).Should(Succeed(), + "Failed in retrieving resource %s", testServiceAccount) + + wantServiceAccount := corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ServiceAccount", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service-account", + Namespace: testNamespace.Name, + }, + } + + serviceAccountCmpOptions := append(cmpOptions, + cmpopts.IgnoreFields(corev1.ServiceAccount{}, "TypeMeta", "Secrets")) + + Expect(cmp.Diff(wantServiceAccount, retrievedServiceAccount, serviceAccountCmpOptions...)).Should(BeEmpty(), + "Validate Service Account %s mismatch (-want, +got):", wantServiceAccount.Name) + + By(fmt.Sprintf("Validating that the resource %s and %s is owned by the respective work", testNamespace.Name, testServiceAccount.Name)) + wantOwnerForNamespace := []metav1.OwnerReference{ + { + APIVersion: workapi.GroupVersion.String(), + Kind: workapi.AppliedWorkKind, + Name: appliedWorkForNamespace.GetName(), + UID: appliedWorkForNamespace.GetUID(), + }, + } + + wantOwnerForServiceAccount := []metav1.OwnerReference{ + { + APIVersion: workapi.GroupVersion.String(), + Kind: workapi.AppliedWorkKind, + Name: appliedWorkForServiceAccount.GetName(), + UID: appliedWorkForServiceAccount.GetUID(), + }, + } + + Expect(cmp.Diff(wantOwnerForNamespace, retrievedNamespace.OwnerReferences, cmpOptions...)).Should(BeEmpty(), + "OwnerReference mismatch for resource %s (-want, +got):", testNamespace.Name) + Expect(cmp.Diff(wantOwnerForServiceAccount, retrievedServiceAccount.OwnerReferences, cmpOptions...)).Should(BeEmpty(), + "OwnerReference mismatch for resource %s (-want, +got):", testServiceAccount.Name) + + // TODO: spec has isn't being created when resources such as namespace and service account is being created by the work-api. Possible bug? + //Expect(testNamespace.ObjectMeta.Annotations[specHashAnnotation]).ToNot(BeEmpty(), + // "SpecHash Annotation does not exist for resource %s", testNamespace.Name) + //Expect(testServiceAccount.ObjectMeta.Annotations[specHashAnnotation]).ToNot(BeEmpty(), + // "SpecHash Annotation does not exist for resource %s", testServiceAccount.Name) + + }) }) Context("Updating Work", func() { From a63694141cda33107e8aea912a79360cf957ed31 Mon Sep 17 00:00:00 2001 From: Youn Jae Kim Date: Thu, 13 Oct 2022 01:45:31 +0900 Subject: [PATCH 2/3] Removed unnecessary comment --- test/e2e/work_api_e2e_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/e2e/work_api_e2e_test.go b/test/e2e/work_api_e2e_test.go index b6a85faa8..23333a0c1 100644 --- a/test/e2e/work_api_e2e_test.go +++ b/test/e2e/work_api_e2e_test.go @@ -783,13 +783,6 @@ var _ = Describe("Work API Controller test", func() { "OwnerReference mismatch for resource %s (-want, +got):", testNamespace.Name) Expect(cmp.Diff(wantOwnerForServiceAccount, retrievedServiceAccount.OwnerReferences, cmpOptions...)).Should(BeEmpty(), "OwnerReference mismatch for resource %s (-want, +got):", testServiceAccount.Name) - - // TODO: spec has isn't being created when resources such as namespace and service account is being created by the work-api. Possible bug? - //Expect(testNamespace.ObjectMeta.Annotations[specHashAnnotation]).ToNot(BeEmpty(), - // "SpecHash Annotation does not exist for resource %s", testNamespace.Name) - //Expect(testServiceAccount.ObjectMeta.Annotations[specHashAnnotation]).ToNot(BeEmpty(), - // "SpecHash Annotation does not exist for resource %s", testServiceAccount.Name) - }) }) From 3927b7db1904054ef174346ab2bacaaf1dc8452a Mon Sep 17 00:00:00 2001 From: Youn Jae Kim Date: Thu, 13 Oct 2022 17:34:27 +0900 Subject: [PATCH 3/3] fixes based on comments --- test/e2e/work_api_e2e_test.go | 64 +++++++++++++++-------------------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/test/e2e/work_api_e2e_test.go b/test/e2e/work_api_e2e_test.go index 23333a0c1..d1a9312bf 100644 --- a/test/e2e/work_api_e2e_test.go +++ b/test/e2e/work_api_e2e_test.go @@ -572,10 +572,6 @@ var _ = Describe("Work API Controller test", func() { Name: "test-namespace", }, } - // Ns abbreviated to avoid duplicate wording - nsNamespaceType := types.NamespacedName{ - Name: testNamespace.Name, - } testServiceAccount := corev1.ServiceAccount{ TypeMeta: metav1.TypeMeta{ @@ -587,13 +583,6 @@ var _ = Describe("Work API Controller test", func() { Namespace: testNamespace.Name, }, } - serviceAccountNamespaceType := types.NamespacedName{ - Name: testServiceAccount.Name, - Namespace: testNamespace.Name, - } - - namespaceTypeForNamespace := types.NamespacedName{Name: workNameForNamespace, Namespace: workNamespace.Name} - namespaceTypeForServiceAccount := types.NamespacedName{Name: workNameForServiceAccount, Namespace: workNamespace.Name} manifestNamespace := testutils.AddManifests([]runtime.Object{&testNamespace}, []workapi.Manifest{}) manifestServiceAccount := testutils.AddManifests([]runtime.Object{&testServiceAccount}, []workapi.Manifest{}) @@ -601,7 +590,7 @@ var _ = Describe("Work API Controller test", func() { workForNamespace := testutils.CreateWork(ctx, *HubCluster, workNameForNamespace, workNamespace.Name, manifestNamespace) workForServiceAccount := testutils.CreateWork(ctx, *HubCluster, workNameForServiceAccount, workNamespace.Name, manifestServiceAccount) - By(fmt.Sprintf("Applied Condition should be set to True for Work %s and %s", namespaceTypeForNamespace, namespaceTypeForServiceAccount)) + By(fmt.Sprintf("Applied Condition should be set to True for Work %s and %s", workNameForNamespace, workNameForServiceAccount)) wantAppliedCondition := []metav1.Condition{ { @@ -614,8 +603,10 @@ var _ = Describe("Work API Controller test", func() { receivedWorkForNamespace := workapi.Work{} receivedWorkForServiceAccount := workapi.Work{} + namespaceTypeForNamespaceWork := types.NamespacedName{Name: workNameForNamespace, Namespace: workNamespace.Name} + Eventually(func() string { - if err := HubCluster.KubeClient.Get(ctx, namespaceTypeForNamespace, &receivedWorkForNamespace); err != nil { + if err := HubCluster.KubeClient.Get(ctx, namespaceTypeForNamespaceWork, &receivedWorkForNamespace); err != nil { return err.Error() } @@ -623,8 +614,10 @@ var _ = Describe("Work API Controller test", func() { }, testutils.PollTimeout, testutils.PollInterval).Should(BeEmpty(), "Validate WorkStatus for work %s mismatch (-want, +got):", workForNamespace) + namespaceTypeForServiceAccountWork := types.NamespacedName{Name: workNameForServiceAccount, Namespace: workNamespace.Name} + Eventually(func() string { - if err := HubCluster.KubeClient.Get(ctx, namespaceTypeForServiceAccount, &receivedWorkForServiceAccount); err != nil { + if err := HubCluster.KubeClient.Get(ctx, namespaceTypeForServiceAccountWork, &receivedWorkForServiceAccount); err != nil { return err.Error() } @@ -632,7 +625,7 @@ var _ = Describe("Work API Controller test", func() { }, testutils.PollTimeout, testutils.PollInterval).Should(BeEmpty(), "Validate WorkStatus for work %s mismatch (-want, +got):", workForServiceAccount) - By(fmt.Sprintf("Manifest Condiitons on Work Objects %s and %s should be applied", namespaceTypeForNamespace, namespaceTypeForServiceAccount)) + By(fmt.Sprintf("Manifest Condiitons on Work Objects %s and %s should be applied", namespaceTypeForNamespaceWork, namespaceTypeForServiceAccountWork)) wantNamespaceManifestCondition := []workapi.ManifestCondition{ { Conditions: []metav1.Condition{ @@ -673,14 +666,14 @@ var _ = Describe("Work API Controller test", func() { } Expect(cmp.Diff(wantNamespaceManifestCondition, receivedWorkForNamespace.Status.ManifestConditions, cmpOptions...)).Should(BeEmpty(), - "Manifest Condition not matching for work %s (-want, +got):", namespaceTypeForNamespace) + "Manifest Condition not matching for work %s (-want, +got):", namespaceTypeForNamespaceWork) Expect(cmp.Diff(wantServiceAccountManifestCondition, receivedWorkForServiceAccount.Status.ManifestConditions, cmpOptions...)).Should(BeEmpty(), - "Manifest Condition not matching for work %s (-want, +got):", namespaceTypeForServiceAccount) + "Manifest Condition not matching for work %s (-want, +got):", namespaceTypeForServiceAccountWork) By(fmt.Sprintf("AppliedWorkStatus should contain the meta for the resource %s and %s", testNamespace.Name, testServiceAccount.Name)) appliedWorkForNamespace := workapi.AppliedWork{} - Expect(MemberCluster.KubeClient.Get(ctx, namespaceTypeForNamespace, &appliedWorkForNamespace)).Should(Succeed(), + Expect(MemberCluster.KubeClient.Get(ctx, namespaceTypeForNamespaceWork, &appliedWorkForNamespace)).Should(Succeed(), "Retrieving AppliedWork %s failed", workNameForNamespace) wantAppliedWorkConditionNamespace := workapi.AppliedtWorkStatus{ @@ -702,7 +695,7 @@ var _ = Describe("Work API Controller test", func() { "Validate AppliedResourceMeta mismatch for appliedWork %s (-want, +got):", appliedWorkForNamespace.Name) appliedWorkForServiceAccount := workapi.AppliedWork{} - Expect(MemberCluster.KubeClient.Get(ctx, namespaceTypeForServiceAccount, &appliedWorkForServiceAccount)).Should(Succeed(), + Expect(MemberCluster.KubeClient.Get(ctx, namespaceTypeForServiceAccountWork, &appliedWorkForServiceAccount)).Should(Succeed(), "Retrieving AppliedWork %s failed", workNameForServiceAccount) wantAppliedConditionServiceAccount := workapi.AppliedtWorkStatus{ @@ -725,41 +718,38 @@ var _ = Describe("Work API Controller test", func() { By(fmt.Sprintf("The resources %s and %s should both be created in the member cluster.", testNamespace.Name, testServiceAccount.Name)) retrievedNamespace := corev1.Namespace{} + + // Ns abbreviated to avoid duplicate wording + nsNamespaceType := types.NamespacedName{ + Name: testNamespace.Name, + } + Expect(MemberCluster.KubeClient.Get(ctx, nsNamespaceType, &retrievedNamespace)).Should(Succeed(), "Failed in retrieving resource %s", testNamespace) wantNamespace := corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "test-namespace", }, + Status: corev1.NamespaceStatus{ + Phase: corev1.NamespaceActive, + }, } namespaceCmpOptions := append(cmpOptions, - cmpopts.IgnoreFields(corev1.Namespace{}, "Spec", "Status")) + cmpopts.IgnoreFields(corev1.Namespace{}, "Spec")) Expect(cmp.Diff(wantNamespace, retrievedNamespace, namespaceCmpOptions...)).Should(BeEmpty(), "Validate Namespace %s mismatch (-want, +got):", wantNamespace.Name) + serviceAccountNamespaceType := types.NamespacedName{ + Name: testServiceAccount.Name, + Namespace: testNamespace.Name, + } + retrievedServiceAccount := corev1.ServiceAccount{} Expect(MemberCluster.KubeClient.Get(ctx, serviceAccountNamespaceType, &retrievedServiceAccount)).Should(Succeed(), "Failed in retrieving resource %s", testServiceAccount) - wantServiceAccount := corev1.ServiceAccount{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "ServiceAccount", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-service-account", - Namespace: testNamespace.Name, - }, - } - - serviceAccountCmpOptions := append(cmpOptions, - cmpopts.IgnoreFields(corev1.ServiceAccount{}, "TypeMeta", "Secrets")) - - Expect(cmp.Diff(wantServiceAccount, retrievedServiceAccount, serviceAccountCmpOptions...)).Should(BeEmpty(), - "Validate Service Account %s mismatch (-want, +got):", wantServiceAccount.Name) - By(fmt.Sprintf("Validating that the resource %s and %s is owned by the respective work", testNamespace.Name, testServiceAccount.Name)) wantOwnerForNamespace := []metav1.OwnerReference{ {