-
Notifications
You must be signed in to change notification settings - Fork 104
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
Document Kubernetes API policy #1642
Merged
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 think all this should be still tight to a KUDO release though. If I update KUDO I don't want to check if at the time when it was released maybe also new k8s release was released thus my 1.15 cluster might not work with KUDO anymore. I think every KUDO release version should explicitly note the minimal kubernetes version.
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.
How about mentioning which version of Kubernetes KUDO was built against in the
version
command? And let's mention the Kubernetes version in our release notes. E.g., "This version of KUDO is built against the Kubernetes 1.18 API and compatible with Kubernetes clusters 1.16-1.18".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 I like that. My comment was just that this mention kind of implies that one has to watch the k8s releases by themselves and align it with KUDO releases, I think that should not be the case.
Maybe we can reword it just a bit and say that this is our overall policy but to know exactly which versions are supported you have to do XXX. For now we can at least mention it on the release page for the latest release, having it in CLI would be also super nice.
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've looked into this and opened a PR #1671 that shows the used k8s API version. I'm not sure what kind of wording we should use here though. We could mention that
kudo version
will show you the used API version...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.
Oh, and this is developer-facing, not user facing.... So, I'm not sure if we need to change anything here?
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 have concerns around wording... which ties to the PR mentioned by @ANeumann82 . There are several topics that come to mind regarding kubernetes versions which includes:
@ANeumann82 mentions "k8s API version". I'm not sure I know exactly what that means... each kind in the API has it's version... I'm guessing for this conversation we are tying together all the APIs for a version of kubernetes... I also imagine in so do we are ignoring feature flags and extensions. The challenge to this is that the server version is the version that determines the APIs... not the client version (which seems to the the version in the PR)
I love @alenkacz recommendation on each release providing a matrix of support.
What most significantly matters for KUDO users IMO, is KUDO is based on a version of client-go. The client-go version that KUDO uses defines the constraint. The version matrix is already defined by client-go here: https://github.com/kubernetes/client-go#compatibility-matrix
based on all this... I would say that KUDO is constrained to k8s versions based on 2 factors:
The linked client-go version should be advertised for each release and the understanding of compatibility should be pushed to the client-go website.
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.
All very good points, I'll reword this section to distinguish between client and server API. IMO, both are important here, like you mentioned @kensipe, but there's more than just a constraint to support CRDs on the server. We need to be clear on what features we are able to support without feature flags and when to change APIs that have been deprecated. Here, CRDs are a good example: Since Kubernetes 1.16, there's a v1 CRD API. The v1beta1 API that KUDO currently uses is deprecated since Kubernetes 1.19. We developers need to decide when to change our CRDs from v1beta1 to v1. I.e. our features work is constrained by the version of the Kubernetes server API we want to target.