kubeadm: pull kubernetes-anywhere per job #9117

neolit123 · 2018-08-21T21:13:55Z

kubernetes-anywhere (k-a) now has a specific branch for use with
e2e tests and it is no longer needed to bake a k-a commit SHA into
the kubeadm image.

ref:
kubernetes-retired/kubernetes-anywhere#555
kubernetes/kubernetes#66338

The following is used to pull the kubeadm-e2e branch of k-a:
--repo=k8s.io/kubernetes-anywhere=kubeadm-e2e

This removes a step where after fixing a k-a bug, the test-infra
kubeadm image has to be updated.

/area jobs
/assign @BenTheElder
/cc @timothysc
/cc @mohammedzee1000

neolit123 · 2018-08-21T21:16:48Z

images/kubeadm/runner

@@ -21,14 +21,6 @@ if [ ! -e test-infra ]; then
  git clone https://github.com/kubernetes/test-infra


should this be moved to --repo as well eventually?

this should be left up to the underlying runner in the base image, when using bootstrap.py we have a runner that does some things like that. that runner is also needed for EG the bazel cache.

really we should just do away with this runner being the entrypoint and do any of:

do things ahead of time, eg pre-installing tools in the image

let bootstrap check things out

let the testing script do things
instead of the runner / entrypoint.

i need to look up what the base image is.

neolit123 · 2018-08-21T21:17:20Z

images/kubeadm/runner

@@ -21,14 +21,6 @@ if [ ! -e test-infra ]; then
  git clone https://github.com/kubernetes/test-infra
 fi

-if [ ! -e kubernetes-anywhere ]; then


we have -e checks in the runner. i guess using --repo for the jobs instead would pull every time?
probably OK for smaller repos.

this doesn't really make sense, I don't think this check would be true under any circumstance

neolit123 · 2018-08-21T21:20:30Z

images/kubeadm/runner

-  # immediately break e2e jobs, and we have control over upgrading/downgrading.
-  git -C kubernetes-anywhere checkout a26f42c716028c4ba967b2d2f3de5ed50acc51da
-fi
-
 # This is required until https://github.com/kubernetes/kubernetes-anywhere/issues/332


i've re-opened this issue as it still exists.

neolit123 · 2018-08-21T21:21:53Z

images/kubeadm/runner

-  # immediately break e2e jobs, and we have control over upgrading/downgrading.
-  git -C kubernetes-anywhere checkout a26f42c716028c4ba967b2d2f3de5ed50acc51da
-fi
-
 # This is required until https://github.com/kubernetes/kubernetes-anywhere/issues/332
 # is implemented.
 if [ ! -e kubernetes-anywhere/phase1/gce/account.json ]; then


@BenTheElder you've mentioned that --repo would pull in $PWD/../kubernetes-anywhere
do i need to adjust the paths in this last part of the file to be ../kubernetes-anywhere/?

i also saw the discussion about not having a custom image.

i'm not exactly sure why the ln -s action is here and not in the kubernetes-anywhere tree, but is it possible to move the ./test-infra/jenkins/bootstrap.py call in the job?

this can go in a separate PR too, i guess.

So when the init scripts hand off to the testing code the current directory will be a checkout of the first --repo flag, which is typically the repo under test. Additional repos will be adjacent in ./../$OTHER_REPO.

I'm not sure why we do anything in this image. Ideally we'd use the underlying runner in the base image only and not do anything at runtime in this image.

timothysc · 2018-08-21T21:51:50Z

/lgtm

neolit123 · 2018-08-22T14:32:48Z

@BenTheElder
i've removed the code in question from the kubeadm/runner and made a k-a PR to add that code in there instead:
kubernetes-retired/kubernetes-anywhere#557

the kubeadm runner now looks like the one for gcloud:
https://github.com/kubernetes/test-infra/blob/master/images/gcloud/runner

i still don't understand what the base image is, but for the kubeadm image i'm seeing that it installs terraform, so i'm not exactly sure how the kubeadm image can be removed:
https://github.com/kubernetes/test-infra/blob/master/images/kubeadm/Dockerfile#L44-L49

possibly this has to happen in a separate PR.

neolit123 · 2018-08-27T13:00:34Z

le bump. 👀

BenTheElder · 2018-08-27T16:55:18Z

images/kubeadm/runner

@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-# Copyright 2017 The Kubernetes Authors.
+# Copyright 2018 The Kubernetes Authors.


we should drop this runner entirely, we can use the entrypoint from the base image

BenTheElder · 2018-08-27T16:57:16Z

Sorry I got behind on PR reviews, going through the backlog today.

i still don't understand what the base image is, but for the kubeadm image i'm seeing that it installs terraform, so i'm not exactly sure how the kubeadm image can be removed:

We don't need to remove this image, but we should drop the runner. The base images for this go like:
images/bootstrap (generic-ish base testing image) -> images/kubekins-e2e (generic-ish e2e / build image) -> images/kubeadm (adds tools specifically for kubernetes-anywhere).

