Support differentiation of OS distro in e2e tests #23466
Conversation
|
Labelling this PR as size/S |
k8s-github-robot
added
the
size/S
|
GCE e2e build/test passed for commit 745c0170ba7f140834546442b6efd79654d8163d. |
|
GCE e2e build/test failed for commit d89c3340198ae428991e2336b494a7df193add2f. Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake. |
|
GCE e2e build/test passed for commit d89c3340198ae428991e2336b494a7df193add2f. |
|
@ixdy we need to get this merged and cherry picked in 1.2 branch. Otherwise, the test case kube-addons update will keep failing in our 1.2 Jenkins. Please add a "cherrypick-candidate" label and mark it as 1.2 milestone. Thanks |
|
I added the label & milestone. |
| @@ -102,6 +102,7 @@ export PATH=$(dirname "${e2e_test}"):"${PATH}" | |||
| --repo-root="${KUBE_ROOT}" \ | |||
| --node-instance-group="${NODE_INSTANCE_GROUP:-}" \ | |||
| --prefix="${KUBE_GCE_INSTANCE_PREFIX:-e2e}" \ | |||
| --os-distro="${OS_DISTRIBUTION:-debian}" \ | |||
There was a problem hiding this comment.
I'd prefer keeping defaults out of this file, leaving them only in e2e.go. instead do something like
${OS_DISTRIBUTION:+"--os-distro=${OS_DISTRIBUTION}"} \so that we only include this flag when $OS_DISTRIBUTION is set.
There was a problem hiding this comment.
also, is the canonical environment variable OS_DISTRIBUTION or KUBE_OS_DISTRIBUTION? I see both littered throughout, though I suspect KUBE_OS_DISTRIBUTION is what we want.
There was a problem hiding this comment.
Will revise it.
@roberthbailey @ixdy, does jenkins running GKE tests use the env variable OS_DISTRIBUTION by default? If not, @wonderfly may need to add a line to export it in our own GKE jenkins jobs.
There was a problem hiding this comment.
does jenkins running GKE tests use the env variable OS_DISTRIBUTION by default?
OS_DISTRIBUTION has a default value of debian. Only Trusty e2e jobs reset it AFAIK, through KUBE_OS_DISTRIBUTION.
https://github.com/kubernetes/kubernetes/blob/master/cluster/gce/config-test.sh#L38
There was a problem hiding this comment.
this script doesn't source cluster/gce/config-test.sh though, so OS_DISTRIBUTION isn't going to be set.
There was a problem hiding this comment.
If so, I think we need to use KUBE_OS_DISTRIBUTION and export it in our trusty-based jenkins jobs.
|
cc @kubernetes/sig-testing (do we have any CoreOS folks in this team?) |
|
I think this may also cause issues for AWS, which uses a few different OS distros. |
|
I am not sure how important to make all e2e tests green on CoreOS or AWS. I guess the corresponding maintainer will make a call. CC them this change is a good idea. As for our "trusty" distro type, we have release cycles so we are trying to make everything green on it. |
|
@ixdy please check the newly updated version |
|
GCE e2e build/test failed for commit 57c807123b621bfd94dec67cc723e5ebe30fd72f. Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake. |
|
GCE e2e build/test passed for commit 364ac85. |
ixdy
added
the
lgtm
|
@wonderfly, after this PR is merged and cherry picked in 1.2 branch, please double check if our testing suite needs to explicitly export KUBE_OS_DISTRIBUTION=trusty. |
Noted. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 364ac85. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 364ac85. |
|
Automatic merge from submit-queue |
bgrant0607
added
the
cherry-pick-approved
|
@andyzheng0831 This cherrypick is approved, but you need to create the cherrypick PR in order for it to get into the patch release. |
|
@bgrant0607 Thanks for the info, I will make it. |
|
I'm struggling to understand why we're changing a timeout / polling value based on OS? What is it about the default value of |
|
Please refer to the original test code On Mon, Mar 28, 2016 at 11:36 AM, Aaron Crickenberger <
|
|
got it, completely misread, thanks |
…-#23466-upstream-release-1.2 Automated cherry pick of #23466
bgrant0607
added
the
release-note
|
Commit found in the "release-1.2" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
…ry-pick-of-#23466-upstream-release-1.2 Automated cherry pick of kubernetes#23466
…ry-pick-of-#23466-upstream-release-1.2 Automated cherry pick of kubernetes#23466
This change fixes issue #23221 by adding a flag "os-distro" in e2e testing framework. This flag inherits the value of env variable OS_DISTRIBUTION. The change also fixes the e2e test case of addon update for trusty by revising the testing code and kube-addons job in trusty master support.
I verified it by running 'go run hack/e2e.go --v --test --test_args="--ginkgo.focus=should\spropagate\sadd-on\sfile\schanges"'. Now this test case can pass on trusty.
@ixdy @roberthbailey @dchen1107 please review this change.
cc/ @wonderfly @fabioy FYI