-
Notifications
You must be signed in to change notification settings - Fork 211
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
Issue 329: CRD apiVersion check #330
Issue 329: CRD apiVersion check #330
Conversation
Codecov Report
@@ Coverage Diff @@
## master #330 +/- ##
=======================================
Coverage 84.67% 84.67%
=======================================
Files 11 11
Lines 1312 1312
=======================================
Hits 1111 1111
Misses 133 133
Partials 68 68 Continue to review full report at Codecov.
|
@lspecian-olx Thanks for the contribution. Do we need similar check here as well https://github.com/lspecian-olx/zookeeper-operator/blob/issue-329-crd-apiversion-check/charts/zookeeper-operator/templates/pre-delete-hooks.yaml#L3 ? |
Indeed we need a similar check as well, I made a new commit including the change |
@lspecian-olx could you please address the review comment? |
5909a60
to
c4eab90
Compare
…rization.k8s.io/v1 on the pre-delete-hooks
c4eab90
to
b2c9c4d
Compare
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
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
@lspecian-olx when i tried to install Operator on a cluster with
|
Yes, I'm also seeing the same, preparing a PR with a fix for the issue #338 |
Change log description
Purpose of the change
Closes #329
What the code does
The code is using .Capabilities.APIVersions.Has to check for the apiVersion apiextensions.k8s.io/v1, if it finds it it uses to create the CRD, if it doesn't it falls back to apiextensions.k8s.io/v1beta1
How to verify it
You are able to install the chart with the CRD in eks v1.19.0 version from which apiextensions.k8s.io/v1beta1 was removed