[DO-NOT-MERGE] Add optional E2E test artifact to binary artifacts for openshift #6541

jayunit100 · 2016-01-05T18:59:40Z

Im now testing this on real clusters, will update accordingly. This Implementation takes into account all feedback from the previous implementation, and rolls it into a fresh PR.

This obsoletes #6395

jayunit100 · 2016-01-05T19:03:51Z

Fixes #5765

Here is a reimplementation of the e2e build product for openshift.

reimplemented so that, by default, no e2e build product is produced (per @liggitt s concerns about performance).
includes a sample e2e test which can be borrowed/extended.
it manages test dependencies by copying them dynamically.
have done some basic re-testing, will do more.

cc @pmorie @timothysc

jayunit100 · 2016-01-05T23:33:59Z

Since its disabled by default ; To test this, sudo OS_BUILD_E2E_TEST=true make clean build, And you should see ./_output/local/bin/linux/amd64/e2e.test in the outputs.
Then you can run the tests:
sudo _output/local/bin/linux/amd64/e2e.test --host="https://0.0.0.0:8443" --provider="local" --ginkgo.v=true --ginkgo.focus=Networking --kubeconfig=_output/local/bin/linux/amd64/master/openshift-master.kubeconfig --repo-root=./

liggitt · 2016-01-06T00:04:38Z

Should be a separate build target, and should not need sudo to build

jayunit100 · 2016-01-06T13:45:34Z

Yes in this case I figured as per your comment, so long as

we were capable of building the binary for releases and
it was not a product of make build

then that satisfied the requirement of "fast builds by default".

good idea to definitely add explicit make target support, but... even so, common.sh is the natural place to build these things it seems. which is called by build-go.sh.
Is it a hard requirement, also, to dereference e2e from build-go.sh entirely?

The reason I'm asking, is that build-go.sh by definition builds all go components. , so I am hesitant to completely decouple e2e from build-go.sh.

TL;DR I think we are in agreement, with one question - do we need to eliminate references to e2e from common.sh entirely?

jayunit100 · 2016-01-06T15:25:38Z

pushed some more updates and rebased/cleaned. testing now. I think this might be close to consensus.

jayunit100 · 2016-01-06T15:36:55Z

Okay, here are the new results. Taking into account the above suggestions regarding the Make target.

Sudo is not required to build. I was using it unnecessarily in the above example.
The Makefile now supports a make e2e goal.
The regular build targets now have no reference to the e2e build.

Regular build:

➜  origin git:(e2e-bindings-dev-PR) ✗ make build
hack/build-go.sh
++ Building go targets for darwin/amd64: cmd/openshift cmd/oc
Artifact: github.com/openshift/origin/cmd/openshift
... Building core binary github.com/openshift/origin/cmd/openshift
Artifact: github.com/openshift/origin/cmd/oc
... Building core binary github.com/openshift/origin/cmd/oc
++ Placing binaries
hack/build-go.sh took 19 seconds

And for E2Es:

➜  origin git:(e2e-bindings-dev-PR) ✗ make e2e
hack/build-go.sh test/e2e/e2e.test
++ Building go targets for darwin/amd64: test/e2e/e2e.test
Artifact: github.com/openshift/origin/test/e2e/e2e.test
... Building e2e artifact
++ Placing binaries
hack/build-go.sh took 10 seconds

stevekuznetsov · 2016-01-06T17:11:14Z

hack/common.sh

@@ -3,6 +3,7 @@
 # This script provides common script functions for the hacks
 # Requires OS_ROOT to be set

+set -o nounset


duplicate of line 8

Default to false. Formatting fixed, explicit make target.

jayunit100 · 2016-01-06T19:18:38Z

Thanks. Ive rebased, squashed, and fixed. I think this is ready for a final look.

jayunit100 · 2016-01-07T15:38:17Z

You can also test this locally in DIND, it appears to work, and we can continue using the test-kube-e2e.sh wrapper for consistency, (with just one minor change) to hack/test-kube-e2e.sh.

