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 additional manifest checks to detect missing CRDs & CRs #5921

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

joelsmith
Copy link
Contributor

@joelsmith joelsmith commented Jun 27, 2024

This PR has three parts:

  1. Add additional PR checks to make sure that every CRD is listed in the CRD kustomize.yaml file and that every CRD also has a sample CR for each of its API versions and is similarly listed in the kustomize.yaml file for sample CR files.
  2. Add missing sample CRs, CRDs and references.
  3. Update existing sample CR for triggerauthentication which had the old API group of keda.k8s.io instead of the new keda.sh

Checklist

Fixes #5920

@joelsmith joelsmith requested a review from a team as a code owner June 27, 2024 22:50
@joelsmith joelsmith force-pushed the kedamain branch 15 times, most recently from 140f909 to 7ae86fc Compare June 28, 2024 06:20
Signed-off-by: Joel Smith <joelsmith@redhat.com>
@joelsmith
Copy link
Contributor Author

joelsmith commented Jun 28, 2024

The check seems to be working. For example: https://github.com/kedacore/keda/actions/runs/9708520106/job/26795477554?pr=5921

Run make verify-manifests
./hack/verify-manifests.sh
ERROR: CR config/samples/eventing_v1alpha1_cloudeventsource.yaml is not listed in the resources section of config/samples/kustomization.yaml
ERROR: CRD config/crd/bases/keda.sh_clustertriggerauthentications.yaml does not have a sample CR config/samples/keda_v1alpha1_clustertriggerauthentication.yaml
ERROR: CR config/samples/keda_v1alpha1_clustertriggerauthentication.yaml is not listed in the resources section of config/samples/kustomization.yaml
make[1]: Entering directory '/__w/keda/keda'
...
./hack/../config up to date.
Check failed due to previous errors. See output above
make: *** [Makefile:1[5](https://github.com/kedacore/keda/actions/runs/9708520106/job/26795477554?pr=5921#step:12:6)3: verify-manifests] Error 1
Error: Process completed with exit code 2.

So now I need to add another commit to add the missing manifests and references.

Edit: commit 2 added and now the new test passes without errors.

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@zroubalik
Copy link
Member

zroubalik commented Jun 28, 2024

/run-e2e internal
Update: You can check the progress here

Add missing CR references to config/sample/kustomization.yaml

Signed-off-by: Joel Smith <joelsmith@redhat.com>
Signed-off-by: Joel Smith <joelsmith@redhat.com>
@joelsmith
Copy link
Contributor Author

Sorry for the last-minute change. I just noticed that the sample CR I copied (triggerauthentication) had an API group of keda.k8s.io instead of keda.sh so I fixed the new clustertriggerauthentication one and also fixed the existing triggerauthentication.

Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

Thank you!

@tomkerkhove tomkerkhove merged commit 2bb1cfb into kedacore:main Jul 1, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CRD & sample CR manifest data for downstream OLM operator incomplete
4 participants