feat(tiller): support CRD installation by technosophos · Pull Request #3982 · helm/helm

technosophos · 2018-04-27T05:24:51Z

This adds support for installing CRDs well before any other resource
kinds are installed.

This PR introduces a new hook, crd-install, that fires before
manifests are even validated. It is used to install a CRD before any
other part of a chart is installed.

Currently, this hook is only implemented for install. That means we
currently cannot add new CRDs during helm upgrade, nor can they
be rolled back. This is the safest configuration, as the update/rollback
cycle gets very challenging when CRDs are added and removed.

rimusz · 2018-04-27T08:46:33Z

yay, long awaited feature 😀

technosophos · 2018-04-28T00:28:00Z

Okay, here's how this works

First, if you are just creating a CRD with no instances of that CRD, you do nothing (just as always). This is designed to allow for a CRD and instances of the CRD to be installed in the same chart. It is also designed to be maximally backward compatible, though I did have to eliminate one set of validations in order to do this.

To add a CRD to a chart, create a CRD template and set an annotation on the template:

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: crontabs.stable.example.com
  # THE IMPORTANT PART!!!
  annotations:
    "helm.sh/hook": crd-install
spec:
  group: stable.example.com
  version: v1
  scope: Namespaced
  names:
    plural: crontabs
    singular: crontab
    kind: CronTab
    shortNames:
    - ct

You can also create a template that references that CRD:

apiVersion: stable.example.com/v1
kind: CronTab
metadata:
  name: {{ .Release.Name }}-inst

Now, when you run helm install ./mychart, the following will happen:

Manifests will be sorted and rendered as usual
crd-install hooks will be separated
If dry-run and there's a CRD hook, validation will be skipped and the user will be warned
Otherwise, execute the crd-install hook (install the CRD)
Validate the remaining normal manifests
Run pre-install
Continue on the normal installation path

If helm install --no-crd-hook is specified, then the crd-install hook is skipped, and no CRDs are installed. If --no-hooks is specified, crd-install is skipped. If --no-crd-hook --dry-run are specified together, the validator will run.

This adds support for installing CRDs well before any other resource kinds are installed. This PR introduces a new hook, `crd-install`, that fires before manifests are even validated. It is used to install a CRD before any other part of a chart is installed. Currently, this hook is _only implemented for install_. That means we currently cannot add new CRDs during `helm upgrade`, nor can they be rolled back. This is the safest configuration, as the update/rollback cycle gets very challenging when CRDs are added and removed.

technosophos · 2018-04-28T00:31:56Z

pkg/tiller/hooks.go

 			return e
 		}

