Skip to content
This repository has been archived by the owner on Apr 22, 2020. It is now read-only.

improve the k-a versioning in the test-infra kubeadm runner #555

Closed
neolit123 opened this issue Aug 10, 2018 · 14 comments
Closed

improve the k-a versioning in the test-infra kubeadm runner #555

neolit123 opened this issue Aug 10, 2018 · 14 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.

Comments

@neolit123
Copy link
Contributor

neolit123 commented Aug 10, 2018

we eventually want to deprecate kubernetes-anywhere in the e2e suite.
but this isn't that easy, so we have to stick to it for a while.

right now we have a fixed k-a (kuberenetes-anywhere) commit in the image for the kubeadm runner.
this imposes the difficulty that we need to PR two changes:

  • one that fixes k-a
  • one that updates the SHA in runner.

https://github.com/neolit123/test-infra/blob/8f0bd23ac99e7c8809ccc6aefb42273dc96ca066/images/kubeadm/runner#L29

my proposal:

  • create a k-a branch called kubeadm-e2e in this repo
  • push e2e related changes and fixes to this branch.
  • always use the tip of the branch kubeadm-e2e in the runner!!
  • cherry pick changes from the kubeadm-e2e branch to the k-a master branch if needed.

future changes in kubeadm are going to make k-a very hard to maintain for both users and e2e tests.

/assign @timothysc
/assign @BenTheElder
/priority critical-urgent
/kind feature

/cc @errordeveloper @luxas @mikedanese
for super powers on creating a k-a branch called kubeadm-e2e if this is approved by @BenTheElder and @timothysc

@k8s-ci-robot k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. kind/feature Categorizes issue or PR as related to a new feature. labels Aug 10, 2018
@BenTheElder
Copy link

All we need is some way to note the stable version to use for CI. I'm open to anything...

I think if we do keep the version specified at the test-config we should remove this checkout from the runner and instead specify it with --repo on the kubeadm jobs. This will update when the PR merges instead of after upgrading the images, and it potentially lets us freeze the version for older kubernetes releases.

@neolit123
Copy link
Contributor Author

i see a --repo argument already present here:
https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes/sig-cluster-lifecycle/kubeadm.yaml

but it points to the main repo.

there is this check:
https://github.com/neolit123/test-infra/blob/8f0bd23ac99e7c8809ccc6aefb42273dc96ca066/images/kubeadm/runner#L24

if we remove it and always checkout a fresh copy of a stable k-a branch wouldn't this image work when the periodics trigger without updating the images everytime?

i must admit that i don't understand how it all works.

the idea of the kubeadm-runner branch is to also move the e2e work outside of the k-a master because:

future changes in kubeadm are going to make k-a very hard to maintain for both users and e2e tests.

@BenTheElder
Copy link

i see a --repo argument already present here: ...

Yes, you can supply multiple repos, the first one will be $PWD for testing, but the others will be checked out along side it as well, and you can specify a hash for those. We can remove the checkout from the runner and instead specify which release to checkout on each job as an extra repo, which also makes it configurable per-job / per-kubernetes-release

@neolit123
Copy link
Contributor Author

neolit123 commented Aug 10, 2018

if we hardcode the hash or release (is it a k-a release?) in the job config, wouldn't that mean that if we bump k-a by one patch/fix/commit we still need to update the job config too and possibly make a new k-a release?

is it possible to specify a branch (tip) instead?
i know that this removes the "stable" / release factor from k-a.

@BenTheElder
Copy link

if we hardcode the hash or release (is it a k-a release?) in the job config, wouldn't that mean that if we bump k-a by one patch/fix/commit we still need to update the job config too

We can do that though, and selectively for the tests that actually need it (probably just recent k/k branches)

and possibly make a new k-a release?

does k-a have releases?

is it possible to specify a branch (tip) instead?

yep you can just specify a branch without a sha. independently of what way we specify the stable version (sha, branch, ...) we should probably move specifying it to be on the jobs so updates to this are quick (and likely only involve cluster-lifecycle to review & merge), versus needing a new image to be published first.

