Download all binaries instead of building them locally for compat version tests #34930
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @michaelasp. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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-sigs/prow repository. |
k8s-ci-robot
added
needs-ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: michaelasp The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
sig/testing
|
/cc @BenTheElder |
| build_docker(){ | ||
| build/run.sh make all WHAT="cmd/kubectl cmd/kubelet" 1> /dev/null | ||
| make quick-release-images 1> /dev/null | ||
| download_docker(){ |
There was a problem hiding this comment.
this name is misleading (the old one is too, it's a carry over from when we had a bazel build mode, but this is even more misleading)
maybe "download_images"
| } | ||
|
|
||
| download_current_version_bins() { | ||
| wget https://dl.k8s.io/ci/$(curl -Ls https://dl.k8s.io/ci/latest.txt)/kubernetes-test-$(go env GOOS)-$(go env GOARCH).tar.gz |
There was a problem hiding this comment.
maybe something like this:
| wget https://dl.k8s.io/ci/$(curl -Ls https://dl.k8s.io/ci/latest.txt)/kubernetes-test-$(go env GOOS)-$(go env GOARCH).tar.gz | |
| wget 'https://dl.k8s.io/ci/'"${1}"/'kubernetes-test-'"$(go env GOOS)-$(go env GOARCH)"'.tar.gz' |
and pass in the version (see below)
There was a problem hiding this comment.
Makes sense, this can make for some nasty edge cases. I made it so only the create_cluster step obtains the latest_version. I think that's the best place to put it in common.sh since we need to create a cluster in every test and we have access to artifacts at that point.
| @@ -235,6 +235,30 @@ build_prev_version_bins() { | |||
| echo "Finished building e2e.test binary from ${PREV_RELEASE_BRANCH}." | |||
| } | |||
|
|
|||
| download_prev_version_bins() { | |||
| local version=$1 | |||
| wget https://dl.k8s.io/release/$(curl -Ls https://dl.k8s.io/release/stable-${version}.txt)/kubernetes-test-$(go env GOOS)-$(go env GOARCH).tar.gz | |||
There was a problem hiding this comment.
TODO: this script as-written will only work on a linux host, but it should work on mac too.
For that you need to split binaries being downloaded for kind (GOOS should always be linux) versus for the host (like the test binaries)
| @@ -32,8 +32,9 @@ CONTROL_PLANE_COMPONENTS="kube-apiserver kube-controller-manager kube-scheduler" | |||
| KUBE_ROOT="." | |||
| # KUBE_ROOT="$(go env GOPATH)/src/k8s.io/kubernetes" | |||
| source "${KUBE_ROOT}/hack/lib/version.sh" | |||
| kube::version::get_version_vars | |||
| DOCKER_TAG=${KUBE_GIT_VERSION/+/_} | |||
| LATEST_IMAGE=$(curl -Ls https://dl.k8s.io/ci/latest.txt) | |||
There was a problem hiding this comment.
we should make sure to use this value when downloading the current version, otherwise we're going to wind up with a race where we record one version and test another.
| LATEST_IMAGE=$(curl -Ls https://dl.k8s.io/ci/latest.txt) | |
| LATEST_VERSION=$(curl -Ls https://dl.k8s.io/ci/latest.txt) |
There was a problem hiding this comment.
Ack, made it so only the common script modifies it in the create cluster step.
|
Thanks, sorry for the delay (PRR crunch...), some comments above ... |
| make quick-release-images 1> /dev/null | ||
| download_docker(){ | ||
| curl -L 'https://dl.k8s.io/ci/'${LATEST_IMAGE}'/kubernetes-server-linux-amd64.tar.gz' > kubernetes-server-linux-amd64.tar.gz | ||
| tar -xvf kubernetes-server-linux-amd64.tar.gz |
There was a problem hiding this comment.
You probably want to stage files in a tempdir instead of in the current working directory, it can make the repo dirty (also, they might not get cleaned up ...)
This server tarball can also be used to build a node image, so we don't need to checkout and build k/k at all.
kind build node-image --type file kubernetes-server-linux-amd64.tar.gz
https://kind.sigs.k8s.io/docs/user/quick-start/#building-images
There was a problem hiding this comment.
Done! I don't think we build k/k at all here, the create cluster step uses a precompiled image. I did however remove the cloning of the k8s repo, this is no longer necessary since we pull the precompiled binary.
|
/cc @Jefftree |
This pr downloads all required docker and test binaries and uses those to run the tests rather than building them locally. Running these tests locally showed a major performance improvement from this.
The two major changes are downloading all the control plane docker images rather than building them from source and also downloading the test binaries instead of building them to run conformance tests.