Delete the KUBE_ROOT stuff, and do

-hack/ginkgo-e2e.sh $@
+.//_output/local/bin/linux/amd64/e2e.test $@

Then, you can do vagrant ssh ; make e2e; hack/test-kube-e2e.sh ; --provider=local --kubeconfig=/tmp/openshift-dind-cluster/openshift/openshift.local.config/master/openshift-master.kubeconfig --ginkgo.focus="30 pods"

pmorie · 2016-01-07T21:05:13Z

test/e2e/e2e_test.go

+import (
+ . "github.com/onsi/ginkgo"
+ "k8s.io/kubernetes/test/e2e"
+ "testing"


nit, golang style is to separate the stdlib imports from third party with a line break

jayunit100 · 2016-01-07T22:09:36Z

fixed the formatting for imports

liggitt · 2016-01-09T03:18:50Z

Makefile

+# Example:
+# make e2e
+e2e:
+ hack/build-go.sh test/e2e/e2e.test


I like how this looks

liggitt · 2016-01-09T03:42:33Z

hack/common.sh

- go install "${goflags[@]:+${goflags[@]}}" \
- -ldflags "${version_ldflags}" \
- "${binaries[@]}"
+ for artifact in ${binaries[@]}; do


if the whole implementation of building the e2e test artifact is different, this probably should be a separate function, maybe even a separate script (build-e2e.sh, modeled after a combination of build-go.sh and test-integration.sh?)

if a strong case can be made for a wrapped kube e2e, we should do the upstream work to allow wrapping it (putting the guts in a non _test file, and making the _test file a 3-line function we wouldn't mind duplicating

making upstream test into a utility is doable, but likely will meet a little pushback.

I'll say it again, if a strong case can be made for a wrapped kube e2e, we should do the upstream work to allow wrapping it. If a strong case can't be made, we should not wrap it :)

timothysc · 2016-01-12T15:34:15Z

cross-ref kubernetes/kubernetes#19476
/cc @jeremyeder

jeremyeder · 2016-01-12T17:23:36Z

We in performance engineering have a need for a "verify environment" function, and were about to write one. I understand this suite includes conformance tests that could be used for this purpose.

From a correctness and reproducibility standpoint, having access to these conformance tests in OpenShift would increase confidence in our deployments.

jayunit100 · 2016-01-13T15:29:26Z

Added "Do Not Merge", see produce e2e.test as a build product with the ability to extend from upstream #5765 for details (we are focusing on the upstream implementation of this patch).
Just in case, ill still keep this PR open, until we factor upstream acceptance / rebase costs.

jayunit100 · 2016-01-16T01:08:39Z

Closing, upstream alternative is pending. See the original issue for details.

jayunit100 · 2016-02-05T19:22:34Z

Subsumed by #7049

jayunit100 force-pushed the e2e-bindings-dev-PR branch from 86e7c00 to 57bb5b9 Compare January 5, 2016 19:01

This was referenced Jan 5, 2016

produce e2e.test as a build product with the ability to extend from upstream #5765

Closed

[WIP] First Pass E2e bindings #6395

Closed

jayunit100 force-pushed the e2e-bindings-dev-PR branch from 3894c32 to 53e65f2 Compare January 5, 2016 20:19

jayunit100 mentioned this pull request Jan 6, 2016

Evaluate E2E Tests in context of local build product. #6551

Closed

jayunit100 force-pushed the e2e-bindings-dev-PR branch 4 times, most recently from 93197c2 to 8e7661b Compare January 6, 2016 15:25

jayunit100 force-pushed the e2e-bindings-dev-PR branch 2 times, most recently from d24af58 to bb3629b Compare January 6, 2016 15:31

stevekuznetsov reviewed Jan 6, 2016
View reviewed changes

Add optional E2E test artifact to binary artifacts for openshift.

d5c2ca4

Default to false. Formatting fixed, explicit make target.

jayunit100 force-pushed the e2e-bindings-dev-PR branch from bb3629b to d5c2ca4 Compare January 6, 2016 19:17

jayunit100 mentioned this pull request Jan 6, 2016

Decide whether the future E2Es should publish test/extended as single binary #6557

Closed

jayunit100 mentioned this pull request Jan 7, 2016

[E2E] [DIND] intra-pod communication tests should work OOTB. #6577

Closed

pmorie reviewed Jan 7, 2016
View reviewed changes

formatting for imports

6f84758

jayunit100 mentioned this pull request Jan 8, 2016

[E2E][DIND] EmptyDir tests fail. #6589

Closed

liggitt reviewed Jan 9, 2016
View reviewed changes

Makefile

# Example:

# make e2e

e2e:

hack/build-go.sh test/e2e/e2e.test

Copy link

Contributor

liggitt Jan 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how this looks

liggitt reviewed Jan 9, 2016
View reviewed changes

jayunit100 changed the title ~~Add optional E2E test artifact to binary artifacts for openshift~~ [DO-NOT-MERGE] Add optional E2E test artifact to binary artifacts for openshift Jan 13, 2016

jayunit100 mentioned this pull request Jan 14, 2016

E2E libraryzation (minimal patch) kubernetes/kubernetes#19624

Merged

jayunit100 closed this Jan 16, 2016

timothysc mentioned this pull request Feb 2, 2016

Finalize Kube items 3.2 #6766

Closed

85 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO-NOT-MERGE] Add optional E2E test artifact to binary artifacts for openshift #6541