-		if entry.Version != "" && !file.apis.Has(entry.Version) {


To be really clear: I am removing this set of checks. It appears to duplicate the check in validateManifest, and it fires well before we have a chance to install the CRD.

tamalsaha · 2018-05-23T03:04:05Z

@technosophos , what should I do if I want to support last 2-3 version of helm with the same chart? Is there a better option that use if-else with Capabilities.TillerVersion ?

bamb00 · 2018-06-22T22:34:14Z

What helm version is this feature going to be in?

bacongobbler · 2018-06-22T23:29:03Z

2.10

gyliu513 · 2018-07-27T07:47:45Z

What is the difference of this fix with #3019 ? @technosophos

gyliu513 · 2018-07-27T07:47:57Z

/cc @morvencao

bacongobbler · 2018-07-27T14:48:41Z

#3019 is for custom resources (CRs) to the sorting list, whereas this PR adds support for custom resource definitions (CRDs). Prior to this hook being available, charts could not install CRs and CRDs because the manifests would be validated all in one step prior to the CRDs being installed.

The docs that were added to this PR goes into a little more detail on this new hook, which can be found here: https://docs.helm.sh/developing_charts/#defining-a-crd-with-the-crd-install-hook

Does that help explain this fix, or is there something else we should document?

gyliu513 · 2018-07-27T14:59:09Z

@bacongobbler I think #3019 is also for CRD but not CR, #3019 is trying to enable all of the CRDs should be created first before other resources take reference of this CRD. Am I missing anything?

This adds support for installing CRDs well before any other resource kinds are installed. This PR introduces a new hook, `crd-install`, that fires before manifests are even validated. It is used to install a CRD before any other part of a chart is installed. Currently, this hook is _only implemented for install_. That means we currently cannot add new CRDs during `helm upgrade`, nor can they be rolled back. This is the safest configuration, as the update/rollback cycle gets very challenging when CRDs are added and removed. Signed-off-by: Sebastien Plisson <sebastien.plisson@gmail.com>

This adds support for installing CRDs well before any other resource kinds are installed. This PR introduces a new hook, `crd-install`, that fires before manifests are even validated. It is used to install a CRD before any other part of a chart is installed. Currently, this hook is _only implemented for install_. That means we currently cannot add new CRDs during `helm upgrade`, nor can they be rolled back. This is the safest configuration, as the update/rollback cycle gets very challenging when CRDs are added and removed.

technosophos self-assigned this Apr 27, 2018

k8s-ci-robot added do-not-merge/work-in-progress size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 27, 2018

technosophos force-pushed the feat/2994-crd-hook branch 3 times, most recently from ed70473 to 8de8ab9 Compare April 28, 2018 00:19

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 28, 2018

technosophos changed the title ~~WIP feat(tiller): support CRD installation~~ feat(tiller): support CRD installation Apr 28, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress label Apr 28, 2018

technosophos force-pushed the feat/2994-crd-hook branch from 8de8ab9 to f2899d3 Compare April 28, 2018 00:28

technosophos added the awaiting review label Apr 28, 2018

technosophos commented Apr 28, 2018

View reviewed changes

adamreese removed the awaiting review label May 11, 2018

adamreese approved these changes May 11, 2018

View reviewed changes

technosophos merged commit 0699ec4 into helm:master May 11, 2018

technosophos deleted the feat/2994-crd-hook branch May 11, 2018 18:09

This was referenced Jun 13, 2018

Use Helm's crd-resource hook cert-manager/cert-manager#651

Closed

Create issuers with helm chart cert-manager/cert-manager#394

Closed

yevhenvolchenko mentioned this pull request Jun 22, 2018

crd-install annotation doesn't work #4259

Closed

munnerz mentioned this pull request Jun 25, 2018

Add validating webhook and webhook tls autoconfiguration cert-manager/cert-manager#478

Merged

gyliu513 mentioned this pull request Jul 28, 2018

CRDs must be installed separately istio/istio.io#1969

Merged

bonifaido mentioned this pull request Oct 4, 2018

ARK chart banzaicloud/banzai-charts#385

Merged

This was referenced Oct 22, 2018

crd-install hook is not working on upgrade #4259 #4697

Closed

Helm is not deleting CRDs #4840

Closed

joshhsoj1902 mentioned this pull request Oct 31, 2018

Lifecycle of a CRD (CustomResourceDefinition) as handled by Helm2 #4863

Closed

sdake mentioned this pull request Nov 9, 2018

Performance Regression: mixer cpu use significantly higher in 1.0.3 istio/istio#9604

Closed

sfrique mentioned this pull request Nov 19, 2018

Upgrade docs 0.4 to 0.5 break CRD fluxcd/flux#1534

Closed

ethiebaut mentioned this pull request Jan 8, 2019

custom_resource_definitions.md recommends use of pre-install instead of crd-install #5138

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(tiller): support CRD installation#3982

feat(tiller): support CRD installation#3982
technosophos merged 1 commit intohelm:masterfrom
technosophos:feat/2994-crd-hook

technosophos commented Apr 27, 2018

Uh oh!

rimusz commented Apr 27, 2018

Uh oh!

technosophos commented Apr 28, 2018

Uh oh!

technosophos Apr 28, 2018

Uh oh!

tamalsaha commented May 23, 2018 •

edited

Loading

Uh oh!

bamb00 commented Jun 22, 2018

Uh oh!

bacongobbler commented Jun 22, 2018

Uh oh!

gyliu513 commented Jul 27, 2018

Uh oh!

gyliu513 commented Jul 27, 2018

Uh oh!

bacongobbler commented Jul 27, 2018 •

edited

Loading

Uh oh!

gyliu513 commented Jul 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

technosophos commented Apr 27, 2018

Uh oh!

rimusz commented Apr 27, 2018

Uh oh!

technosophos commented Apr 28, 2018

Uh oh!

technosophos Apr 28, 2018

Choose a reason for hiding this comment

Uh oh!

tamalsaha commented May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bamb00 commented Jun 22, 2018

Uh oh!

bacongobbler commented Jun 22, 2018

Uh oh!

gyliu513 commented Jul 27, 2018

Uh oh!

gyliu513 commented Jul 27, 2018

Uh oh!

bacongobbler commented Jul 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gyliu513 commented Jul 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

tamalsaha commented May 23, 2018 •

edited

Loading

bacongobbler commented Jul 27, 2018 •

edited

Loading