Revive conformance test. #9816
Revive conformance test. #9816
Conversation
|
GCE e2e build/test failed for commit 111ff5bdf99595b43a58f48bc456f2d0e97e9510. |
| date | ||
|
|
||
| # Conformance test does not have to pass all the e2es. Just a subset for now: | ||
| export GINKGO_TEST_ARGS="--focus=DNS|Downward|EmptyDir|Events|Kubectl|Networking|Pods|PreStop|Secrets|Services --skip=Skipped|Service\sendpoints\slatency|Services.*functioning\sexternal|Services.*Type.*LoadBalancer.*NodePort|Services.*change\sthe\stype|Service.*should.release.*|Service.*external\sload\sbalancer" |
There was a problem hiding this comment.
GINKGO_TEST_ARGS is already used in hack/jenkins/e2e.sh to define what is passed in the --test_args flag to hack/e2e.go, which is in turn passed as arguments to hack/ginkgo-e2e.sh.
It's a complete roundabout mess, but using the same name here is likely to cause problems.
An alternative to creating this new env var is to just append --ginkgo.focus=... to the hack/ginkgo-e2e.sh line below.
There was a problem hiding this comment.
Should be --ginkgo.focus?
I think if you supply --ginkgo.focus, everything else is skipped, so you don't need both.
How we will be able to add a test to this list without making everyone angry? One idea is to keep a queue of lists; so you'd say JUNE_REQ=foo, JULY_REQ=foo,bar, AUGUST_REQ=foo,bar,baz... etc. This way people will see what's going to be required of them and fix their distros preemptively, and we can add requirements in the future without instantly disqualifying people today. (Or the requirements could be tagged by version instead of by month... the point is just that we need to give maintainers enough warning.)
There was a problem hiding this comment.
Regarding the argument name:
If you pass the focus argument to the ginkgo binary itself (as is currently done in this PR), then it's named --focus. If you pass it to the test (by passing it after -- on the ginkgo cmdline - as was my suggestion, passing it as an argument to hack/ginkgo-e2e.sh), then it's named --ginkgo.focus.
There was a problem hiding this comment.
@lavalamp about needing both:
the --focus and --skip arguments are anded together.
This allows me to say "run all service tests except these ones" and so forth.
In terms of versioning:
- I plan to add language that says that the conformance test suite may change. Let's first solve the problem of separating the set of distros that ever worked from the set that never worked. Then later solve the problem of what to do when we raise the bar.
- I will add language soon which says that the conformance test suite may change over time.
- I don't think we need versions: we can just use corresponding version of the repository (commit sha, or release tag) to be clear about what version of the conformance test someone ran. I'll change the script to print the sha and any version tags of the current repo.
There was a problem hiding this comment.
Gotcha. SGTM
On Mon, Jun 15, 2015 at 3:05 PM, Eric Tune notifications@github.com wrote:
In hack/conformance-test.sh
#9816 (comment)
:+# declare -x KUBE_CONFIG_FILE="=$HOME/.kube/config"
+# - Specify the location of the master with, e.g.:
+# declare -x KUBE_MASTER_IP="1.2.3.4"
+# - Make sure only essential pods are running and there are no failed/pending pods.
+# Then run:
+# hack/conformance-test.sh 2>&1 | tee conformance.$(date +%FT%T%z).log
+
+: ${KUBE_CONFIG_FILE:?"Must set KUBE_CONFIG_FILE before running conformance test."}
+: ${KUBE_MASTER_IP:?"Must set MASTER_IP before running conformance test."}
+
+echo "Conformance test version: 0.1"
+echo -n "Conformance test run date:"
+date
+
+# Conformance test does not have to pass all the e2es. Just a subset for now:
+export GINKGO_TEST_ARGS="--focus=DNS|Downward|EmptyDir|Events|Kubectl|Networking|Pods|PreStop|Secrets|Services --skip=Skipped|Service\sendpoints\slatency|Services._functioning\sexternal|Services._Type._LoadBalancer._NodePort|Services._change\sthe\stype|Service.should.release.|Service._external\sload\sbalancer"@lavalamp https://github.com/lavalamp about needing both:
the --focus and --skip arguments are anded together.
This allows me to say "run all service tests except these ones" and so
forth.In terms of versioning:
- I plan to add language that says that the conformance test suite may
change. Let's first solve the problem of separating the set of distros that
ever worked from the set that never worked. Then later solve the problem of
what to do when we raise the bar.- I will add language soon which says that the conformance test suite
may change over time.- I don't think we need versions: we can just use corresponding
version of the repository (commit sha, or release tag) to be clear about
what version of the conformance test someone ran. I'll change the script to
print the sha and any version tags of the current repo.—
Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/9816/files#r32472407
.
|
GCE e2e build/test passed for commit b70b7084c93d4ce80b7463f48c23d5ac04edb2b1. |
|
Okay, @lavalamp was right, I can't use focus and skip together. |
| # Shell: replies on optional ssh access to nodes. | ||
| # SSH: optional feature. | ||
| # Volumes: contained only skipped tests. | ||
| export GINKGO_TEST_ARGS="--focus=DNS|Docker\sContainers|Downward|EmptyDir|Events|hostDir|Kubectl|Networking|Pods|PreStop|ReplicationController|Secrets|Services.*master|Services.*endpoint|Services.*multiport" |
There was a problem hiding this comment.
you still didn't address my comment regarding GINKGO_TEST_ARGS.
|
GCE e2e build/test passed for commit b8c9b37a5efd2082b81b7eb1f546bcf2c3f3a516. |
|
Okay, I think I addressed your request @ixdy |
|
GCE e2e build/test failed for commit 2a4ad28a86ef0af06be8f3c1ae15d685cc047bcc. |
|
You can actually focus and skip. It works for me: http://onsi.github.io/ginkgo/#focused-specs "When using the command line flags you can specify one or both of --focus and --skip. If both are specified the constraints will be ANDed together." |
|
@k8s-bot please test this again. |
|
GCE e2e build/test passed for commit 5ed62cb0bfb2856d74b6b938fa94db4c7087b3cb. |
|
GCE e2e build/test passed for commit 664c3c092347c54bc6f6de1743eaf9ae666757cb. |
|
@erictune are you still working on this? I see some obvious debugging statements still lurking here. |
|
Ready for review now. |
|
GCE e2e build/test failed for commit 11f442211a1df882e2311f8152864944adeddcf7. |
|
|
||
| : ${KUBECONFIG:?"Must set KUBECONFIG before running conformance test."} | ||
| : ${KUBE_MASTER_IP:?"Must set KUBE_MASTER_IP before running conformance test."} | ||
| echo "Conformance test using $KUBECONFIG against master at $KUBE_MASTER_IP" |
There was a problem hiding this comment.
nit: ${KUBECONFIG} and ${KUBE_MASTER_IP}
| auth_config=( | ||
| "--kubeconfig=${KUBECONFIG:-$DEFAULT_KUBECONFIG}" | ||
| ) | ||
| if [[ "${KUBERNETES_PROVIDER}" == "gke" ]]; then |
There was a problem hiding this comment.
the GKE stuff was removed in #9227.
|
GCE e2e build/test passed for commit e2b8449fa7a4e868eb3cee26efed39eedac5f354. |
Use KUBE_CONFIG_FILE instead of AUTH_CONFIG (and CERT_DIR). Pass GINKGO_TEST_ARGS for a subset of e2e tests which @erictune has deemed initially sufficient for conformance. Allow GINKGO_TEST_ARGS to pass through hack/ginkgo-e2e.sh. Set NUM_MINIONS (need better way to handle this). Remove use of "KUBERNETES_CONFORMANCE_TEST" variable and use of KUBERNETES_PROVIDER="conformance_test" convention, both of which have no apparent purpose. Allow unset testContext.provider in test/e2e/e2e_test.go Allow testContext.Provider to be unset in providerIs().
|
GCE e2e build/test passed for commit c5efb12. |
|
Helps fix #9717 |
|
@ixdy PTAL |
|
LGTM |
ixdy
added
the
lgtm
|
Risk is low because it would have broken e2e if it had any issues, and it is mostly adding a separate testing path, and not changing existing behavior. @lavalamp can I get a Leads LGTM. |
|
@quinton-hoole can I get a Leads LGTM |
| @@ -81,6 +73,9 @@ if [[ "${KUBERNETES_PROVIDER}" == "gke" ]]; then | |||
| fi | |||
|
|
|||
| ginkgo_args=() | |||
| if [[ -n ${CONFORMANCE_TEST_SKIP_REGEX:-} ]]; then | |||
|
LGTM; my comments are all very minor, fix only if you want to. |
|
LGTM OK-to-merge |
ghost
added
the
|
Please merge. I'll address @lavalamp comments in a follow up PR. |
|
@erictune Why was NUM_MINIONS hardcoded to 4 in this change? I was under the impression that the e2e tests only required 2 minions. Did that change at some point? I see many of the clusters default to 4 minions now in config-default.sh, but are still set to 2 in config-test.sh. |
|
In some places it's 2, in some places, it's 3, in others, it's 4. We should probably be more consistent about this. Context for bumping up from 2 is in #10899. |
|
Thanks for the context, @ixdy After reading all that I'm a bit disappointed that the cluster sizes were increased. It makes the defaults more expensive to both deploy and test on IaaS. And it looks like it was increased primarily a work around for a test pollution issue. I have another conformant test question tho. Why is KUBE_MASTER_IP explicitly required by conformance-test.sh? According to the comments in ginkgo-e2e.sh:
That makes it sound like KUBE_MASTER_IP, which is used to populate --host, should be optional, since KUBECONFIG is enforced (and required, because it contains the SSL certs). |
|
@karlkfi As a workaround for the cost of 4-node clusters, would it work to exclude just those tests that require 4 nodes from your continuous testing, (using ginkgo_skip)? Perhaps run those tests only prior to a major release? |
|
Are there tests that actually require 4 nodes? I got the impression from the config-test.sh files (https://github.com/kubernetes/kubernetes/blob/master/cluster/gce/config-test.sh#L23) that they were all running on 2, even tho the default is 4 (https://github.com/kubernetes/kubernetes/blob/master/cluster/gce/config-default.sh#L23). It looks like Jenkins is also using 2: https://github.com/brendandburns/kubernetes/blob/master/hack/jenkins/e2e.sh#L57 From reading #10899 I got the impression that the soak tests were breaking subsequent tests with only 2, but that seems like a good reason not to run soak tests as part of the larger e2e test suite. We've been discussing at Mesosphere that it would make sense to have another layer of testing, one that deploys a new cluster for each test, in order to test things like node failover, load testing, or anything that is potentially destructive. The test suite needs to be able to destroy and recreate the cluster for tests like these, but not for most other e2e tests. |
|
your fork of hack/jenkins/e2e.sh is a bit out of date - see https://github.com/kubernetes/kubernetes/blob/master/hack/jenkins/e2e.sh#L74. |
|
nice catch. not sure how i got on brendan's fork... Perhaps we should tag tests individually for how many nodes they need? That way they can skip themselves or error out early if the node count is too low, rather than breaking subsequent tests. |
Use KUBE_CONFIG_FILE instead of AUTH_CONFIG (and CERT_DIR).
Pass GINKGO_TEST_ARGS for a subset of e2e tests which
@erictune has deemed initially sufficient for conformance.
Allow GINKGO_TEST_ARGS to pass through hack/ginkgo-e2e.sh.
Set NUM_MINIONS (need better way to handle this).
Remove use of "KUBERNETES_CONFORMANCE_TEST" variable
and use of KUBERNETES_PROVIDER="conformance_test" convention,
both of which have no apparent purpose.
Allow unset testContext.provider in test/e2e/e2e_test.go
Allow testContext.Provider to be unset in providerIs().