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

Remove crd-install hook, as it break CRD updates #441

Merged

Conversation

markmandel
Copy link
Member

@markmandel markmandel commented Dec 8, 2018

This is a problem that Helm is going to solve going forward, but for now, if you use the crd-install hook, then you can only install CRDs, and not update them at any point during a chart lifecycle.

Also, prior to Helm 2.12, if you installed chart with a crd-install hook that did not have one previously, it deleted the CRDs.

Therefore, removing the crd-install hook, so that CRDs are again managed by the Helm charts.

Added a agones.crd.install parameter, in case someone wants to subchart this chart, then can set this to false, and copy the Agones CRDs into their own charts to be included in the right place for their chart lifecycle.

Also, since we have a agones.crd config section, moved agones.enableHelmCleanupHooks into agones.crds.cleanupOnDelete

Unfortunately, with this back and forth on the crd-install hook, if you are using the Helm chart, you will need to do a full Agones helm delete --purge and cleanup any remaining CRDs to upgrade.

More context on helm + crds:

@markmandel markmandel added kind/bug These are bugs. area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Dec 8, 2018
@markmandel markmandel added this to the 0.7.0 milestone Dec 8, 2018
@markmandel
Copy link
Member Author

/cc @EricFortin please let me know if you have any issues with me moving your config to agones.crds.cleanupOnDelete - felt like a good spot for it, but not got a strong opinion.

/cc @smoya FYI, since you first proposed this. Unfortunately crd-install hooks are really broken 😭 , but I wanted to give you a heads up on the workaround I put in the docs - feedback appreciated.

@markmandel markmandel changed the title This is a problem that Helm is going to solve going forward, but for now Remove crd-install hook, as it break CRD updates Dec 8, 2018
This is a problem that Helm is going to solve going forward, but for now
if you use the crd-install hook, then you can only install CRDs, and not
update them at any point during a chart lifecycle.

Also, prior to Helm 2.12, if you installed chart with a crd-install hook
that did not have one previously, it deleted the CRDs.

Therefore, removing the crd-install hook, so that CRDs are again managed
by the Helm charts.

Added a `agones.crd.install` parameter, in case someone wants to subchart
this chart, then can set this to false, and copy the Agones CRDs into
their own charts to be included in the right place for their chart lifecycle.

Also, since we have a `agones.crd` config section, moved
`agones.enableHelmCleanupHooks` into `agones.crds.cleanupOnDelete`

Unfortunately, with this back and forth on the crd-install hook, if you are
using the Helm chart, you will need to do a full Agones
`helm delete --purge` and cleanup any remaining CRDs to upgrade.

More context on helm + crds:
- helm/helm#4697
- istio/istio#9604
- istio/istio#7688
- helm/community#64
- helm/helm#4863
- helm/helm#4709
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 36d3e4c7-7322-410f-9f5e-7475e983246c

Build Logs
starting build "36d3e4c7-7322-410f-9f5e-7475e983246c"

FETCHSOURCE
Initialized empty Git repository in /workspace/.git/
fatal: remote error: want eb2d07fa231427c5ca4d80ad038ce714de4daf4a not valid
fatal: the remote end hung up unexpectedly
Reinitialized existing Git repository in /workspace/.git/
fatal: remote origin already exists.
fatal: remote error: want eb2d07fa231427c5ca4d80ad038ce714de4daf4a not valid
fatal: the remote end hung up unexpectedly
Reinitialized existing Git repository in /workspace/.git/
fatal: remote origin already exists.
fatal: remote error: want eb2d07fa231427c5ca4d80ad038ce714de4daf4a not valid
fatal: the remote end hung up unexpectedly
ERROR
ERROR: fetching git source: Failure fetching git repo: exit status 128

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 3058f107-108f-43eb-b0cc-0bf4ec3c5f6e

The following development artifacts have been built, and will exist for the next 30 days:

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/441/head:pr_441 && git checkout pr_441
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.7.0-422768e

@markmandel markmandel merged commit 938e23e into googleforgames:master Dec 9, 2018
@markmandel markmandel deleted the bug/remove-crd-install branch December 9, 2018 15:49
@smoya
Copy link
Contributor

smoya commented Dec 9, 2018

Thanks for taking care @markmandel ! And thanks for mentioning me as well. Good solution btw.

@markmandel markmandel added the kind/breaking Breaking change label Dec 10, 2018
@sdake
Copy link

sdake commented Jan 6, 2019

@markmandel note Istio came up with a slightly different solution:

See: istio/istio#10562

And specifically tiller-hack here:
istio/tools#31

This tool works around the problem of CRDs being deleted. Note, this tool is specific to Istio, but the solution could be generalized.

Just thought I'd share.

Cheers
-steve

@markmandel
Copy link
Member Author

@sdake I'm struggling to work out what they are doing here. Can you give a quick summary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/breaking Breaking change kind/bug These are bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants