-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add Git <-> Kubernetes mapping for 1.10 #6919
Conversation
this will need a new Dockerfile in https://github.com/kubernetes/test-infra/tree/master/images/kubekins-test |
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.
ok, figured it out. this isn't well documented, but:
-
BRANCH_VERSION
converts a git branch name to the kubernetes version to use. 1.2 and 1.3 probably aren't needed anymore, but at the time we created this, we wanted to use the k8s 1.4 test image for those branches as well. -
VERSION_TAG
below defines which image to use for tests. we probably need a line there for 1.10 - though I'm not sure if we've built an appropriate test image yet for 1.10.
scenarios/kubernetes_verify.py
Outdated
@@ -29,6 +29,7 @@ | |||
BRANCH_VERSION = { | |||
'1.2': '1.4', | |||
'1.3': '1.4', | |||
'1.10': '1.9', | |||
'master': '1.9', |
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.
change this from 1.9 to 1.10
scenarios/kubernetes_verify.py
Outdated
@@ -29,6 +29,7 @@ | |||
BRANCH_VERSION = { | |||
'1.2': '1.4', | |||
'1.3': '1.4', | |||
'1.10': '1.9', |
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.
we don't need this
/assign |
@cjwagner @BenTheElder looks like we need a kubekins-test image for 1.10 still. |
On it. |
6f2c096
to
ac29049
Compare
let me know when you're done @BenTheElder and I'll update this PR. Updated the PR with comments, @ixdy |
PR with images pending here: #6920 |
...also annotates maps with minimal comments
ac29049
to
a62461d
Compare
Updated PTAL, @BenTheElder. Thanks! |
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.
/lgtm
/hold
I think this is fine, but I'm not totally certain about the master -> 1.10 mapping @krzyzacy can you confirm?
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, calebamiles 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:
Approvers can indicate their approval by writing |
seems this is a subset of #6921 |
We're still using Golang 1.9
/area so-crazy-it-just-might-work
/area prow