BenTheElder · 2018-08-27T16:58:49Z

config/jobs/kubernetes/sig-cluster-lifecycle/kubeadm.yaml

@@ -9,6 +9,7 @@ periodics:
    - image: gcr.io/k8s-testimages/e2e-kubeadm:v20180730-c8036c3a7
      args:
      - --repo=k8s.io/kubernetes=master
+      - --repo=k8s.io/kubernetes-anywhere=kubeadm-e2e


skimming through this PR I don't see us doing this for other k-a jobs (the presubmits?) we should find them and do this for all k-a jobs

neolit123 · 2018-08-27T18:32:55Z

updated:

removed the kubeadm runner
added --repo 3 times for k-a in config/jobs/kubernetes/sig-testing/bazel-build-test.yaml as the periodics seem to be there now (?)

k8s-ci-robot · 2018-08-27T18:34:35Z

@neolit123: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-test-infra-gubernator	`b7798f9`	link	`/test pull-test-infra-gubernator`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

kubernetes-anywhere (k-a) now has a specific branch for use with e2e tests and it is no longer needed to bake a k-a commit SHA into the kubeadm image. The following is used to pull the `kubeadm-e2e` branch of k-a: `--repo=k8s.io/kubernetes-anywhere=kubeadm-e2e` This removes a step where after fixing a k-a bug, the test-infra kubeadm image has to be updated. Also removes the kubeadm runner as it is no longer needed.

BenTheElder

/lgtm
/hold
we should bump the images after this, will do that

k8s-ci-robot · 2018-08-28T16:38:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, neolit123, timothysc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [BenTheElder]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

BenTheElder · 2018-08-28T16:40:23Z

Thanks for tackling this! :-)

neolit123 · 2018-08-28T17:02:19Z

thanks a lot.
hope the tests go 📗 after this.

BenTheElder · 2018-08-29T00:28:42Z

alright let's merge this, I'll send out an image bump PR to go with it
/hold cancel

k8s-ci-robot · 2018-08-29T00:35:14Z

@neolit123: Updated the job-config configmap using the following files:

key generated-security-jobs.yaml using file config/jobs/kubernetes-security/generated-security-jobs.yaml
key kubeadm.yaml using file config/jobs/kubernetes/sig-cluster-lifecycle/kubeadm.yaml
key bazel-build-test.yaml using file config/jobs/kubernetes/sig-testing/bazel-build-test.yaml

In response to this:

kubernetes-anywhere (k-a) now has a specific branch for use with
e2e tests and it is no longer needed to bake a k-a commit SHA into
the kubeadm image.

ref:
kubernetes-retired/kubernetes-anywhere#555
kubernetes/kubernetes#66338

The following is used to pull the kubeadm-e2e branch of k-a:
--repo=k8s.io/kubernetes-anywhere=kubeadm-e2e

This removes a step where after fixing a k-a bug, the test-infra
kubeadm image has to be updated.

/area jobs
/assign @BenTheElder
/cc @timothysc
/cc @mohammedzee1000

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/test-infra repository.

neolit123 · 2018-08-29T22:53:46Z

@BenTheElder

i think this might be the first indication that something is not right about the new k-a setup:
https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-gce-upgrade-1-10-1-11/961

/workspace/kubernetes-anywhere/.config

perhaps it's not cloning in the correct folder.

https://k8s-testgrid.appspot.com/sig-cluster-lifecycle-all#gce-kubeadm-upgrade-1.10-1.11

waiting on some of the other tests to start too.

BenTheElder · 2018-08-29T22:58:56Z

I don't see a .config in that repo? https://github.com/kubernetes/kubernetes-anywhere/tree/kubeadm-e2e

neolit123 · 2018-08-29T23:26:55Z

i don't have enough details.
should it be added and if so what should it contain?

edit: investigating.

BenTheElder · 2018-08-29T23:37:37Z

not sure, but /workspace/kubernetes-anywhere/.config, /workspace/kubernetes-anywhere should actually have the k-a checkout IIRC, and I don't know why it would have a .config already 🤔

neolit123 · 2018-08-30T00:24:21Z

i think i found a problem in the logs:
https://storage.googleapis.com/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-gce-upgrade-1-10-1-11/961/build-log.txt

Checkout: /workspace/k8s.io/kubernetes-anywhere kubeadm-e2e to /workspace/k8s.io/kubernetes- anywhere
I0829 21:00:56.980] Call:  git init k8s.io/kubernetes-anywhere
I0829 21:00:56.987] Initialized empty Git repository in /workspace/k8s.io/kubernetes-anywhere/.git/

so it's checked out in /workspace/k8s.io

i think i need to update the value of --kubernetes-anywhere-path for all jobs:

test-infra/kubetest/anywhere.go

Line 160 in b5f298b

path: *kubernetesAnywherePath,

as --kubernetes-anywhere-path=/workspace/kubernetes-anywhere is not correct.
should be --kubernetes-anywhere-path=/workspace/k8s/kubernetes-anywhere

