Fixes to kubekins test-image, add makefile #16287

j3ffml · 2015-10-26T18:08:47Z

Fix issues, address comments from #16140. Also, closes #14781

@ixdy, to answer "why separate gotest-pr.sh"; tests run in container as root, so build/test output files in the workspace get owned by root as well. This requires extra cleanup steps I didn't want to reproduce for the kubernetes-test-go job, but I can if you think it's worth doing.

k8s-github-robot · 2015-10-26T18:18:51Z

Labelling this PR as size/S

ashcrow · 2015-10-26T18:32:25Z

hack/jenkins/test-image/Makefile

+	gcloud docker push gcr.io/google_containers/kubekins-test # Push image tagged as latest to repository
+	gcloud docker push gcr.io/google_containers/kubekins-test:$(TAG) # Push version tagged image to repository (since this image is already pushed it will simply create or update version tag)
+
+clean:


No generated files, so nothing to clean. Empty target is the pattern used elsewhere.

k8s-bot · 2015-10-26T18:35:34Z

GCE e2e test build/test passed for commit c114b9d27f2f522c636429745d0aad871abe565a.

j3ffml · 2015-10-26T18:43:42Z

@k8s-bot unit test this

ixdy · 2015-10-27T00:21:39Z

hack/jenkins/test-image/run.sh

@@ -42,6 +38,10 @@ export KUBE_COVERPROCS=8
 export KUBE_INTEGRATION_TEST_MAX_CONCURRENCY=4
 export LOG_LEVEL=4

+./hack/build-go.sh
+godep go install ./...
+./hack/install-etcd.sh


maybe make this ./hack/install-etcd.sh || ./hack/travis/install-etcd.sh?

or we can cherrypick #15336 into release-1.0 and release-1.1.

Unfortunately #15336 is only one of several changes in test scripts between 1.0/1.1 and head. We can either go through and cherrypick all the changes, or we can have a branch-specific hack/jenkins/test-image/run.sh for each branch that runs the version of verify* scripts that exist on the branch.

another option (maybe this is getting into too much indirection?) is that this file calls hack/jenkins/gotest.sh (or equivalent).

the advantage there is that the list of tests run follows the branch, rather than using the same list across all builds. using the same run.sh for all branches means that we'll have to have special cases.

ixdy · 2015-10-27T00:28:54Z

I guess my uneasiness with having gotest.sh vs. gotest-pr.sh is that we have yet another place where the list of tests to run is specified, with a high probability that things will drift out of sync.

Eventually we should probably just make kubernetes-test-go use the containerized version. Then we can retire gotest.sh or whatever. (I guess gotest-dockerized.sh is a better name than gotest-pr.sh, in that case?)

j3ffml · 2015-10-27T16:49:01Z

Please hold off reviewing further, I'm going to see if I can tweak this to make it run on branches with minimal cherrypicking.

k8s-github-robot · 2015-10-27T18:25:05Z

Labelling this PR as size/M

k8s-bot · 2015-10-27T18:51:10Z

GCE e2e build/test failed for commit de913a4a24ef89d71fcd7b111bcf8a90b237a2aa.

j3ffml · 2015-10-28T00:02:39Z

Ok I think this is in a more reasonable state now, ptal. Should be able to cherrypick this onto 1.0 and 1.1 and just modify hack/jenkins/test-dockerized.sh to work for old script names.

Also cc @nikhiljindal for changes to hack/verify-api-reference-docs.sh

j3ffml · 2015-10-28T00:07:29Z

@ixdy, I took your idea to rename the test script gotest-dockerized.sh. Once this is in, I'm happy to update kubernetes-test-go to use this version and retire hack/jenkins/gotest.sh

nikhiljindal · 2015-10-28T00:23:30Z

Changes to update-api-reference-docs LGTM.
Note that that script does not exist in 1.0 or 1.1

cc @caesarxuchao

k8s-bot · 2015-10-28T00:29:34Z

GCE e2e test build/test passed for commit 56132779f3b44259a61eb4357a0dbd03c54bb51e.

j3ffml · 2015-10-28T00:31:55Z

@nikhiljindal, yup, not planning on cherrypicking that script onto release branches; test-dockerized.sh will just call verify-all.sh to run whatever exists on the branch.

caesarxuchao · 2015-10-28T00:38:43Z

hack/verify-api-reference-docs.sh

+# We want to pass absolute dir to update-api-reference-docs.sh, but diff
+# against local path to dir. This only matters in the case where this
+# test is itself running inside a container (with the host docker mapped in).
+REPO_DIR=${REPO_DIR:-"${PWD}/${KUBE_ROOT}"}


I thought ${KUBE_ROOT} will become an absolute path after running source "${KUBE_ROOT}/hack/lib/init.sh", I guess this is not the case if you are running the script inside a container?

You are correct, thanks for pointing out. As it was, this was broken when run standalone. Comment was a bit disingenuous, now describes the actual reason for using REPO_DIR if provided.

Thanks for the fix.

k8s-bot · 2015-10-28T01:24:02Z

GCE e2e test build/test passed for commit 176008430220d6604306aa78ad1b1c6a3674a0ff.

k8s-bot · 2015-10-28T19:29:53Z

GCE e2e test build/test passed for commit 349d4bba6e3c7b815084bf4ecf26969c90d5a2ec.

ixdy · 2015-10-28T21:16:16Z

I think the test image needs file after #16160. It's otherwise failing builds, e.g. #16382 (comment)

j3ffml · 2015-10-29T05:48:28Z

@k8s-bot unit test this

ixdy · 2015-10-29T07:31:14Z

hack/verify-api-reference-docs.sh

+# Use REPO_DIR if provided so we can set it to the host-resolvable path
+# to the repo root if we are running this script from a container with
+# docker mounted in as a volume.
+# We pass the host output dir to update-api-reference-docs.sh, but use


So I just want to fully understand what's happening here - HOST_OUTPUT_DIR and TMP_OUTPUT_DIR end up mapping to the same files always, even if the paths are different, all due to docker wonkiness?

Correct. They point to the same files, you just need a different absolute path to resolve them on the host vs from inside the test container.

k8s-bot · 2015-10-29T16:49:15Z

GCE e2e test build/test passed for commit ea6ec21d7e2cf5917126199aadea2b30a7e092ea.

ixdy · 2015-10-29T18:00:53Z

LGTM, but please squash commits.

(A side note: Shippable currently tests 2 versions of Go. Do we want to keep testing 2 versions or only the latest?)

Also add a makefile. This will need a cherrypick onto 1.0,1.1 with edits to hack/jenkins/test-dockerized.sh to run branch-specific test scripts. Also had to modify hack/verify-api-reference.sh to handle volume mount path peculiarity when doing `docker run -v` inside a container started with `docker run -v`. See associated comment in hack/jenkins/test-dockerized.sh

j3ffml · 2015-10-29T18:17:58Z

Squashed.

