Use a random port for the test etcd server #15586

ixdy · 2015-10-14T00:36:09Z

This allows us to run multiple instances of the verification checks, the integration test, etc. simultaneously on a single host (e.g. the PR Jenkins).

ref #14781

@kubernetes/goog-testing

k8s-bot · 2015-10-14T00:59:38Z

GCE e2e test build/test passed for commit d9838d7f93978dd428625ed8c383354eb800a601.

k8s-github-robot · 2015-10-14T01:05:25Z

Labelling this PR as size/L

wojtek-t · 2015-10-14T09:51:25Z

cmd/integration/integration.go

@@ -955,6 +959,7 @@ type testFunc func(*client.Client)
 func addFlags(fs *pflag.FlagSet) {
 	fs.IntVar(
 		&maxConcurrency, "max-concurrency", -1, "Maximum number of tests to be run simultaneously. Unlimited if set to negative.")
+	fs.StringVar(&etcdServer, "etcd-server", "", "etcd server URL.")


this also needs port, right? (maybe ... URL in http://host:port format)?

Also, how about being consistent with other places and use "etcd-servers" instead to handle multiple etcd servers too:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kube-apiserver/app/server.go#L214

Or make it consistent with the other integration tests... weird to use a flag in one place and env vars in the other.

wojtek-t · 2015-10-14T10:07:37Z

Just one minor comment - other than that LGTM.

lavalamp · 2015-10-14T16:37:52Z

hack/lib/etcd.sh

-  kube::log::info "etcd -data-dir ${ETCD_DIR} --bind-addr ${host}:${port} >/dev/null 2>/dev/null"
-  etcd -data-dir ${ETCD_DIR} --bind-addr ${host}:${port} >/dev/null 2>/dev/null &
+  kube::log::info "Starting etcd with data-dir ${ETCD_DIR} and bind-addr ${ETCD_HOST}:${ETCD_PORT}"
+  etcd -data-dir ${ETCD_DIR} --bind-addr ${ETCD_HOST}:${ETCD_PORT} >/dev/null 2>/dev/null &


If it fails, retry with different port?

I think you need to change the negotiation port, too? (the 7001 port)

lavalamp · 2015-10-14T16:41:43Z

cc @timothysc

timothysc · 2015-10-14T16:53:24Z

@ixdy this affects the integration tests but do we run the UTs concurrently at all?

I'm currently working on a PR which will standup and teardown etcd on a well known port for the UTs. If they are run concurrently upstream then that could be an issue.

lavalamp · 2015-10-14T21:35:10Z

Yes, we run unit tests concurrently (but before the integration tests).

The tests in test/integration are also possibly run concurrently with each
other?

On Wed, Oct 14, 2015 at 9:53 AM, Timothy St. Clair <notifications@github.com

wrote:

@ixdy https://github.com/ixdy this affects the integration tests but do
we run the UTs concurrently at all?

—
Reply to this email directly or view it on GitHub
#15586 (comment)
.

k8s-bot · 2015-10-15T23:07:31Z

GCE e2e test build/test passed for commit 190e89b1601bf0173ee129caabe6f8539e56fea8.

k8s-bot · 2015-10-16T07:30:05Z

GCE e2e test build/test passed for commit d80665e9f3ca939ca964271f4813f754f81b02e0.

timothysc · 2015-10-16T13:37:27Z

@lavalamp, my mods are available and starts up an etcd on a random port. imho we could probably leverage in the integration tests as well...

#15392

ixdy · 2015-10-19T20:51:53Z

The local etcd is started in a number of tests:

hack/local-up-cluster.sh:    kube::etcd::start
hack/test-update-storage-objects.sh:kube::etcd::start
hack/after-build/update-swagger-spec.sh:kube::etcd::start
hack/test-cmd.sh:kube::etcd::start
hack/test-integration.sh:  kube::etcd::start
hack/benchmark-integration.sh:  kube::etcd::start

My main focus here was on update-swagger-spec.sh (used in the verification scripts), test-cmd.sh, and test-integration.sh, since we'd like to start running all of those in the PR Jenkins instance, which runs several builds in parallel.

While the etcd stuff seems to work now in parallel, I've noticed that the integration test (in particular cmd/integration) still uses static ports, so the integration test is still not parallel-invocation-safe.

Also refactor etcd client utils in integration test to reduce code duplication.

k8s-bot · 2015-10-20T02:31:48Z

GCE e2e build/test failed for commit 785c0cd.

timothysc · 2015-10-20T13:57:51Z

imho #15392 should solve this issue, and randomizes the port properly to the clients to run in parallel.

The issue will be updating the integration tests to leverage.

ixdy · 2015-10-20T23:50:22Z

@timothysc will that also work for hack/after-build/update-swagger-spec.sh, which starts an apiserver and thus needs etcd?

ixdy · 2015-10-29T18:58:26Z

Closing since #16287 makes this unnecessary.

ixdy added the area/test-infra label Oct 14, 2015

googlebot added the cla: yes label Oct 14, 2015

k8s-github-robot assigned lavalamp Oct 14, 2015

k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 14, 2015

wojtek-t reviewed Oct 14, 2015
View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2015

lavalamp reviewed Oct 14, 2015
View reviewed changes

ixdy mentioned this pull request Oct 15, 2015

Add Jenkins shell script to setup and run verification tests #14587

Merged

ixdy force-pushed the etcd-in-tests branch from d9838d7 to 190e89b Compare October 15, 2015 22:40

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2015

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2015

ixdy force-pushed the etcd-in-tests branch from 190e89b to d80665e Compare October 16, 2015 07:05

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2015

ixdy added 6 commits October 19, 2015 19:02

Make get_random_port only return unprivileged ports

acf3208

Make etcd server address configurable in integration test

7099e7e

Also refactor etcd client utils in integration test to reduce code duplication.

Use a random port for the test etcd server

73f9cd3

Use a better random seed in get_random_port

3e1109c

Randomize etcd peer port and retry 3 times

d61c7b6

Standardize on flag "--etcd-servers" in integration test

785c0cd

ixdy force-pushed the etcd-in-tests branch from d80665e to 785c0cd Compare October 20, 2015 02:02

ixdy closed this Oct 29, 2015

ixdy deleted the etcd-in-tests branch December 1, 2015 01:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a random port for the test etcd server #15586

Use a random port for the test etcd server #15586

ixdy commented Oct 14, 2015

k8s-bot commented Oct 14, 2015

k8s-github-robot commented Oct 14, 2015

wojtek-t Oct 14, 2015

lavalamp Oct 14, 2015

wojtek-t commented Oct 14, 2015

lavalamp Oct 14, 2015

lavalamp Oct 14, 2015

lavalamp commented Oct 14, 2015

timothysc commented Oct 14, 2015

lavalamp commented Oct 14, 2015

k8s-bot commented Oct 15, 2015

k8s-bot commented Oct 16, 2015

timothysc commented Oct 16, 2015

ixdy commented Oct 19, 2015

k8s-bot commented Oct 20, 2015

timothysc commented Oct 20, 2015

ixdy commented Oct 20, 2015

ixdy commented Oct 29, 2015

Use a random port for the test etcd server #15586

Use a random port for the test etcd server #15586

Conversation

ixdy commented Oct 14, 2015

k8s-bot commented Oct 14, 2015

k8s-github-robot commented Oct 14, 2015

wojtek-t Oct 14, 2015

Choose a reason for hiding this comment

lavalamp Oct 14, 2015

Choose a reason for hiding this comment

wojtek-t commented Oct 14, 2015

lavalamp Oct 14, 2015

Choose a reason for hiding this comment

lavalamp Oct 14, 2015

Choose a reason for hiding this comment

lavalamp commented Oct 14, 2015

timothysc commented Oct 14, 2015

lavalamp commented Oct 14, 2015

k8s-bot commented Oct 15, 2015

k8s-bot commented Oct 16, 2015

timothysc commented Oct 16, 2015

ixdy commented Oct 19, 2015

k8s-bot commented Oct 20, 2015

timothysc commented Oct 20, 2015

ixdy commented Oct 20, 2015

ixdy commented Oct 29, 2015