Skip to content
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 Version Server Check For Kudo Install #836

Merged
merged 2 commits into from
Sep 18, 2019

Conversation

kensipe
Copy link
Member

@kensipe kensipe commented Sep 17, 2019

What type of PR is this?
/kind feature

What this PR does / why we need it:
Adding check of operator requested kubernete version against the server.
kubernetesVersion is now a required field for operators

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

kubernetesVersion is a required operator field

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So with this PR we made kubernetes version a required field, but it was not a required field so far. There are operators that will be broken by this https://github.com/kudobuilder/operators/blob/master/repository/redis/operator/operator.yaml so we need to fix the operators or make it optional again.

My personal preference is to make it optional because if I am not using any special kubernetes features I am fine with running the operator on all k8s versions kudo can run on. Also ... helm has that optional as well.

pkg/version/version.go Outdated Show resolved Hide resolved
pkg/version/version.go Outdated Show resolved Hide resolved
pkg/version/version.go Outdated Show resolved Hide resolved
pkg/version/version.go Outdated Show resolved Hide resolved
Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address this one, the naming is still a bit confusing to me. Otherwise LGTM #836 (comment)

@kensipe kensipe merged commit 9b27e7a into master Sep 18, 2019
@porridge porridge added release/highlight This PR is a highlight for the next release and removed release-highlight labels Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/highlight This PR is a highlight for the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants