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

Helm CRD deployment/deletion #107

Merged
merged 1 commit into from
May 2, 2019
Merged

Helm CRD deployment/deletion #107

merged 1 commit into from
May 2, 2019

Conversation

lee0c
Copy link
Contributor

@lee0c lee0c commented Apr 22, 2019

Fixes #105

  • Added value to toggle whether or not the CRD is created so that helm can be deployed w/out error while CRD still exists.
  • Added hook to delete CRD before reinstall - still not sure if this is the preferable path Killed this. I think we should go with the route least likely to suddenly wipe someone's setup
  • Generic helm chart cleanup

Not sure which of the paths mentioned in linked issue is preferred - delete CRD w/ deployment, or delete CRD before attempting to install?

@lee0c lee0c changed the title [WIP] Helm CRD deployment/deletion Helm CRD deployment/deletion Apr 22, 2019
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: scaledobjects.kore.k8s.io
annotations:
"helm.sh/hook": crd-install
# This will delete the CRD before any attempt to install it
# However, it only works if you use the --wait option on `helm install`
"helm.sh/hook-delete-policy": "before-hook-creation"
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't clear about this in the meeting today. Would this cause all ScaledObjects to get deleted on update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On update, no. They would only get wiped if you were doing a fresh helm install after deleting the deployment. If we don't want people to unintentionally wipe them, I can revert that and just leave in the var that lets you not create the CRD.

@ahmelsayed
Copy link
Contributor

You'll need to rebase your pr. The file names changed so keep that in mind.

@lee0c
Copy link
Contributor Author

lee0c commented Apr 25, 2019

Will rebase at some point this afternoon, I have been at an event and am catching up on all the changes.

@lee0c lee0c force-pushed the helm-crd-fix branch 2 times, most recently from 8a22d25 to 68260af Compare April 26, 2019 04:08
@lee0c
Copy link
Contributor Author

lee0c commented May 2, 2019

@Aarthisk I'd love to see this merged or closed, just rebased again so it is up to date

@ahmelsayed ahmelsayed merged commit 13071d2 into master May 2, 2019
@ahmelsayed ahmelsayed deleted the helm-crd-fix branch May 2, 2019 22:45
patnaikshekhar pushed a commit to patnaikshekhar/keda that referenced this pull request Jul 17, 2019
Helm CRD deployment/deletion

Former-commit-id: 13071d2
Aarthisk pushed a commit that referenced this pull request Aug 12, 2019
Helm CRD deployment/deletion

Former-commit-id: 3405d42
preflightsiren pushed a commit to preflightsiren/keda that referenced this pull request Nov 7, 2021
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.

Have helm chart delete CRD or add option to not create CRD
2 participants