[DO-NOT-MERGE] Add optional E2E test artifact to binary artifacts for openshift #6541

jayunit100 commented Jan 5, 2016

jayunit100 commented Jan 5, 2016

jayunit100 commented Jan 5, 2016

liggitt commented Jan 6, 2016

jayunit100 commented Jan 6, 2016

jayunit100 commented Jan 6, 2016

jayunit100 commented Jan 6, 2016

stevekuznetsov Jan 6, 2016

jayunit100 commented Jan 6, 2016

jayunit100 commented Jan 7, 2016

pmorie Jan 7, 2016

jayunit100 commented Jan 7, 2016

liggitt Jan 9, 2016

liggitt Jan 9, 2016

liggitt Jan 9, 2016

jayunit100 Jan 9, 2016

liggitt Jan 9, 2016

timothysc commented Jan 12, 2016

jeremyeder commented Jan 12, 2016

jayunit100 commented Jan 13, 2016

jayunit100 commented Jan 16, 2016

jayunit100 commented Feb 5, 2016

[DO-NOT-MERGE] Add optional E2E test artifact to binary artifacts for openshift #6541

[DO-NOT-MERGE] Add optional E2E test artifact to binary artifacts for openshift #6541

Conversation

jayunit100 commented Jan 5, 2016

jayunit100 commented Jan 5, 2016

jayunit100 commented Jan 5, 2016

liggitt commented Jan 6, 2016

jayunit100 commented Jan 6, 2016

jayunit100 commented Jan 6, 2016

jayunit100 commented Jan 6, 2016

stevekuznetsov Jan 6, 2016

Choose a reason for hiding this comment

jayunit100 commented Jan 6, 2016

jayunit100 commented Jan 7, 2016

pmorie Jan 7, 2016

Choose a reason for hiding this comment

jayunit100 commented Jan 7, 2016

liggitt Jan 9, 2016

Choose a reason for hiding this comment

liggitt Jan 9, 2016

Choose a reason for hiding this comment

liggitt Jan 9, 2016

Choose a reason for hiding this comment

jayunit100 Jan 9, 2016

Choose a reason for hiding this comment

liggitt Jan 9, 2016

Choose a reason for hiding this comment

timothysc commented Jan 12, 2016

jeremyeder commented Jan 12, 2016

jayunit100 commented Jan 13, 2016

jayunit100 commented Jan 16, 2016

jayunit100 commented Feb 5, 2016