test: Workload e2e #235

Arvindthiru · 2022-08-16T04:20:45Z

Description of your changes

Fixes #

I have:

Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

test/e2e/join_leave_member_test.go

test/e2e/work_load_test.go

test/e2e/join_leave_member_test.go

test/e2e/work_load_test.go

Arvindthiru · 2022-08-18T00:48:49Z

test/e2e/work_load_test.go

+	var crp *v1alpha1.ClusterResourcePlacement
+
+	BeforeEach(func() {
+		memberNS = NewNamespace(fmt.Sprintf(utils.NamespaceNameFormat, MemberCluster.ClusterName))


This needs to be setup here because once we create namespace resource version gets added, this resets it once we delete between tests.

ryanzhang-oss · 2022-08-18T05:24:44Z

test/e2e/framework/customresources.go

+// CreateClusterRole create cluster role in the hub cluster.
+func CreateClusterRole(cluster Cluster, cr *rbacv1.ClusterRole) {
+	ginkgo.By(fmt.Sprintf("Creating ClusterRole (%s)", cr.Name), func() {
+		err := cluster.KubeClient.Create(context.TODO(), cr)
+		gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+	})
+}
+
+// WaitClusterRole waits for cluster roles to be created.
+func WaitClusterRole(cluster Cluster, cr *rbacv1.ClusterRole) {
+	klog.Infof("Waiting for ClusterRole(%s) to be synced", cr.Name)
+	gomega.Eventually(func() error {
+		err := cluster.KubeClient.Get(context.TODO(), types.NamespacedName{Name: cr.Name, Namespace: ""}, cr)
+		return err
+	}, PollTimeout, PollInterval).ShouldNot(gomega.HaveOccurred())
+}
+
+// DeleteClusterRole deletes cluster role on cluster.
+func DeleteClusterRole(cluster Cluster, cr *rbacv1.ClusterRole) {
+	ginkgo.By(fmt.Sprintf("Deleting ClusterRole(%s)", cr.Name), func() {
+		err := cluster.KubeClient.Delete(context.TODO(), cr)
+		gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+	})
+}
+


why is clusterRole part of the common file?

Moved to test/e2e/utils/helper.go

ryanzhang-oss · 2022-08-18T05:25:28Z

test/e2e/join_leave_member_test.go

 	"fmt"

 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"golang.org/x/net/context"
+	"go.goms.io/fleet/pkg/utils"


this should be with the local group

ryanzhang-oss · 2022-08-18T05:26:13Z

test/e2e/join_leave_member_test.go

-					return apierrors.IsNotFound(err)
-				}, framework.PollTimeout, framework.PollInterval).Should(Equal(true))
-			})
+		AfterEach(func() {


normally, by convention, the AfterEach is put in the front. It's like defer

Moved it below BeforeEach

ryanzhang-oss · 2022-08-18T05:27:39Z

test/e2e/join_leave_member_test.go

+			framework.CreateNamespace(*MemberCluster, memberNS)
+			framework.WaitNamespace(*MemberCluster, memberNS)


consider group them together?

ryanzhang-oss · 2022-08-18T05:37:14Z

test/e2e/join_leave_member_test.go

+			framework.CreateMemberCluster(*HubCluster, mc)
+			framework.WaitMemberCluster(*HubCluster, mc)


ryanzhang-oss · 2022-08-18T05:37:41Z

test/e2e/join_leave_member_test.go

+			By("check if member cluster is marked as readyToJoin")
+			framework.WaitConditionMemberCluster(*HubCluster, mc, v1alpha1.ConditionTypeMemberClusterReadyToJoin, v1.ConditionTrue, 3*framework.PollTimeout)
+
+			By("check if internal member cluster created in the hub cluster")
+			imc = NewInternalMemberCluster(MemberCluster.ClusterName, memberNS.Name)
+			framework.WaitInternalMemberCluster(*HubCluster, imc)


shouldn't the order be reversed?

ryanzhang-oss · 2022-08-18T05:42:53Z

test/e2e/join_leave_member_test.go

+			By("check if internal member cluster condition is updated to Joined")
+			framework.WaitConditionInternalMemberCluster(*HubCluster, imc, v1alpha1.AgentJoined, v1.ConditionTrue, 3*framework.PollTimeout)

+			By("check if member cluster condition is updated to Joined")
+			framework.WaitConditionMemberCluster(*HubCluster, mc, v1alpha1.ConditionTypeMemberClusterJoin, v1.ConditionTrue, 3*framework.PollTimeout)


at this point, this IT has only 2 things, why not either fold it into beforeEach or merge with the leave test

ryanzhang-oss · 2022-08-18T05:49:15Z

test/e2e/work_load_test.go

+		cr = &rbacv1.ClusterRole{
+			ObjectMeta: v1.ObjectMeta{
+				Name:   "test-cluster-role",
+				Labels: map[string]string{"fleet.azure.com/name": "test"},


this label should be extract as a var as it's used by the CRP

Added labelKey and labelValue vars

ryanzhang-oss · 2022-08-18T18:36:17Z

test/e2e/join_leave_member_test.go

+	AfterEach(func() {
+		framework.DeleteNamespace(*MemberCluster, memberNS)
+		Eventually(func() bool {
+			err := MemberCluster.KubeClient.Get(context.TODO(), types.NamespacedName{Name: memberNS.Name, Namespace: ""}, memberNS)
+			return apierrors.IsNotFound(err)
+		}, framework.PollTimeout, framework.PollInterval).Should(Equal(true))
+		framework.DeleteMemberCluster(*HubCluster, mc)
+		Eventually(func() bool {
+			err := HubCluster.KubeClient.Get(context.TODO(), types.NamespacedName{Name: memberNS.Name, Namespace: ""}, memberNS)
+			return apierrors.IsNotFound(err)
+		}, framework.PollTimeout, framework.PollInterval).Should(Equal(true))


can you move afterEach block to the top?

Moved it to the top

ryanzhang-oss · 2022-08-18T18:36:35Z

test/e2e/work_load_test.go


 		By("check if internal member cluster created in the hub cluster")
-		imc = NewInternalMemberCluster(MemberCluster.ClusterName, memberNS.Name)
+		imc = &v1alpha1.InternalMemberCluster{


why make this change?

Reverted back

github-actions bot added the title-needs-formatting label Aug 16, 2022

Arvindthiru changed the title ~~Workload e2e~~ test: Workload e2e Aug 16, 2022

github-actions bot removed the title-needs-formatting label Aug 16, 2022

ryanzhang-oss reviewed Aug 16, 2022

View reviewed changes

test/e2e/join_leave_member_test.go Outdated Show resolved Hide resolved

ryanzhang-oss reviewed Aug 16, 2022

View reviewed changes

test/e2e/work_load_test.go Outdated Show resolved Hide resolved

test/e2e/work_load_test.go Outdated Show resolved Hide resolved

Arvindthiru marked this pull request as ready for review August 16, 2022 20:43

aagusuab reviewed Aug 16, 2022

View reviewed changes

test/e2e/work_load_test.go Outdated Show resolved Hide resolved

aagusuab reviewed Aug 16, 2022

View reviewed changes

test/e2e/work_load_test.go Outdated Show resolved Hide resolved

aagusuab reviewed Aug 16, 2022

View reviewed changes

test/e2e/work_load_test.go Outdated Show resolved Hide resolved

ryanzhang-oss reviewed Aug 17, 2022

View reviewed changes

test/e2e/work_load_test.go Outdated Show resolved Hide resolved

test/e2e/work_load_test.go Outdated Show resolved Hide resolved

Arvindthiru added 12 commits August 17, 2022 13:18

Workload e2e

b8d225c

Initial working e2e

d52ab19

Wait for work

f322f25

add afterEach

312d038

e2e Fix

1c3212c

Fix import

c10925f

rearrange methods

1afb291

Fix comments

c9d50b4

Address comments

5dda0cf

Fix By node text

a0ee701

Address comments

2ee2bb6

Address comments

54cad6e

Arvindthiru force-pushed the WorkloadE2E branch from 60daed6 to 54cad6e Compare August 17, 2022 21:20

Arvindthiru added 2 commits August 17, 2022 14:21

Fix import

44f4ed8

Remove DeferCleanUp

5b9e598

Arvindthiru commented Aug 18, 2022

View reviewed changes

test/e2e/join_leave_member_test.go Show resolved Hide resolved

Arvindthiru commented Aug 18, 2022

View reviewed changes

test/e2e/work_load_test.go Show resolved Hide resolved

Arvindthiru added 2 commits August 17, 2022 17:45

Revert changes to makefile

b6707d5

Fix

a0f779d

Arvindthiru commented Aug 18, 2022

View reviewed changes

ryanzhang-oss reviewed Aug 18, 2022

View reviewed changes

Address comments

fdaab1e

ryanzhang-oss reviewed Aug 18, 2022

View reviewed changes

Arvindthiru added 4 commits August 18, 2022 19:58

Address comments

4a9cd2c

Fix

6946671

Add vars for label

6e0a6bb

static check fix

9d70cb8

ryanzhang-oss approved these changes Aug 19, 2022

View reviewed changes

ryanzhang-oss merged commit 2933825 into Azure:main Aug 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Workload e2e #235

test: Workload e2e #235

Arvindthiru commented Aug 16, 2022

Arvindthiru Aug 18, 2022 •

edited

Loading

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 19, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 18, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 19, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 18, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 18, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 18, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 18, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 19, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 19, 2022

ryanzhang-oss Aug 18, 2022

Arvindthiru Aug 19, 2022

		framework.CreateNamespace(*MemberCluster, memberNS)
		framework.WaitNamespace(*MemberCluster, memberNS)

		framework.CreateMemberCluster(*HubCluster, mc)
		framework.WaitMemberCluster(*HubCluster, mc)

test: Workload e2e #235

test: Workload e2e #235

Conversation

Arvindthiru commented Aug 16, 2022

Description of your changes

How has this code been tested

Special notes for your reviewer

Arvindthiru Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Arvindthiru Aug 18, 2022 •

edited

Loading