Install etcd in the kubekins-test image #522
Conversation
ixdy
force-pushed
the
etcd-in-kubekins-test
branch
from
785e764
to
ab44d46
Compare
|
I'm not a big fan of this. The S3 download is flaky. It could be replaced with a gsutil cat instead, and be more reliable (since gsutil includes retries). How would a PR that upgrades etcd be tested? |
|
IMO downloading unchanging dependencies at runtime is an anti-pattern. New versions would likely be tested locally. We also have facilities to create a new |
|
I still think that having the test code go through a proxy, and caching all downloads there, is a better way to go. That way we wouldn't have to push a new test image every time someone changes the etcd version, and we get this behavior automatically for all dependencies. |
|
I don't have the bandwidth to do something like that right now. If that's the direction we want to go, someone else will need to own it. etcd downloads have been rather flaky of late, see kubernetes/kubernetes#31191 (comment) and kubernetes/kubernetes#31492 (comment). |
|
I'll have a go at it. |
|
It may make sense to check with @fejta, if he's in. I'm just worried that On Tue, Sep 6, 2016 at 5:04 PM, Ryan Hitchman notifications@github.com
|
|
I prefer this to caching on GCS. Even if the S3 download is flaky it only needs to happen once every time we update etcd in the image. It's pretty easy to build and push a new |
|
Prior art: the cross-build image has included etcd since inception, I guess cause we run integration tests inside it? (We shouldn't be running integration tests inside the cross-build image. ugh.) |
|
I see installing etcd as a prerequisite for testing. I do not see installing etcd as something we are intentionally testing. I would prefer to have etcd (and all other prerequisites) already installed and configured in the image used for integration testing. On the other hand I would strongly prefer that the image used for unit and e2e testing does not have any references to etcd, such that people do mistakenly do integration testing in the wrong location... However since my understanding is that a) many unit tests use etcd and b) we only have a single image used for all types of testing I am happy to leave this second part for a future exercise. |
| @@ -43,6 +43,13 @@ RUN apt-get -o Acquire::Check-Valid-Until=false update && apt-get install -y \ | |||
| --no-install-recommends \ | |||
| && rm -rf /var/lib/apt/lists/* | |||
|
|
|||
| # Download and symlink etcd. We need this for verification and integration tests. | |||
| RUN export ETCD_VERSION=v3.0.4; \ | |||
There was a problem hiding this comment.
This is super cool! I love that changing the ETCD version requires a) a PR to update this line and b) pushing a new image.
On the other hand, is there any chance this can more closely follow the instructions at https://github.com/kubernetes/kubernetes/blob/master/docs/devel/testing.md#install-etcd-dependency? It is not clear that hack/install-etcd.sh does the following... in fact this says it installs it into ./third_party/etcd.
There was a problem hiding this comment.
Modifying just the hack/install-etcd.sh script keeps the weirdness
localized. If we want to have etcd always available in $PATH and not use
hack/install-etcd.sh at all in our CI, that's fine too.
On Thu, Sep 8, 2016 at 9:01 PM, Erick Fejta notifications@github.com
wrote:
In jenkins/test-image/Dockerfile
#522 (comment):@@ -43,6 +43,13 @@ RUN apt-get -o Acquire::Check-Valid-Until=false update && apt-get install -y
--no-install-recommends
&& rm -rf /var/lib/apt/lists/*+# Download and symlink etcd. We need this for verification and integration tests.
+RUN export ETCD_VERSION=v3.0.4; \This is super cool! I love that changing the ETCD version requires a) a PR
to update this line and b) pushing a new image.On the other hand, is there any chance this can more closely follow the
instructions at https://github.com/kubernetes/kubernetes/blob/master/docs/
devel/testing.md#install-etcd-dependency? It is not clear that
hack/install-etcd.sh does the following... in fact this says it installs it
into ./third_party/etcd.—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/test-infra/pull/522/files/ab44d468ccc40a462a03620ca092a2511df5b6b8#r78125897,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAM7nEzdUXSyf0qOW1dkoS6Nrfm_y9IIks5qoNn8gaJpZM4J2WxO
.
There was a problem hiding this comment.
the problem with using hack/install-etcd.sh (which also is the case with the fix I have, and with the go version we use, etc) is that different releases of kubernetes use different versions of etcd, Go, etc.
right now we run tests on all release branches with the latest image, which will likely result in incorrect behavior for older branches.
There was a problem hiding this comment.
hack/install-etcd.sh from HEAD is packaged in an image and used in all branch testing?
|
closing for now. we can revisit when we have test images for each release. |
Automatic merge from submit-queue Change cluster default name and add more configurable **Release note**: ```release-note none ``` Cluster name changed by https://github.com/istio/istio/pull/902/files#diff-4d4b343aca91a15218a827b3b0385f10R36
Address some complaints from kubernetes/kubernetes#31915 (though it doesn't help a developer's local workflow).
Also a step towards fixing the "downloading etcd" flake.
Next steps:
a) push a new
kubekins-testimage and use itb) update all of the Jenkins scripts to use
etcdfrom the PATH rather than downloading itWe probably also need to document somewhere that changes in kubernetes/kubernetes to the default etcd version need to be included in this repo too.
cc @david-mcmahon @kubernetes/test-infra-maintainers
This change is