Add ginkgo e2e testing framework #242

munnerz · 2018-02-06T11:56:15Z

What this PR does / why we need it:

Adds a ginkgo based e2e testing framework, and updates the Elasticsearch e2e tests to be written with it.

fixes #32, fixes #156

Special notes for your reviewer:

Stacked on #241, #240 and #239

Release note:

NONE

munnerz · 2018-02-06T12:11:52Z

/test e2e

munnerz · 2018-02-06T17:04:22Z

/hold

jetstack-ci-bot · 2018-02-06T19:31:50Z

@munnerz PR needs rebase

Skip not found errors in DeleteAllStatefulSets

munnerz · 2018-03-01T11:59:01Z

/hold cancel

kragniz

This looks like a great move, I'll be glad to write less bash!

Some things:

This needs a rebase (the cassandra e2e tests no longer use helm)
It'd be great if the code added from k/k (eg in test/e2e/framework) could have a note about where it was sourced from to help with maintenance in the future (I think it'd be best to try and keep them in sync rather than diverging)

jetstack-ci-bot · 2018-03-07T22:38:00Z

@munnerz PR needs rebase

jetstack-bot · 2018-03-21T12:40:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: munnerz

Assign the PR to them by writing /assign @munnerz in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

munnerz · 2018-03-21T17:33:58Z

/retest

wallrj

Thanks @munnerz

I've taken a first look through and left some comments.

I can see that this is probably going to be more easily maintainable than the bash script.
But I'm going to miss the clarity and debugability of the bash script.
Also going to miss the verbose xtrace output which shows exactly what kubectl command are being run so that I can dive in and run those commands myself if they fail.
I got the following error when I tried running the tests on my dind cluster:

make e2e-test REGISTRY=gcr.io/jetstack-richard IMAGE_TAGS=build > e2e.stdout 2>e2e.stderr

...
==> e2e.stdout <==                   
# Execute e2e tests                  
./e2e-tests \                        
        -kubeconfig=$HOME/.kube/config \                                   
        -context=$HOSTNAME \         
        -elasticsearch-pilot-image-repo="gcr.io/jetstack-richard/navigator-pilot-elasticsearch" \                                                     
        -elasticsearch-pilot-image-tag="build" \                           
        -clean-start=true \          
        -report-dir=./_artifacts     

==> e2e.stderr <==                   
I0322 09:28:18.242017   26104 e2e.go:178] Starting e2e run "5ae8c698-2db3-11e8-829e-42010a8c0002" on Ginkgo node 1                                    

==> e2e.stdout <==                   
Running Suite: Navigator e2e suite   
==================================   
Random Seed: 1521710898 - Will randomize all specs                         
Will run 4 of 4 specs                

Mar 22 09:28:18.242: INFO: >>> kubeConfig: /home/richard/.kube/config      
Mar 22 09:28:18.243: INFO: >>> kubeContext: pet-instance-1                 

==> e2e.stderr <==                   
F0322 09:28:18.243381   26104 e2e.go:35] Error loading client: error creating client: invalid configuration: [context was not found for specified context: pet-instance-1, cluster has no server defined]

==> e2e.stdout <==                   
Makefile:37: recipe for target '.run_e2e' failed                           

==> e2e.stderr <==                   
make: *** [.run_e2e] Error 255

richard@pet-instance-1:~/go/src/github.com/jetstack/navigator$ cat /home/richard/.kube/config
apiVersion: v1
clusters:
- cluster:
    insecure-skip-tls-verify: true
    server: http://localhost:8080
  name: dind
contexts:
- context:
    cluster: dind
    namespace: default
    user: ""
  name: dind
current-context: dind
kind: Config
preferences: {}
users: []

wallrj · 2018-03-22T09:22:22Z

Makefile

+		-context=$$HOSTNAME \
+		-elasticsearch-pilot-image-repo="${REGISTRY}/navigator-pilot-elasticsearch" \
+		-elasticsearch-pilot-image-tag="${BUILD_TAG}" \
+		-clean-start=true \


The command line flags for -clean-start say that it cleans out all namespaces except default and system.
So do the ginkgo tests re-install navigator?

-clean-start If true, purge all namespaces except default and system before running tests. This serves to Cleanup test namespaces from failed/interrupted e2e runs in a long-lived cluster.

wallrj · 2018-03-22T09:24:50Z

hack/libe2e.sh

