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

[Bug] CRD description fields in all CRDs lost between 5.8.1 and 5.9.0 #1567

Closed
smuda opened this issue Jun 4, 2024 · 8 comments · Fixed by #1579
Closed

[Bug] CRD description fields in all CRDs lost between 5.8.1 and 5.9.0 #1567

smuda opened this issue Jun 4, 2024 · 8 comments · Fixed by #1579
Labels
bug Something isn't working triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@smuda
Copy link
Contributor

smuda commented Jun 4, 2024

Describe the bug
In the CRD there is a description field for each custom field. For example, in grafanadashboard, the field allowCrossNamespaceImport had the description

"allow to import this resources from an operator in a different namespace"

This is really nice when not having a familarity with the api to be able to use kubectl explain

% kubectl explain grafanadashboard.spec.allowCrossNamespaceImport
GROUP:      grafana.integreatly.org
KIND:       GrafanaDashboard
VERSION:    v1beta1

FIELD: allowCrossNamespaceImport <boolean>

DESCRIPTION:
    allow to import this resources from an operator in a different namespace

Version
This is a change between 5.8.1 and 5.9.0. Both 5.8.0 and 5.8.1 has description in CRD, but not 5.9.0, 5.9.1 and 5.9.2.

Expected behavior
Included descriptions in CRDs.

Suspect component/Location where the bug might be occurring
CRDs? :-)

@smuda smuda added bug Something isn't working needs triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 4, 2024
@smuda smuda changed the title [Bug] CRD description fields in all CRDs lost between 5.8.0 and 5.9.1 [Bug] CRD description fields in all CRDs lost between 5.8.1 and 5.9.0 Jun 4, 2024
@weisdd
Copy link
Collaborator

weisdd commented Jun 5, 2024

We've previously had problems where size of CRDs was causing problems when trying to apply it (e.g. for kubectl, only server-side applies would work). So, at some point (probably, even before we released v5), we decided to generate two sets of CRDs:

  • with description, used for generating documentation;
  • without description, used for deployments.

If some of the deployments methods contained field descriptions, that, I guess, was not intentional.

I guess we can reassess if having embedded descriptions still would cause any issues.

@smuda
Copy link
Contributor Author

smuda commented Jun 5, 2024

When 5.8.0 arrived, we had to change to serverSideAppy because of the size (and because kubectl stores the CRD twice in the CRD) and that does pose a usabillity problem.

To minimize support I'm trying to encourage our users to always use kubectl explain and without the explainations, well it looses some value.

We're not using the CRDs included in the helm chart but rather download them separately from the release. Perhaps one way forward would be to include CRDs with and without explainations in the release?

@weisdd
Copy link
Collaborator

weisdd commented Jun 5, 2024

@smuda Do you mean GitHub release?

@smuda
Copy link
Contributor Author

smuda commented Jun 5, 2024

@weisdd
Copy link
Collaborator

weisdd commented Jun 5, 2024

I don't have a strong opinion on whether we should publish both versions or simply include descriptions in crds.yaml again.
We'll discuss it during the next meeting. :)

@weisdd weisdd added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 5, 2024
@theSuess
Copy link
Member

We'll investigate if enabling descriptions everywhere will cause any issues and enable them when everything is fine.

Until then, you can use the CRDs from the deploy folder directly. If you need a specific version, check out the respective tag: https://github.com/grafana/grafana-operator/tree/master/config/crd-for-docs-generation

@weisdd
Copy link
Collaborator

weisdd commented Jun 11, 2024

@theSuess I've tested deployment of full CRDs with kubectl (using server-side apply) and helm, it worked well. That's a good sign. Could you test it on OpenShift?

@theSuess
Copy link
Member

OpenShift should not be an issue. All other operators ship descriptions and have way larger CRDs than we do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants