-
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
kubernetes-csi: add Kubernetes 1.20 #20559
Conversation
The prow.sh script has ignored the patch version in the Kubernetes version number for a while now, therefore we can simplify the generator script and drop the patch version from all version numbers.
1.17.0 | ||
1.18.0 | ||
1.19.1 | ||
1.17 |
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 can remove 1.17 as it's not supported anymore.
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.
I was wondering about that too. I kept it because I first wanted to add 1.20 (i.e. always test on three releases), but I agree that combining the removal of the old release with the addition of the new (even if not enabled immediately) cuts down the overall effort - I'll change it.
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.
Has this been removed yet?
@@ -288,7 +280,7 @@ snapshotter_version() { | |||
# All other jobs test against the latest stable snapshotter | |||
# release. Additional jobs could be created to cover version | |||
# skew, if desired. | |||
echo '"v3.0.2"' | |||
echo '"v3.0.3"' |
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.
hm now we have 4.0 but it requires 1.20 as a min version cc @xing-yang
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.
I suppose we need a switch statement somewhere then?
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.
Yeah, maybe set it to 4.0 for 1.20 and higher and set it to 3.0.3 for 1.19 or lower.
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.
So we don't need (or want) to test 3.0.3 on 1.20? Some users might run it there.
That's fine with me, I just wanted to be sure.
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.
Added a Kubernetes check.
/lgtm 1.17 can be removed as a followup |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: msau42, pohly 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 |
@pohly: Updated the
In response to this:
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. |
The prow.sh script has ignored the patch version in the Kubernetes
version number for a while now, therefore we can simplify the
generator script and drop the patch version from all version numbers.
/cc @msau42