Add e2e test that deploys two Cass clusters and checks that Clusters … #16

burmanm · 2020-11-25T14:15:39Z

…are re-registered. Closes #10

┆Issue is synchronized with this Jira Bug by Unito

burmanm · 2020-11-25T18:56:42Z

Last commit allows doing

[michael@nina reaper-operator]$ ORG=burmanm REG=localhost:5000 make docker-build docker-push e2e-test

And it runs the e2e-tests using the just built reaper-operator (if the kind for example would read from repo localhost:5000 like in their example)

jsanda · 2020-12-03T04:55:50Z

Makefile

@@ -82,6 +82,7 @@ vet:
 PHONY: e2e-test
 e2e-test:
 	@echo Running e2e tests
+	cd test/config/deploy_reaper_test && $(KUSTOMIZE) edit set image controller=${REV_IMAGE} && cd -


I have been thinking about this since you starting working on reaper-operator and was about to create a ticket but decided to start reviewing this PR instead. I am glad I did :)

The problem with calling kustomize edit here is that it makes the change to the kustomization.yaml file which is stored in git. That turns into an ugly situation. Unfortunately kustomize build does not allow you to pass arguments. I need to look into this some more. multibases might help here.

jsanda · 2020-12-03T05:00:38Z

test/config/cassdc/cassdc-non-reaper.yaml

@@ -0,0 +1,62 @@
+apiVersion: cassandra.datastax.com/v1beta1


It looks like the only differences between cassdc-non-reaper.yaml and cassdc.yaml are the name and clusterName properties. We can avoid a lot of that duplication. I will work on an example of how best to do that.

Also the size is different (again, to speed up the start and reduce resource usage). But yes, templating this would be a better option.

jsanda · 2020-12-03T05:02:52Z

test/e2e/deploy_reaper_test.go

