Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

cmd/operator-sdk: add --crd-version flag to relevant commands #2684

Merged
merged 2 commits into from
Mar 24, 2020

Conversation

joelanford
Copy link
Member

@joelanford joelanford commented Mar 19, 2020

Description of the change:
Add CRD version flag to relevant subcommands that enables users to generate CRDs with either v1beta1 or v1.

v1beta1 is still the default until OLM supports operator bundles with v1 CRDs.

Motivation for the change:
CRD v1beta1 is deprecated and will be removed in Kubernetes 1.19, so we need to give users time to begin migrating.

Closes #2579

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Mar 19, 2020

I think it can close: #2579

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2020
err error
)
switch g.crdVersion {
case "v1beta1":
Copy link
Member

Choose a reason for hiding this comment

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

Since we're going to switch to v1.CustomResourceDefinition by default in the future, we could set crd := apiextv1.CustomResourceDefinition{} above and return a apiextv1.CustomResourceDefinition from newCRDForResource(), then convert v1 -> v1beta. We'd also have to do some conversion if the file exists on disk and is v1beta1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the reason I didn't do that was because of the possibility of a v1beta1 CRD on disk. v1 moved several top level fields from v1beta1 into the versions items, so it is not possible to unmarshal the v1beta1 YAML into a v1 struct.

However, the v1beta1 struct is a superset of the v1 struct, so it is possible to unmarshal v1 YAML into a v1beta1 struct and then convert back.

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use the generic apiextensions struct for both?

Copy link
Contributor

Choose a reason for hiding this comment

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

If I properly understood the suggestion here, IHMO is better we keep each version implementation since in the future we will just remove the v1beta1 and keep all generating v1 only.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it possible to use the generic apiextensions struct for both?

I can't get that to work either. For some reason (still trying to figure out why) everything gets marshaled without having the correct YAML layout (e.g. in the output there's actually an objectMeta key containing the GVK info, instead of that being inlined at the root).

Copy link
Member

Choose a reason for hiding this comment

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

I’ve had trouble marshaling the generic CRD struct before. The current approach is fine then.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

Once we bump to v1.19, we'll have to basically revert this PR and change from v1beta1 to v1. This function will basically go back to how it looked before, except with the v1 types.

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

Changelog then LGTM

joelanford added a commit to joelanford/operator-sdk that referenced this pull request Mar 24, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update deprecated apiextensions.k8s.io/v1beta to apiextensions.k8s.io/v1
5 participants