@neolit123
Copy link
Contributor Author

neolit123 commented Aug 10, 2018

We can do that though, and selectively for the tests that actually need it (probably just recent k/k branches)

ok, given right now we always use the tip of k-a, all tests are conformant with the tip of k-a.

ideally the tests by default should use the latest (presumably patched branch tip), but if a test complains we can specify an exact commit.

does k-a have releases?

it has one...
the project is quite inactive in terms of maintenance and it appears we only use it for e2e.
it does have some user base, but i'm considering sending a README.md update to note about the above.

and eventually in time deprecating it for e2e.

yep you can just specify a branch without a sha. independently of what way we specify the stable version (sha, branch, ...) we should probably move specifying it to be on the jobs so updates to this are quick (and likely only involve cluster-lifecycle to review & merge), versus needing a new image to be published first.

would that be a valid diff and would it replace the checkout in the runner?

- name: ci-kubernetes-e2e-kubeadm-gce-selfhosting
  interval: 6h
  labels:
    preset-service-account: "true"
    preset-k8s-ssh: "true"
  spec:
    containers:
    - image: gcr.io/k8s-testimages/e2e-kubeadm:v20180730-c8036c3a7
      args:
      - --repo=k8s.io/kubernetes=master
      - --timeout=320
      - --upload=gs://kubernetes-jenkins/logs
      - --scenario=kubernetes_e2e
      - --
      - --cluster=
      - --deployment=kubernetes-anywhere
      - --extract=ci/latest-bazel
      - --gcp-zone=us-central1-f
      - --kubeadm=ci
+     - --repo=k8s.io/kubernetes-anywhere=<some-branch>
      - --kubernetes-anywhere-kubeadm-feature-gates=SelfHosting=true
      - --kubernetes-anywhere-kubelet-ci-version=latest-bazel
      - --kubernetes-anywhere-kubernetes-version=ci-cross/latest
      - --provider=kubernetes-anywhere
      - --test_args=--ginkgo.focus=\[Conformance\]|\[Feature:BootstrapTokens\]|\[Feature:NodeAuthorizer\] --minStartupPods=8
      - --timeout=300m

@BenTheElder
Copy link

Hmm I'll defer on how we should manage this repo, but:

would that be a valid diff and would it replace the checkout in the runner?

move the repo flag up by the other one (but AFTER it!), somewhere before the bare --. flags before -- go to the bootstrapping script, flags after go to the job-specific script (e2e script in this case). We also may need to check how other scripts look for k-a (make sure the paths are right, with this flag it will be at $PWD/../kubernetes-anywhere.

@timothysc
Copy link

@detiber @chuckha - 1st use case for aws implementation of clusterapi. Need to chat with @countspongebob on this.

@neolit123
Copy link
Contributor Author

neolit123 commented Aug 10, 2018

@BenTheElder ack! thanks for the explanation.
@timothysc do we have a blocker related to this issue?

@neolit123
Copy link
Contributor Author

neolit123 commented Aug 16, 2018

can we please get some super-powers for the decision on this?
this is currently blocking all kubeadm e2e tests that use kubernetes-anywhere.
not that we cannot fix them, it's just that the process needs this improvement.

summary:
we create a new branch specific for kubeadm e2e tests in the kubernetes-anywhere repo and always use the tip of that branch in the test-infra jobs instead of specific commits. proposed branch name kubeadm-e2e

@errordeveloper agreed to this, but he wanted confirmation from others too.
he told me to ping @jessicaochen as well.

also /cc @roberthbailey as he might have write access and an opinion on this.

@timothysc
Copy link

I'm a +1 to just getting this done, but I have now powers here.

@neolit123
Copy link
Contributor Author

@errordeveloper
hi, given @timothysc has given a +1, would that serve as enough confirmation for you and can you create the branch?

@errordeveloper
Copy link
Contributor

@neolit123 I've create the kubeadm-e2e branch just now, please go ahead and start using it!

@neolit123
Copy link
Contributor Author

@errordeveloper
thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.
Projects
None yet
Development

No branches or pull requests

5 participants