k8s-ci-robot assigned BenTheElder Aug 21, 2018

k8s-ci-robot added the area/jobs label Aug 21, 2018

k8s-ci-robot requested review from mohammedzee1000 and timothysc August 21, 2018 21:13

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/config Issues or PRs related to code in /config labels Aug 21, 2018

neolit123 commented Aug 21, 2018

View reviewed changes

k8s-ci-robot assigned timothysc Aug 21, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 21, 2018

neolit123 mentioned this pull request Aug 22, 2018

phase1-gce: make it possible to use GOOGLE_APPLICATION_CREDENTIALS kubernetes-retired/kubernetes-anywhere#557

Merged

neolit123 force-pushed the kubeadm-k-a-branch branch from d52fa12 to 9901d56 Compare August 22, 2018 14:27

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 22, 2018

BenTheElder reviewed Aug 27, 2018

View reviewed changes

neolit123 force-pushed the kubeadm-k-a-branch branch from 9901d56 to b7798f9 Compare August 27, 2018 18:15

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. area/prow Issues or PRs related to prow labels Aug 27, 2018

neolit123 force-pushed the kubeadm-k-a-branch branch from b7798f9 to 30cd445 Compare August 27, 2018 18:19

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 27, 2018

neolit123 force-pushed the kubeadm-k-a-branch branch from 30cd445 to 97ed199 Compare August 27, 2018 18:30

neolit123 force-pushed the kubeadm-k-a-branch branch from 97ed199 to 370260f Compare August 27, 2018 18:52

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Aug 28, 2018

BenTheElder approved these changes Aug 28, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 28, 2018

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 29, 2018

k8s-ci-robot merged commit fbf06e2 into kubernetes:master Aug 29, 2018

BenTheElder mentioned this pull request Aug 29, 2018

bump e2e images #9192

Merged

This was referenced Aug 29, 2018

Merge changes from test-infra upstream jetstack/test-infra#246

Merged

Move all prow job configs into config/ jetstack/test-infra#247

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kubeadm: pull kubernetes-anywhere per job #9117

kubeadm: pull kubernetes-anywhere per job #9117

neolit123 commented Aug 21, 2018

neolit123 Aug 21, 2018

BenTheElder Aug 21, 2018

BenTheElder Aug 21, 2018

neolit123 Aug 21, 2018

neolit123 Aug 21, 2018 •

edited

Loading

BenTheElder Aug 21, 2018

neolit123 Aug 21, 2018

neolit123 Aug 21, 2018

neolit123 Aug 21, 2018

BenTheElder Aug 21, 2018

timothysc commented Aug 21, 2018

neolit123 commented Aug 22, 2018 •

edited

Loading

neolit123 commented Aug 27, 2018

BenTheElder Aug 27, 2018

BenTheElder commented Aug 27, 2018

BenTheElder Aug 27, 2018

neolit123 commented Aug 27, 2018

k8s-ci-robot commented Aug 27, 2018 •

edited

Loading

BenTheElder left a comment

k8s-ci-robot commented Aug 28, 2018

BenTheElder commented Aug 28, 2018

neolit123 commented Aug 28, 2018

BenTheElder commented Aug 29, 2018

k8s-ci-robot commented Aug 29, 2018

neolit123 commented Aug 29, 2018 •

edited

Loading

BenTheElder commented Aug 29, 2018

neolit123 commented Aug 29, 2018 •

edited

Loading

BenTheElder commented Aug 29, 2018

neolit123 commented Aug 30, 2018

		@@ -21,14 +21,6 @@ if [ ! -e test-infra ]; then
		git clone https://github.com/kubernetes/test-infra

kubeadm: pull kubernetes-anywhere per job #9117

kubeadm: pull kubernetes-anywhere per job #9117

Conversation

neolit123 commented Aug 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neolit123 Aug 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timothysc commented Aug 21, 2018

neolit123 commented Aug 22, 2018 • edited Loading

neolit123 commented Aug 27, 2018

Choose a reason for hiding this comment

BenTheElder commented Aug 27, 2018

Choose a reason for hiding this comment

neolit123 commented Aug 27, 2018

k8s-ci-robot commented Aug 27, 2018 • edited Loading

BenTheElder left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 28, 2018

BenTheElder commented Aug 28, 2018

neolit123 commented Aug 28, 2018

BenTheElder commented Aug 29, 2018

k8s-ci-robot commented Aug 29, 2018

neolit123 commented Aug 29, 2018 • edited Loading

BenTheElder commented Aug 29, 2018

neolit123 commented Aug 29, 2018 • edited Loading

BenTheElder commented Aug 29, 2018

neolit123 commented Aug 30, 2018

neolit123 Aug 21, 2018 •

edited

Loading

neolit123 commented Aug 22, 2018 •

edited

Loading

k8s-ci-robot commented Aug 27, 2018 •

edited

Loading

neolit123 commented Aug 29, 2018 •

edited

Loading

neolit123 commented Aug 29, 2018 •

edited

Loading