Use a consistent build id between bazel/kubeadm pull jobs. #2509
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
/assign I would like the two of us to work on a plan to migrate these jobs (and all the other kubeadm jobs) to a python scenario like the other PR jobs: https://github.com/kubernetes/test-infra/blob/master/jobs/config.json#L286 Also jobs should be hermetic, we should depend on the output from job X in order to run Y. |
|
Sure, this particular job (the pull variant) is the only one that uses a shell script, and should be easy to convert. The CI and periodic variants of kubeadm e2e jobs are all scenario-based. |
|
Any chance I can convince you to do this in this PR or would you prefer to defer to a later one? |
|
Sure. I think the two changes are fundamentally isolated, though, so I'll keep them as separate commits. |
|
/lgtm I would really prefer to see these shell scripts die in a fiery ball of death |
k8s-ci-robot
added
the
lgtm
pipejakob
force-pushed
the
bazel-pull-ci-id-pr
branch
2 times, most recently
from
1b011d1 to
431379e
Compare
|
Okay, I've added a second commit to convert the pull job to the |
|
(After I fix all of those new bazel failures, of course!) |
pipejakob
force-pushed
the
bazel-pull-ci-id-pr
branch
5 times, most recently
from
ab983de to
dcf0219
Compare
pipejakob
removed
the
lgtm
scenarios/kubernetes_e2e.py
Outdated
| @@ -313,28 +339,15 @@ def main(args): | |||
| cluster = args.cluster or 'e2e-gce-%s-%s' % ( | |||
| os.environ['NODE_NAME'], os.getenv('EXECUTOR_NUMBER', 0)) | |||
|
|
|||
| if args.kubeadm: | |||
| if args.kubeadm is not None: | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Hunh, I thought I was having trouble with that syntax after changing this from a boolean, but it definitely works. I've reverted it back to the original "if args.kubeadm:".
|
Fabulous, thanks so much for doing this! /cc @krzyzacy |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
| @@ -252,6 +254,8 @@ presubmits: | |||
| - "--pull=$(PULL_REFS)" | |||
| - "--upload=gs://kubernetes-jenkins/pr-logs" | |||
| - "--git-cache=/root/.cache/git" | |||
| - "--timeout=75" | |||
| - "--json" | |||
There was a problem hiding this comment.
maybe now --json can be in the image
There was a problem hiding this comment.
Oh yeah, good point. I'll add a personal TODO to follow this up with another PR.
|
/lgtm |
Since the kubeadm e2e job uses the CI artifacts pushed by the bazel job, make sure we use a consistent id for their storage (the Kubernetes version isn't stable for pull jobs since each job pulls and applies the PR at different times, resulting in different commit SHAs).
This also changes the --kubeadm boolean flag in kubernetes_e2e.py to an option to specify the type of job (ci, pull, periodic) so that it knows where to find the .deb artifacts created by the previous bazel job, since the output location can differ by type.
pipejakob
force-pushed
the
bazel-pull-ci-id-pr
branch
from
dcf0219 to
c0287dd
Compare
Since the kubeadm e2e job uses the CI artifacts pushed by the bazel job, make sure we use a consistent id for their storage (the Kubernetes version isn't stable for pull jobs since each job pulls and applies the PR at different times, resulting in different commit SHAs).
The
PULL_NUMBERisn't actually required for uniqueness of this location, but having it as part of the hierarchy makes it easy to find all bazel builds for a PR since this identifier is much friendlier thanPULL_REFS, e.g.: