Cloudp-80268 e2e initial #84

leo-ri · 2021-02-03T14:12:34Z

part 1: base test
flaky parts:
a) project creating (probably fixed)
b) sometimes 35min is not enough for updating cluster (at least in qa environment, increased to 50 min(?))

what is not ready:
configurations table

antonlisovenko

Great work! 🥇 The main suggestion is around the test logic (check for the resource status.Ready) field.

We can discuss the cluster configuration part with the team later!

antonlisovenko · 2021-02-05T09:17:08Z

.github/workflows/test.yml


+  prepare-e2e:
+    name: Prepare E2E configuration and image
+    needs: [unit-test, int-test]


can we please remove these and just run the test in parallel with the others?

agree with int-test as they take 20 min. I think it makes sense to wait for unit-test as it take a min, but will avoid an unnecessary run of e2e in case of error.

antonlisovenko · 2021-02-05T09:19:06Z

.github/workflows/test.yml

+      id: prepare
+      uses: ./.github/actions/set-tag
+
+    - name: Push Atlas Operator to Registry


It seems, that if we push the dev image here - there's no need in the "Publish image to Registry" workflow?

I removed push trigger from build image workflow, but I would like to leave this workflow for a while until I am sure that e2e are stable

antonlisovenko · 2021-02-05T09:24:30Z

.github/workflows/test.yml

+    # run if platform = kind #TODO
+    - name: Create k8s Kind Cluster
+      if: ${{ steps.properties.outputs.k8s_platform == 'kind' && !env.ACT }}
+      uses: helm/kind-action@v1.1.0


good to know there is an action for Kind already!

.github/workflows/test.yml

antonlisovenko · 2021-02-05T09:33:40Z

test/e2e/e2e_suite_test.go

