-
Notifications
You must be signed in to change notification settings - Fork 363
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
Switch CRD apiVersion to v1 from v1beta1 #1009
Conversation
Thanks for your PR. The following commands are available:
These commands can only be run by members of the vmware-tanzu organization. |
/test-all |
reopening the old PR.. by mistake renamed the branch and created a new one instead. closed the original PR #508 |
schema: | ||
openAPIV3Schema: | ||
type: object | ||
x-kubernetes-preserve-unknown-fields: true |
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.
this ensures original behavior. but imo we should update them to perform strict validation in a follow up for individual CRDs
/test-all |
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.
"apiextensions.k8s.io/v1beta1" is deprecated in 1.19 but will not be removed until 1.22 (kubernetes/kubernetes#82022)
Unless the default K8s version for managed services for which we claim support (GKE, EKS) is updated before we release Antrea v0.9.0, I do not think we should merge this. In GKE, it seems the default K8s version is currently 1.15.12. In EKS, it seems that the default version is 1.17 (but 1.15 is still supported). So for Antrea v0.9.0, I believe we should target K8s 1.15 -> 1.19, and postpone merging this PR for v0.10.0.
This PR should also reference this issue: #684
@abhiraut if you want to make that change for Antrea v0.10, it's ok with me. I see that the GKE stable channel is still using 1.15, but the regular channel is using 1.16. |
thanks.. will spin out a new PR sometime today |
Thanks for your PR. The following commands are available:
|
Codecov Report
@@ Coverage Diff @@
## master #1009 +/- ##
==========================================
+ Coverage 54.40% 54.49% +0.08%
==========================================
Files 115 115
Lines 10807 10807
==========================================
+ Hits 5880 5889 +9
+ Misses 4353 4347 -6
+ Partials 574 571 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
/test-all |
/test-networkpolicy |
/skip-hw-offload |
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.
This LGTM, but let's get more reviews and ensure everyone is ok with this change, since it means we will not support K8s 1.16 any more
Besides that, any impact of adding OpenAPI validation schemas to the CRDs?
I think it is that we no longer support K8s 1.15? |
Yes, that's a typo, we will no longer support 1.15, which is the K8s version used by GKE's "stable channel" (the "regular channel" uses 1.16). |
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
I have added "x-kubernetes-preserve-unknown-fields: true" to the fields for CRDs which did not have schema defined before so that unknown fields are preserved. The other CRDs listed in PR description had schema which i translated to the new format. |
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
/test-all |
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, one question inline
@@ -27,6 +31,10 @@ spec: | |||
- name: v1beta1 | |||
served: true | |||
storage: true | |||
schema: |
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.
Is the a new field that is not present in v1beta1? want to check if a cluster that already uses antrea can upgrade to 0.10 by simply fixing apiextensions.k8s.io/v1beta1
, or they need to fix all the CRD definition.
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.
they do need to update the definition.. as there is some renaming "JSONPath -> jsonPath" and some fields no longer supported (preserveUnknownFields) and some fields have a different parent field (validation -> schema)
apiVersion v1beta1 has been deprecated for CRDs in favor of v1.
Signed-off-by: abhiraut <rauta@vmware.com>
/test-all |
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.
Approving again
apiVersion v1beta1 has been deprecated for CRDs in favor of v1.
This PR takes care of updating versions of all the CRDs available.
In addition to the version update, this PR also ensures strict schema validation for the following CRDs:
Related-Issue: #684