-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow concurrent runs of kubeadm e2e job. #2265
Allow concurrent runs of kubeadm e2e job. #2265
Conversation
This will need to disable checks for leaked resources -- otherwise you're likely to encounter flakiness around extra resources existing when testing ends (created by other runs of the job) |
it's already handled in https://github.com/kubernetes/test-infra/blob/master/scenarios/kubernetes_e2e.py#L301-L302, guess better just handle https://github.com/kubernetes/test-infra/blob/master/jobs/ci-kubernetes-e2e-kubeadm-gce.sh#L34 and migrate it over. |
add something like
|
hummmm and now |
@fejta As it turns out, resource leaks are already disabled for this (since the https://github.com/kubernetes/test-infra/blob/master/jobs/ci-kubernetes-e2e-kubeadm-gce.sh#L28 |
Thanks for the clarification! /assign @krzyzacy |
7158b3a
to
2eab8b3
Compare
@@ -33,13 +33,18 @@ export KUBERNETES_PROVIDER=kubernetes-anywhere | |||
# succeeded. | |||
export SCM_VERSION=$(./hack/print-workspace-status.sh | grep ^STABLE_BUILD_SCM_REVISION | cut -d' ' -f2) | |||
|
|||
export E2E_NAME="e2e-kubeadm-gce" | |||
export E2E_NAME="e2e-kubeadm-${BUILD_NUMBER:=0}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
${BUILD_NUMBER:-0}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
2eab8b3
to
5a1a37b
Compare
in |
I've updated the commit to set |
@krzyzacy Yikes at that assertion. I think in this case, converting to a regex and matching |
@rmmh It publishes build results based on the job name: https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-gce/504/ |
Since resources are named using this cluster name, include the BUILD_NUMBER so it should differ between multiple concurrent runs in the same project. Also, adjust bootstrap_test.py to allow the word "resource" to be used.
5a1a37b
to
0866d08
Compare
Okay, I patched the bootstrap_test assertion, and ran it with a few changes to my job to make sure the regex was sane:
|
/lgtm I'll try to rebase #2141, wonder if there's other ways to get |
Thanks, @krzyzacy! |
@@ -1814,7 +1814,7 @@ def testJobsDoNotSourceShell(self): | |||
continue # No clean way to determine version | |||
with open(job_path) as fp: | |||
script = fp.read() | |||
self.assertNotIn('source ', script, job) | |||
self.assertFalse(re.search(r'\Wsource ', script), job) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This no longer works:
re.search(r'\Wsource', 'source foo.sh')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, there's going to be a '\n' before, nice!
seems still troubled, who generates |
That's a separate race condition I have a PR out to fix: kubernetes-retired/kubernetes-anywhere#357 |
Since resources are named using this cluster name, include the
BUILD_NUMBER
so it should differ between multiple concurrent prow runs in the same project.Here's an example of one such failure, complaining about resources already existing: https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-gce/503?log#log