+	RunSpecs(t, "E2e Suite")
+}
+
+var _ = SynchronizedBeforeSuite(func() []byte {


[q] why do we need the SynchronizedBeforeSuite instead of the BeforeSuite?

this check run once instead of each time
UPD. move checks to beforeSuite

antonlisovenko · 2021-02-05T09:41:51Z

test/e2e/deploy_test.go

+
+		By("Wait creating and check that it was created")
+		Eventually(cli.GetGeneration(namespaceUserResources)).Should(Equal("1"))
+		Eventually(


after the Project is ready - then the Atlas is expected to have (not eventually) the project created

I use exec command inside , eventually here means : this command eventually finish with exit 0 (there is a wait() inside) . probably need to discuss

antonlisovenko · 2021-02-05T09:44:07Z

test/e2e/deploy_test.go

+			"7m", "10s",
+		).Should(BeTrue())
+
+		projectID := cli.GetProjectID(userProjectConfig.Spec.Name)


I'd recommend to get the project id from the AtlasProject.status.id instead of the mongocli. This will make sure that the Operator writes the correct id to the status

hm... it probably could fix this:
a) project creating

antonlisovenko · 2021-02-05T09:48:06Z

test/e2e/deploy_test.go

+		GinkgoWriter.Write([]byte("projectID = " + projectID))
+
+		Eventually(
+			cli.IsClusterExist(projectID, userClusterConfig.Spec.Name),


the same as above: I'd recommend to check for the "status.conditions.Ready=true" instead of checking Atlas. This will make sure the status is updated correctly.

Basically the logic should be: wait for the AtlasCluster to get Ready:true, then make the request to Atlas to check if the cluster created there matches the one that we have just created

antonlisovenko · 2021-02-05T09:50:22Z

test/e2e/utils/cli.go

+}
+
+// GetPodStatus TODO move
+func GetPodStatus(ns string) func() string {


What do you think about splitting the helper functions for mongocli and kubectl to separate packages? Then we'd import them with the appropriate names and it would be clear from the calling code what is happening under the hood

that s why I added "TODO move" )

antonlisovenko · 2021-02-05T09:54:03Z

test/e2e/deploy_test.go

+
+		By("Wait creation")
+		userClusterConfig = cli.LoadUserClusterConfig("data/updated_atlascluster_basic.yaml")
+		Eventually(cli.GetGeneration(namespaceUserResources)).Should(Equal("2"))


The same as for the create logic the update the code should wait for two things:

the observed generation is equal to the generation field (it's ok to check for "2" directly as well)

the "status.conditions.Ready : true"

antonlisovenko

Reviewed p2p

antonlisovenko · 2021-02-08T16:43:35Z

test/e2e/configuration_test.go

+		Eventually(
+			cli.IsProjectExist(userProjectConfig.Spec.Name),
+			"7m", "10s",
+		).Should(BeTrue())


as discussed p2p: eventually is not necessary - also we don't need to find by name - try using the 'status.ID` from from the AtlasProject

and cosmetic changes to GetStatus

antonlisovenko · 2021-02-08T16:48:13Z

test/e2e/configuration_test.go

+			"7m", "10s",
+		).Should(BeTrue())
+
+		projectID := cli.GetProjectID(userProjectConfig.Spec.Name)


not necessary - use status.ID

antonlisovenko · 2021-02-08T16:49:56Z

test/e2e/utils/mongocli.go

+	}
+}
+
+func GetClusterStatus(projectID string, clusterName string) func() string {


[nit] rename to StateName?

antonlisovenko · 2021-02-08T16:51:00Z

test/e2e/configuration_test.go

+
+		projectID := cli.GetProjectID(userProjectConfig.Spec.Name)
+		GinkgoWriter.Write([]byte("projectID = " + projectID))
+		Eventually(


add the wait for AtlasCluster k8s resource ready

antonlisovenko · 2021-02-08T16:53:21Z

test/e2e/configuration_test.go

+			"5m", "1s",
+		).Should(BeTrue())
+
+		Eventually(


the same as for the project: after the AtlasCluster is ready in K8s - there is no need in eventually - we can just read the Cluster from Atlas directly and use Expect.. equal

antonlisovenko · 2021-02-08T16:55:05Z

test/e2e/configuration_test.go

+		session = cli.Execute("kubectl", "apply", "-f", ClusterSampleFile, "-n", namespaceUserResources) // TODO param
+		Eventually(session.Wait()).Should(Say("atlascluster-sample configured"))
+
+		By("Wait creation")


the same as for create

test/e2e/configuration_test.go

antonlisovenko

Just a couple of nits - mostly around namings and code formatting. I believe after committing there will be no need in waiting for the tests.

Great work! 🥇

antonlisovenko · 2021-02-09T09:40:54Z

.github/workflows/test.yml

+        MCLI_PRIVATE_API_KEY: ${{ secrets.ATLAS_PRIVATE_KEY }}
+        MCLI_ORG_ID: ${{ secrets.ATLAS_ORG_ID}}
+        MCLI_OPS_MANAGER_URL: "https://cloud-qa.mongodb.com/"
+        K8s_PLATFORM: "${{ steps.properties.outputs.k8s_platform }}"


[nit] K8S_PLATFORM and K8S_VERSION (uppercase)

antonlisovenko · 2021-02-09T09:44:33Z

test/e2e/configuration_test.go

+	})
+
+	It("Release sample all-in-one.yaml should work", func() {
+		By("Prepare namespaces and project configuration") // TODO clusters/keys will be a bit later


[nit] still think that considering the size of the test having By with callbacks (By("", func() {..})) will make the code more readable, WDYT?

the thing is idk which steps will be final, this test - is the initial version, steps will be reformatted later for all tests need

antonlisovenko · 2021-02-09T09:47:27Z

test/e2e/configuration_test.go

+		Eventually(session.Wait()).Should(Say("created"))
+
+		By("Wait project creation")
+		Eventually(cli.GetStatus(namespaceUserResources, "atlasproject.atlas.mongodb.com/"+k8sProjectName)).Should(Equal("True"))


[nit] rename to GetStatusCondition

leo-ri added 11 commits February 1, 2021 13:20

initial e2e

fd04284

chore: linters check

ad5c9d5

Merge branch 'main' into e2ev2

45ccd9f

Merge

6715fc0

tests: e2e local workaround

b3fe94e

check

cd6a4d2

merge test workflows

796d16f

change timeout for update

1c164fd

lint

2c427da

Merge branch 'main' into CLOUDP-80268-e2e

3a2fa6e

fixed go mod

66400de

antonlisovenko suggested changes Feb 5, 2021

View reviewed changes

leo-ri added 7 commits February 6, 2021 01:54

conf update

c27f16a

lint

ac46abb

Merge branch 'main' into CLOUDP-80268-e2e

d07e5a1

gomod

bdf7a75

fix export, waiting project

2680c08

lint + fixes

ab4e33b

split cli.go into kubectl and mongocli

b2807fc

leo-ri requested review from antonlisovenko and vasilevp February 8, 2021 13:32

antonlisovenko reviewed Feb 8, 2021

View reviewed changes

check kube statename

7300d28

leo-ri requested a review from antonlisovenko February 9, 2021 09:22

leo-ri added 2 commits February 9, 2021 17:29

move kube/mongocli to different package

5da2bd0

Update cli.go

a3eb569

antonlisovenko approved these changes Feb 9, 2021

View reviewed changes

leo-ri added 3 commits February 10, 2021 12:46

renaming

fcb5bb1

Merge branch 'main' into CLOUDP-80268-e2e

7dff40f

Merge branch 'main' into CLOUDP-80268-e2e

8c736e3

leo-ri merged commit 33c0748 into main Feb 10, 2021

leo-ri deleted the CLOUDP-80268-e2e branch February 10, 2021 13:53

Cloudp-80268 e2e initial #84

Cloudp-80268 e2e initial #84

Uh oh!

Conversation

leo-ri commented Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antonlisovenko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leo-ri Feb 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonlisovenko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

antonlisovenko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

leo-ri commented Feb 3, 2021 •

edited

Loading

leo-ri Feb 5, 2021 •

edited

Loading