Regarding testing multiple versions, I think it's fine to just do 1.4 for now. We're in the process of bumping to 1.5 (#13838), so we'll probably care about testing 1.4, 1.5. We can accomplish that by building a second test-image with go 1.5.1 as the base image and modifying gotest-dockerized.sh to run the tests with both images. However I'd like to get this in as is, since it will make it easier to debug/fix the tests that are broken on 1.5.1.

j3ffml · 2015-10-29T18:18:06Z

@k8s-bot unit test this

ixdy · 2015-10-29T18:22:48Z

assuming Jenkins passes lgtm

k8s-bot · 2015-10-29T18:38:29Z

GCE e2e test build/test passed for commit 04e891c.

k8s-github-robot · 2015-10-29T18:57:25Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2015-10-29T19:29:12Z

GCE e2e test build/test passed for commit 04e891c.

k8s-github-robot · 2015-10-29T19:29:27Z

Automatic merge from submit-queue

Auto commit by PR queue bot

…16287-#16546-#16565-upstream-release-1.1 Automated cherry pick of #16287 #16546 #16565

…16287-#16546-#16565-upstream-release-1.0 Automated cherry pick of #16287 #16546 #16565

…-pick-of-#16287-kubernetes#16546-kubernetes#16565-upstream-release-1.1 Automated cherry pick of kubernetes#16287 kubernetes#16546 kubernetes#16565

j3ffml added the area/test-infra label Oct 26, 2015

j3ffml assigned ixdy Oct 26, 2015

googlebot added the cla: yes label Oct 26, 2015

k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 26, 2015

ashcrow reviewed Oct 26, 2015
View reviewed changes

ixdy reviewed Oct 27, 2015
View reviewed changes

j3ffml force-pushed the fix-jenkins-test-image branch from c114b9d to de913a4 Compare October 27, 2015 18:24

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 27, 2015

j3ffml force-pushed the fix-jenkins-test-image branch from de913a4 to 5613277 Compare October 27, 2015 23:57

caesarxuchao reviewed Oct 28, 2015
View reviewed changes

j3ffml mentioned this pull request Oct 28, 2015

Automated cherry pick of #13550 #14445 #14580 #16399

Merged

j3ffml force-pushed the fix-jenkins-test-image branch from 1760084 to 349d4bb Compare October 28, 2015 18:53

ixdy reviewed Oct 29, 2015
View reviewed changes

j3ffml force-pushed the fix-jenkins-test-image branch from 49cd8f4 to ea6ec21 Compare October 29, 2015 16:21

j3ffml force-pushed the fix-jenkins-test-image branch from ea6ec21 to 04e891c Compare October 29, 2015 18:08

ixdy added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 29, 2015

ixdy mentioned this pull request Oct 29, 2015

Use a random port for the test etcd server #15586

Closed

k8s-github-robot pushed a commit that referenced this pull request Oct 29, 2015

Merge pull request #16287 from jlowdermilk/fix-jenkins-test-image

0d29759

Auto commit by PR queue bot

k8s-github-robot merged commit 0d29759 into kubernetes:master Oct 29, 2015

a-robinson added a commit that referenced this pull request Oct 30, 2015

Merge pull request #16566 from jlowdermilk/automated-cherry-pick-of-#…

2b3ff49

…16287-#16546-#16565-upstream-release-1.1 Automated cherry pick of #16287 #16546 #16565

a-robinson added a commit that referenced this pull request Oct 30, 2015

Merge pull request #16568 from jlowdermilk/automated-cherry-pick-of-#…

a7ddccb

…16287-#16546-#16565-upstream-release-1.0 Automated cherry pick of #16287 #16546 #16565

j3ffml deleted the fix-jenkins-test-image branch October 30, 2015 23:03

j3ffml mentioned this pull request Nov 6, 2015

Don't update api-reference docs if the only changes are the timestamps #16940

Merged

Fixes to kubekins test-image, add makefile #16287

Fixes to kubekins test-image, add makefile #16287

Conversation

j3ffml commented Oct 26, 2015

k8s-github-robot commented Oct 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Oct 26, 2015

j3ffml commented Oct 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ixdy commented Oct 27, 2015

j3ffml commented Oct 27, 2015

k8s-github-robot commented Oct 27, 2015

k8s-bot commented Oct 27, 2015

j3ffml commented Oct 28, 2015

j3ffml commented Oct 28, 2015

nikhiljindal commented Oct 28, 2015

k8s-bot commented Oct 28, 2015

j3ffml commented Oct 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Oct 28, 2015

k8s-bot commented Oct 28, 2015

ixdy commented Oct 28, 2015

j3ffml commented Oct 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Oct 29, 2015

ixdy commented Oct 29, 2015

j3ffml commented Oct 29, 2015

j3ffml commented Oct 29, 2015

ixdy commented Oct 29, 2015

k8s-bot commented Oct 29, 2015

k8s-github-robot commented Oct 29, 2015

k8s-bot commented Oct 29, 2015

k8s-github-robot commented Oct 29, 2015