@@ -43,7 +43,7 @@ function retry() {

 function kube_create_namespace_with_quota() {
    local namespace=$1
-    kubectl create namespace "${namespace}"
+    kubectl create namespace "${namespace}" || true


Is this added to handle the case where the namespace already exists?
And in that case it sounds like a problem with the logic in the scripts.
And if the namespace already exists then the presumably the quota may also exist and needs a similar || true

wallrj · 2018-03-22T09:26:25Z

test/e2e/e2e.go

@@ -0,0 +1,181 @@
+package e2e


Is this copied from Kubernetes project?
Is there a copyright header missing?
And if we've modified it, is it clear what we've modified?
Perhaps add a few comments at the top of this file.

wallrj · 2018-03-22T09:27:30Z

test/e2e/e2e.go

+				metav1.NamespaceSystem,
+				metav1.NamespaceDefault,
+				metav1.NamespacePublic,
+				framework.TestContext.NavigatorNamespace,


Ah, so this answers my previous point.
Maybe add a comment and update the command line help?

In the shell script we do delete namespace XXX --now. Which speeds up the deletion somewhat. Does this framework do the same?

wallrj · 2018-03-22T09:37:08Z

test/e2e/e2e.go

+
+// Similar to SynchornizedBeforeSuite, we want to run some operations only once (such as collecting cluster logs).
+// Here, the order of functions is reversed; first, the function which runs everywhere,
+// and then the function that only runs on the first Ginkgo node.


typo SynchornizedBeforeSuite

wallrj · 2018-03-22T09:52:11Z

test/e2e/framework/cleanup.go

+// whole test being terminated.  This allows arbitrary pieces of the overall
+// test to hook into SynchronizedAfterSuite().
+func AddCleanupAction(fn func()) CleanupActionHandle {
+	p := CleanupActionHandle(new(int))


What does new(int) do here?

Creates a pointer to an int with the default value (0). It's basically ptr.Int(0)

wallrj · 2018-03-22T09:57:54Z

test/e2e/framework/elasticsearch_utils.go

+		es := &esList.Items[i]
+		Logf("Deleting elasticsearchcluster %v", es.Name)
+		// Use OrphanDependents=false so it's deleted synchronously.
+		// We already made sure the Pods are gone inside Scale().


I don't understand this comment. The AfterEach blocks above seem to run DeleteAllElasticsearchClusters before deleting statefulsets.
Do you mean that when deleting a statefulset it will first be scaled to zero to remove all the pods?

wallrj · 2018-03-22T10:01:12Z

test/e2e/framework/elasticsearch_utils.go

+		Failf("Node pool %q not found in ElasticsearchCluster %q", pool, es.Name)
+	}
+	By("Scaling node pool " + es.Name + "/" + pool + " in namespace " + es.Namespace)
+	es, err = e.navClient.NavigatorV1alpha1().ElasticsearchClusters(es.Namespace).Update(es)


Is this susceptible to conflict errors? If Navigator has since updated the status of the ES cluster resource?
Does this need RetryOnConflict handling?

wallrj · 2018-03-22T10:04:27Z

test/e2e/framework/elasticsearch_utils.go

+		"involvedObject.namespace": ns,
+		"source":                   "navigator-controller",
+	}.AsSelector().String()
+	options := metav1.ListOptions{FieldSelector: selector}


That's cool. Can you use field selectors with the Lister API too?

Yep I'm pretty sure the listers take the same metav1.ListOptions struct 😄

wallrj · 2018-03-22T10:07:33Z

test/e2e/framework/framework.go

@@ -0,0 +1,60 @@
+package framework


I'll assume that all the rest of the framework files are copied from Kubernetes. But note that not all of them have Kubernetes copyright headings.
Maybe add a oneline comment to the files that have been copied showing where from, what version (git hash) and if modified a summary of the changes.

munnerz · 2018-03-22T13:08:32Z

I got the following error when I tried running the tests on my dind cluster:

You've got --context=$HOSTNAME on your e2e test command, hence the error about missing context 😄

wallrj · 2018-03-27T10:08:14Z

You've got --context=$HOSTNAME on your e2e test command, hence the error about missing context

Sure, but I think I should be able to run make e2e-tests on my local cluster and it should just use the default context.

jetstack-bot · 2018-03-28T04:22:23Z

@munnerz: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jetstack-bot added the release-note-none label Feb 6, 2018

jetstack-bot requested a review from wallrj February 6, 2018 11:56

jetstack-bot added the size/XXL label Feb 6, 2018

munnerz force-pushed the ginkgo-e2e branch from ca1acce to 1fb2773 Compare February 6, 2018 11:59

munnerz force-pushed the ginkgo-e2e branch 3 times, most recently from 5e4219f to b365878 Compare February 6, 2018 16:55

jetstack-bot added the do-not-merge/hold label Feb 6, 2018

munnerz mentioned this pull request Feb 6, 2018

Refactor Elasticsearch controller to use a semi-state machine design & add ginkgo based e2e tests #228

Closed

jetstack-ci-bot added the needs-rebase label Feb 6, 2018

munnerz mentioned this pull request Feb 21, 2018

Mark the first pod in a cassandra nodepool as a seed #254

Merged

Add ginkgo e2e testing framework

0812970

Skip not found errors in DeleteAllStatefulSets

munnerz force-pushed the ginkgo-e2e branch from b365878 to 0812970 Compare February 23, 2018 00:12

jetstack-ci-bot removed the needs-rebase label Feb 23, 2018

Run dep ensure

16a5a84

jetstack-bot removed the do-not-merge/hold label Mar 1, 2018

kragniz reviewed Mar 5, 2018

View reviewed changes

jetstack-ci-bot added the needs-rebase label Mar 7, 2018

Merge branch 'master' into ginkgo-e2e

b7c7eac

jetstack-bot removed the needs-rebase label Mar 21, 2018

James Munnelly added 4 commits March 21, 2018 12:53

Remove sysctls field

f7102d3

Add notice on where some of the e2e test framework is sourced from

4b7cb4a

Move env vars from prepare-e2e after changes to bash e2e tests

7e6974d

Fix navigator installation step during e2e

c580055

James Munnelly added 2 commits March 21, 2018 15:36

Fix CHART_VALUES

9a60917

Add esc shortname test

faa243f

wallrj mentioned this pull request Mar 22, 2018

Cassandra nodetool tests fail to compile with Go 1.8.x #300

Closed

wallrj suggested changes Mar 22, 2018

View reviewed changes

munnerz added this to the v0.1 milestone Mar 27, 2018

munnerz self-assigned this Mar 27, 2018

jetstack-bot added the needs-rebase label Mar 28, 2018

munnerz modified the milestones: v0.1, v0.2 Apr 3, 2018

wallrj added the kind/feature label May 15, 2018

wallrj mentioned this pull request Jun 6, 2018

Use local volumes instead of hostpath volumes #366

Merged

munnerz closed this Mar 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ginkgo e2e testing framework #242

Add ginkgo e2e testing framework #242

munnerz commented Feb 6, 2018 •

edited

Loading

munnerz commented Feb 6, 2018

munnerz commented Feb 6, 2018

jetstack-ci-bot commented Feb 6, 2018

munnerz commented Mar 1, 2018

kragniz left a comment

jetstack-ci-bot commented Mar 7, 2018

jetstack-bot commented Mar 21, 2018

munnerz commented Mar 21, 2018

wallrj left a comment

wallrj Mar 22, 2018

wallrj Mar 22, 2018

wallrj Mar 22, 2018

wallrj Mar 22, 2018

wallrj Mar 22, 2018

wallrj Mar 22, 2018

wallrj Mar 22, 2018

munnerz Mar 27, 2018

wallrj Mar 22, 2018

wallrj Mar 22, 2018

wallrj Mar 22, 2018

munnerz Mar 27, 2018

wallrj Mar 22, 2018

munnerz commented Mar 22, 2018

wallrj commented Mar 27, 2018

jetstack-bot commented Mar 28, 2018

Add ginkgo e2e testing framework #242

Add ginkgo e2e testing framework #242

Conversation

munnerz commented Feb 6, 2018 • edited Loading

munnerz commented Feb 6, 2018

munnerz commented Feb 6, 2018

jetstack-ci-bot commented Feb 6, 2018

munnerz commented Mar 1, 2018

kragniz left a comment

Choose a reason for hiding this comment

jetstack-ci-bot commented Mar 7, 2018

jetstack-bot commented Mar 21, 2018

munnerz commented Mar 21, 2018

wallrj left a 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

munnerz commented Mar 22, 2018

wallrj commented Mar 27, 2018

jetstack-bot commented Mar 28, 2018

munnerz commented Feb 6, 2018 •

edited

Loading