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

Add appVersion and version to CRD to match the structs. #1195

Merged
merged 1 commit into from
Dec 18, 2019

Conversation

porridge
Copy link
Member

What this PR does / why we need it:

The goal is to make CRDs and the api structs match.

I'm not sure whether this is the way to go - are these the fields that the
current ongoing "versioning" discussion is all about?

In that case perhaps we should instead remove these fields (and code which uses
them, if any) from the structs?

This is a baby-step towards #862.

Copy link
Member

@nfnt nfnt left a comment

Choose a reason for hiding this comment

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

This is great, but I'd suggest to wait with this until the versioning KEP is being implemented. As an operator package is different from what is later deployed on a cluster, its metadata is defined somewhere else. This metadata already contains (optional) fields for version and appVersion.

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.

LGTM as this is only about adding already used fields to the openapi validation 👍

Copy link
Member

@nfnt nfnt left a comment

Choose a reason for hiding this comment

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

Thanks!

@nfnt nfnt merged commit 23b777c into master Dec 18, 2019
@nfnt nfnt deleted the crd-correction-appversion branch December 18, 2019 14:38
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants