Investigate using Bazel to deploy CRDs when using Helm

[toast-gear]

Problem
Currently due to how CRDs are handled by Helm users can get themselves into situations like #512 if they are not careful about keeping the CRD versions on their cluster aligned with the controller app version.

Solution
CRDs are included in the deploy order of the templates/ folder in Helm https://github.com/helm/helm/blob/release-3.6/pkg/releaseutil/kind_sorter.go#L31.

If we package the CRDs in the templates/ folder Helm will be happy to perform all CRUD operations on them removing the need for special upgrade instructions and people getting themselves into the infinite runner loop issue.

[mumoshu]

FYI, this is what we're doing today: https://github.com/actions-runner-controller/actions-runner-controller/blob/cb60c1ec3baed8690d4546dacfdfdc564891e852/Makefile#L88-L89

Can we just change the cp destination to charts/actions-runner-controller/templates to achieve the goal? 😃

[toast-gear]

🤷 😄 it's worth testing, I'll try to make time to test in minikube. Helm is annoying about it due to data loss concerns, I don't think we are worried about that due to the nature of the project? We'd need the steps to create the infinite loop problem so we can replicate the issue, then try again with the crds packaged in the templates/ folder?

I guess the problem with that is we can't deploy them conditionally? Some people prefer to wrap their CRDs into their own charts and so we should probably support that as it's a common setup. Would a simple boolean in the values.yaml do the job? I guess it would

Tbh I'm not super clear on what the data loss concern is really, would love someone to clear that up so we can figure out if it's a concern for this project

[mumoshu]

I guess the problem with that is we can't deploy them conditionally? Some people prefer to wrap their CRDs into their own charts and so we should probably support that as it's a common setup. Would a simple boolean in the values.yaml do the job? I guess it would

Ah, that's good point! I somehow forgot about that... So we definitely need some way to "wrap" what we've copied with the cp command into a helm chart template that contains a biiig if-end block to allow toggling CRD installation via a chart value.

[toast-gear]

yeh, I wonder if that alone would be enough! I think it would!

[callum-tait-pbx]

https://github.com/helm/community/blob/main/hips/hip-0011.md this is the authority on CRDs and Helm, I need to read it and process

EDIT Findings from tests:
The below comments were based on deploying master at 46be209

1. Whilst the controller chart is installed, deleting a CRD results in all custom resources of that definition being deleted
2. Helm won't reinstall missing CRDs with an upgrade, it seems that it will only install them (if they don't exist) on an helm install command exclusively
3. If you uninstall the controller chart and then delete CRDs with CRs created from the definitions, the CRD itself gets recreated rather than the CR being removed
4. If we move the combine the CRDs into a single crds.yaml file and include it within the template/ folder rather than the special crd/ folder Helm will uninstall and remove the CRDs upon a helm uninstall however the CRs themselves will remain and the CRDs seem to get recreated. At this point if you try to remove the CRD it gets recreated by the CR, if you delete the CR then it gets recreated and if you delete the pod you end up with an orphaned runner and have to do the finaliser merge operation to remove it too. All this means that you need to delete your CRs anyway before doing upgrade regardless of how the CRDs are managed by Helm.
5. Deleting CRDs via kubectl get crds | grep actions.summerwind. | awk '{print $1}' | xargs kubectl delete crd will hang whilst runners are busy so this isn't a great way of clearing down your CRs before doing an upgrade. This applies even if you uninstall the controller chart beforehand, the runners are not released from their jobs and so the crd deletion operation hangs until they are free.

[toast-gear]

There isn't really a good way of handling this, we are updating the recommended upgrade process as that is the best we can do really. If we did get Helm to do the install and uninstall the CRDs they would:

A) Delete everyone's runners during an upgrade which the end user may not expect unless they have a reasonable level of understanding of CRDs
B) The upgrade process could take a long time as the delete request will wait until all runners are not busy before it will complete