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 disableValidation and disableOpenAPIValidation per release #1373

Merged
merged 1 commit into from
Jul 22, 2020

Conversation

mumoshu
Copy link
Collaborator

@mumoshu mumoshu commented Jul 22, 2020

disableOpenAPIValidation: true might be useful for workaround for broken CRDs that is known to be exist in older OpenShift versions, and disableValidation: true is confirmed to allow installing charts like prometheus-operator that tries to install CRDs and CRs in the same chart.

Strictly speaking, for the latter case I believe you only need disableValidation: true set during the first installation, but for the ease of operation I shall suggest you to always set it.

Obviously turning validation mostly(disableOpenAPIValidation) or entirely(disableValidation) result in deferring any real error until sync time. We need completely client-side validation that is able to read CRDs and use it for validating any CRs to catch any error before sync. But it worth an another (big) issue.

Note that this feature requires helm-diff greater than or equal to v3.1.2

Fixes #1124

`disableOpenAPIValidation: true` might be useful for workaround for broken CRDs that is known to be exist in older OpenShift versions, and `disableValidation: true` is confirmed to allow installing charts like prometheus-operator that tries to install CRDs and CRs in the same chart.

Strictly speaking, for the latter case I believe you only need `disableValidation: true` set during the first installation, but for the ease of operation I shall suggest you to always set it.

Obviously turning validation mostly(disableOpenAPIValidation) or entirely(disableValidation) result in deferring any real error until sync time. We need completely client-side validation that is able to read CRDs and use it for validating any CRs to catch any error before sync. But it worth an another (big) issue.

Fixes #1124
@mumoshu mumoshu merged commit 4fde6e1 into master Jul 22, 2020
@mumoshu mumoshu deleted the disable-validations branch July 22, 2020 14:10
mumoshu added a commit that referenced this pull request Jul 22, 2020
…as chart

This, in combination with #1172, allows you to use `go-getter`-supported URL for K8s manifests on `chart`, so that Helmfile automatically fetches it and then turning it into a temporary local chart, which is then installed by Helmfile as similar as standard Helm charts.

An example usecase of this is to install cert-manager CRDs which is distributed separately from the chart:

```
releases:
- name: cert-manager-crds
  chart: git::http://github.com/jetstack/cert-manager.git@deploy/crds?ref=v0.15.2
```

I'm adding this based on discussion with @lukasmrtvy. He was trying to install cert-manager and prometheus-opreator with Helmfile, and this combined with #1373 should do the job. Thanks for the input!
mumoshu added a commit that referenced this pull request Jul 22, 2020
…as chart (#1374)

This, in combination with #1172, allows you to use `go-getter`-supported URL for K8s manifests on `chart`, so that Helmfile automatically fetches it and then turning it into a temporary local chart, which is then installed by Helmfile as similar as standard Helm charts.

An example usecase of this is to install cert-manager CRDs which is distributed separately from the chart:

```
releases:
- name: cert-manager-crds
  chart: git::http://github.com/jetstack/cert-manager.git@deploy/crds?ref=v0.15.2
```

I'm adding this based on discussion with @lukasmrtvy. He was trying to install cert-manager and prometheus-opreator with Helmfile, and this combined with #1373 should do the job. Thanks for the input!
@4c74356b41
Copy link

hey, would that help with not verifying cert-manager certificate resource? the thing is, i'm trying to do a deployment to a shutdown cluster and I dont want it to validate\wait for any of the resources to come up, I just need it to apply and show diffs.

@mumoshu
Copy link
Collaborator Author

mumoshu commented Aug 28, 2020

@4c74356b41 Yep. disableValidation: true should work for that case.

@4c74356b41
Copy link

mhm, actually, its trying to patch it everytime, so I dont think that will work :(

@mumoshu
Copy link
Collaborator Author

mumoshu commented Aug 28, 2020

@4c74356b41 Sry, but what do you mean by its trying to patch it everytime? disableValidation: true only disables k8s resource validation against real k8s server that is usually done by helm-diff. It should have nothing to do with patching.

@4c74356b41
Copy link

well, its trying to update the resource even when there are no changes - hence it requires talking to the cert-manager webhook:

Error: UPGRADE FAILED: cannot patch "testing-cert" with kind Certificate: Timeout: request did not complete within requested timeout 34s
2020-08-28T06:29:01.6418359Z helmfile indicated failure (exit code 1; full command: helmfile apply).

and I dont want to keep this testing cluster running, its only needed to validate certain things before pushing to develop, so automated CI build.

@mumoshu
Copy link
Collaborator Author

mumoshu commented Aug 28, 2020

@4c74356b41 Well, your issue seems not to relate to validation. helmfile apply uses helm diff to detect if there are changes. If and only if there are changes, helmfile apply runs helm upgrade --install, which should result in the cert-manager admission webhook to come in.

Also, the error indicates that you've already installed cert-manager, but the admission webhook isn't working due to some reason.

If you really encountered validation error while installing cert-manager and Certificate custom resources on the first install, do use disableValidation: true.

If you already installed cert-manager and its admission webhook, ensure that the admission webhook pods are ready and working.

@4c74356b41
Copy link

thats what I said, I never said I do diff, I said I need it to apply and show the diff, but it appears that helm does try to apply the resources that didn't change, hence the issue

@mumoshu
Copy link
Collaborator Author

mumoshu commented Aug 28, 2020

@4c74356b41 Sorry perhaps I had misread your original comment?

hey, would that help with not verifying cert-manager certificate resource? the thing is, i'm trying to do a deployment to a shutdown cluster and I dont want it to validate\wait for any of the resources to come up, I just need it to apply and show diffs.

So, are you just trying to defer helm-upgrade for installing Certficate resources, to happen only after cert-manager is up and running?

@4c74356b41
Copy link

nah, I have a test cluster, which is shutdown most of the time. I use it to test my ci builds, but if its shutdown the certificate that gets deployed times out, because cert-manager is shutdown. I dont think there is any solution to this, unless I configure it to skip certificate deployment on CI builds

@mumoshu
Copy link
Collaborator Author

mumoshu commented Aug 28, 2020

@4c74356b41 Thanks for the response. Sorry but I'm still unable to fully understand your use-case.

nah, I have a test cluster, which is shutdown most of the time

What do you mean by test cluster exactly? Literally the whole K8s cluster, or a set of Helm releases managed by Helmfile?

I thought it's the former(=literally the whole K8s cluster) initially. But then it should be a matter of #1373 (comment). You said it isn't, that remains me confused.

I use it to test my ci builds, but if its shutdown the certificate that gets deployed times out, because cert-manager is shutdown

When is cert-manager up then?

Without knowing details of your setup, I can suggest a few options. Running helmfile apply multiple times with helmfile selectors like helmfile -l phase1 apply && helmfile -l phase2 apply and deploy cert-manager in phase1 and certs in phase2. Note that you need to label your releases within helmfile.yaml.

Otherwise, you should be able to use disableValidation: true to disable validation, while using hooks to defer installing certs until cert-manager is up and running. I mean you can run something like while ! kubectl wait deployment cert-manager; do sleep 1; done in a helmfile hook e.g. presync for the certs release so that is installed after the cert-manager webhook gets ready.

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.

Cannot install prometheus-operator chart with helmfile due to helm-diff error
2 participants