@@ -36,31 +38,21 @@ var (
 var _ = Describe("Deploy Reaper with Cassandra backend", func() {
 	Context("When a Cassandra cluster is deployed", func() {
 		Specify("Reaper is deployed", func() {
+			// Speed up cassandradatacenter_controller status checks


When we initially discussed this scenario, I figured it would be a separate test. Do you think that would make more sense?

Well, the reason I did in the main test was speed. Already the first test takes a lot of time to execute, so setting another with basically the same properties (other than one extra cluster) doubles the execution time. I'm not sure if there's value for two tests with only this change.

If we would get the cassdc startup to be faster, then adding multiple stages / tests would of course be better.

Well, the reason I did in the main test was speed.

I know that speed is an issue, but I still prefer a separate test mostly because it is a different scenario. For CI, we can run tests in parallel. We can also see what can be done to speed things up.

jsanda · 2020-12-03T05:11:42Z

test/config/cassdc/kustomization.yaml

 resources:
 - cassdc.yaml
+- cassdc-non-reaper.yaml


See my comment below about creating a separate test. Assuming we do that we will want to create some new kustomize templates. We might create something like this under test/config/cassdc:

├── kustomization.yaml ├── bases ├── test-a ├── test-b └── test-c

The bases directory would have the common CassandraDatacenter template. Each test- directory would extend the base.

Then under the test/config directory we might have:

├── cass-operator ├── cassdc └── tests ├── test-a └── test-b

where each test- subdirectory would have its own kustomization.yaml file. I need to think about this some more.

jsanda · 2020-12-03T15:10:47Z

test/e2e/deploy_reaper_test.go

@@ -36,31 +38,21 @@ var (
 var _ = Describe("Deploy Reaper with Cassandra backend", func() {
 	Context("When a Cassandra cluster is deployed", func() {
 		Specify("Reaper is deployed", func() {
+			// Speed up cassandradatacenter_controller status checks
+			os.Setenv("REQUEUE_DELAY_STATUS_CHECK", "1m")


Not sure if you aware, but we override the env vars, inlcuding REQUEUE_DELAY_STATUS_CHECK, with a kustomize patch.

Take a look at the output of kustomize build test/config/deploy_reaper_test to see.

This is done in test/config/deploy_reaper_test/reaper-operator-patch.yaml.

jsanda

@burmanm can you go ahead update from master since the kustomize changes for using a different operator image have been merged?

I will make a point to add some docs tomorrow. Here is the short version of what do to:

# Build and push your own image
$ ORG=burmanm make docker-build docker-push

# This will create test/config/deploy_reaper_test/overlays/forks/burmanm/kustomization.yaml
# We will need to update the script to generate the fork overlay dir
$ REGISTRY=docker.io OVERLAY=burmanm ORG=burmanm make init-test-overlays

# Run the test
$ TEST_OVERLAY=burmanm make e2e-test

jsanda · 2020-12-10T03:49:09Z

test/e2e/deploy_reaper_test.go

@@ -36,31 +38,21 @@ var (
 var _ = Describe("Deploy Reaper with Cassandra backend", func() {
 	Context("When a Cassandra cluster is deployed", func() {
 		Specify("Reaper is deployed", func() {
+			// Speed up cassandradatacenter_controller status checks


Well, the reason I did in the main test was speed.

I know that speed is an issue, but I still prefer a separate test mostly because it is a different scenario. For CI, we can run tests in parallel. We can also see what can be done to speed things up.

…are re-registered

…tests and also patch the size of the secondary cluster

burmanm · 2020-12-11T12:13:05Z

Would these changes make it more like you envisioned?

jsanda · 2020-12-11T15:22:46Z

test/config/deploy_reaper_test/kustomization.yaml

@@ -1,2 +1,8 @@
 resources:
 - base
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+images:


How come you have this defined here?

jsanda · 2020-12-11T15:27:38Z

test/e2e/deploy_reaper_test.go

+		})
+
+		Specify("More clusters are created", func() {
+			// Since we didn't clean the previous resources, we don't need to redeploy everything


Are tests guaranteed to execute in the order in which they are declared?

More importantly, this limits our ability to execute tests in parallel in a CI environment. cass-operator puts every test in a separate suite which makes it really easy to run them in parallel. I don't know if we need to do that but I figured separate test files and Describe block would facilitate running them in parallel.

In this case if I understood the documentation correctly they should:

"Ginkgo’s default behavior is to only permute the order of top-level containers – the specs within those containers continue to run in the order in which they are specified in the test file. "

jsanda · 2020-12-11T15:34:59Z

test/framework/framework.go

 // By default this function will run kustomize build on dir/overlays/k8ssandra. This will
 // result in using upstream operator images. If you are testing against a fork, then set
 // the TEST_OVERLAY environment variable to specify the fork overlay to use. When
 // TEST_OVERLAY is set this function will run kustomize build on
 // dir/overlays/forks/TEST_OVERLAY which will allow you to use a custom operator image.
-func KustomizeAndApply(namespace, dir string) {
+func KustomizeAndApply(namespace, dir, overlay string) {


How come you added the overlay parameter here? The goal was to have a way for someone to run the tests against his own image. By adding the overlay parameter, I will have to modify the test to use a different image. And then I would have to do it for every test.

adejanovski · 2022-09-07T06:42:41Z

@burmanm @jsanda, should we close this as won't do, or do we want to get this merged?

burmanm added the complexity: low label Nov 25, 2020

burmanm requested a review from jsanda November 25, 2020 14:15

burmanm self-assigned this Nov 25, 2020

jsanda reviewed Dec 3, 2020

View reviewed changes

jsanda requested changes Dec 10, 2020

View reviewed changes

burmanm added 3 commits December 10, 2020 15:10

Add e2e test that deploys two Cass clusters and checks that Clusters …

87a9da7

…are re-registered

Allow running e2e_tests with local test image

2cfebf7

Deploy secondary cluster in another test, cleanup the mess after the …

18023a6

…tests and also patch the size of the secondary cluster

burmanm force-pushed the e2e_test_changes branch from 1a16eb2 to 18023a6 Compare December 11, 2020 12:12

jsanda requested changes Dec 11, 2020

View reviewed changes

jdonenine added this to the 1.0.0 milestone Jan 8, 2021

jdonenine added the priority: p1 label Jan 8, 2021

burmanm mentioned this pull request Jan 8, 2021

Verify repair schedule was added with autoscheduling #26

Closed

adejanovski added zh:In Progress zh:In-Progress and removed zh:In Progress labels Mar 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add e2e test that deploys two Cass clusters and checks that Clusters … #16

Add e2e test that deploys two Cass clusters and checks that Clusters … #16

burmanm commented Nov 25, 2020 •

edited by sync-by-unito bot

Loading

burmanm commented Nov 25, 2020

jsanda Dec 3, 2020

jsanda Dec 3, 2020

burmanm Dec 3, 2020

jsanda Dec 3, 2020

burmanm Dec 3, 2020

jsanda Dec 10, 2020

jsanda Dec 3, 2020

jsanda Dec 3, 2020

jsanda left a comment

jsanda Dec 10, 2020

burmanm commented Dec 11, 2020

jsanda Dec 11, 2020

jsanda Dec 11, 2020

burmanm Dec 11, 2020

jsanda Dec 11, 2020

adejanovski commented Sep 7, 2022

Add e2e test that deploys two Cass clusters and checks that Clusters … #16

Are you sure you want to change the base?

Add e2e test that deploys two Cass clusters and checks that Clusters … #16

Conversation

burmanm commented Nov 25, 2020 • edited by sync-by-unito bot Loading

burmanm commented Nov 25, 2020

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

jsanda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burmanm commented Dec 11, 2020

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

adejanovski commented Sep 7, 2022

burmanm commented Nov 25, 2020 •

edited by sync-by-unito bot

Loading