grafana / grafana-operator

An operator for Grafana that installs and manages Grafana instances, Dashboards and Datasources through Kubernetes/OpenShift CRs
https://grafana.github.io/grafana-operator/
Apache License 2.0
863 stars 384 forks source link

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

Closed smuda closed 2 months ago

smuda commented 3 months ago

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? :-)

weisdd commented 3 months ago

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:

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 commented 3 months ago

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 commented 3 months ago

@smuda Do you mean GitHub release?

smuda commented 3 months ago

Yes. For example https://github.com/grafana/grafana-operator/releases/download/v5.9.2/crds.yaml

weisdd commented 3 months ago

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. :)

theSuess commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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