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

Cannot install prometheus-operator chart with helmfile due to helm-diff error #1124

Closed
mojochao opened this issue Feb 26, 2020 · 26 comments · Fixed by #1373
Closed

Cannot install prometheus-operator chart with helmfile due to helm-diff error #1124

mojochao opened this issue Feb 26, 2020 · 26 comments · Fixed by #1373

Comments

@mojochao
Copy link

Versions used:

  • helmfile v0.100.2
  • helm v3.1.1
  • databus23/helm-diff plugin v3.1.1

Problem observed:

I can successfully install the stable/prometheus-operator chart with helm directly.

$ helm install prometheus-operator stable/prometheus-operator -n monitoring
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
NAME: prometheus-operator
LAST DEPLOYED: Wed Feb 26 13:13:58 2020
NAMESPACE: monitoring
STATUS: deployed
REVISION: 1
NOTES:
The Prometheus Operator has been installed. Check its status by running:
  kubectl --namespace monitoring get pods -l "release=prometheus-operator"

Visit https://github.com/coreos/prometheus-operator for instructions on how
to create & configure Alertmanager and Prometheus instances using the Operator.

I cannot install the same chart with helmfile:

$ helmfile apply
Comparing release=prometheus-operator, chart=stable/prometheus-operator
in ./helmfile.yaml: in .helmfiles[0]: in helmfiles/prometheus-operator.yaml: failed processing release prometheus-operator: helm exited with status 1:
  Error: Failed to render chart: exit status 1: manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
  manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
  manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
  manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
  manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
  Error: unable to build kubernetes objects from release manifest: [unable to recognize "": no matches for kind "Alertmanager" in version "monitoring.coreos.com/v1", unable to recognize "": no matches for kind "Prometheus" in version "monitoring.coreos.com/v1", unable to recognize "": no matches for kind "PrometheusRule" in version "monitoring.coreos.com/v1", unable to recognize "": no matches for kind "ServiceMonitor" in version "monitoring.coreos.com/v1"]

  Error: plugin "diff" exited with error

I see that the error is coming out of the helm-diff plugin. What command(s) is helmfile using that causes these errors to surface? I will open an issue in the databus23/helm-diff plugin repo once I understand how to repro this with the helm diff command directly.

Many thanks in advance!

@mojochao mojochao changed the title Cannot install prometheus-operator chart with helmfile Cannot install prometheus-operator chart with helmfile due to helm-diff error Feb 26, 2020
@mojochao
Copy link
Author

I made an educated guess as to the helm diff command:

$ helm diff --allow-unreleased upgrade -n monitoring prometheus-operator stable/prometheus-operator
********************

	Release was not present in Helm.  Diff will show entire contents as new.

********************
Error: Failed to render chart: exit status 1: manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
manifest_sorter.go:192: info: skipping unknown hook: "crd-install"
Error: unable to build kubernetes objects from release manifest: [unable to recognize "": no matches for kind "Alertmanager" in version "monitoring.coreos.com/v1", unable to recognize "": no matches for kind "Prometheus" in version "monitoring.coreos.com/v1", unable to recognize "": no matches for kind "PrometheusRule" in version "monitoring.coreos.com/v1", unable to recognize "": no matches for kind "ServiceMonitor" in version "monitoring.coreos.com/v1"]

Error: plugin "diff" exited with error

Hopefully that command is in the ballpark of the real command. My guess is that until the helm diff command succeeds, helmfile won't be able to successfully install that chart. :-) I'll report the Issue in their repo.

@mojochao
Copy link
Author

Issue being tracked in databus23/helm-diff repo with databus23/helm-diff#189

@andrewnazarov
Copy link
Contributor

What version of prometheus-operator are you trying to install?

@andrewnazarov
Copy link
Contributor

Probably your version is a bit outdated. Helm 3 works differently with CRDs: https://helm.sh/docs/topics/chart_best_practices/custom_resource_definitions/

@mojochao
Copy link
Author

mojochao commented Feb 27, 2020 via email

@andrewnazarov
Copy link
Contributor

We are successfully deploying version 8.7.0 with Helm 3.

Can you check the exact version of the prometheus-operator chart that is used?

@imtpot
Copy link

imtpot commented Mar 24, 2020

I'm using hooks to workaround

hooks:
  - events: ["prepare"]
    command: "/bin/sh"
    args: ["-c", "kubectl get crd prometheuses.monitoring.coreos.com >/dev/null 2>&1 || \
            kubectl apply --validate=false -f https://raw.githubusercontent.com/coreos/prometheus-operator/release-0.37/example/prometheus-operator-crd/monitoring.coreos.com_alertmanagers.yaml"]
  - events: ["prepare"]
    command: "/bin/sh"
    args: ["-c", "kubectl get crd alertmanagers.monitoring.coreos.com >/dev/null 2>&1 || \
            kubectl apply --validate=false -f https://raw.githubusercontent.com/coreos/prometheus-operator/release-0.37/example/prometheus-operator-crd/monitoring.coreos.com_podmonitors.yaml"]
  - events: ["prepare"]
    command: "/bin/sh"
    args: ["-c", "kubectl get crd prometheusrules.monitoring.coreos.com >/dev/null 2>&1 || \
            kubectl apply --validate=false -f https://raw.githubusercontent.com/coreos/prometheus-operator/release-0.37/example/prometheus-operator-crd/monitoring.coreos.com_prometheuses.yaml"]
  - events: ["prepare"]
    command: "/bin/sh"
    args: ["-c", "kubectl get crd servicemonitors.monitoring.coreos.com >/dev/null 2>&1 || \
            kubectl apply --validate=false -f https://raw.githubusercontent.com/coreos/prometheus-operator/release-0.37/example/prometheus-operator-crd/monitoring.coreos.com_prometheusrules.yaml"]
  - events: ["prepare"]
    command: "/bin/sh"
    args: ["-c", "kubectl get crd podmonitors.monitoring.coreos.com >/dev/null 2>&1 || \
        kubectl apply --validate=false -f https://raw.githubusercontent.com/coreos/prometheus-operator/release-0.37/example/prometheus-operator-crd/monitoring.coreos.com_servicemonitors.yaml"]
  - events: ["prepare"]
    command: "/bin/sh"
    args: ["-c", "kubectl get crd podmonitors.monitoring.coreos.com >/dev/null 2>&1 || \
        kubectl apply --validate=false -f https://raw.githubusercontent.com/coreos/prometheus-operator/release-0.37/example/prometheus-operator-crd/monitoring.coreos.com_thanosrulers.yaml"]  
  set:
  - name: prometheusOperator.createCustomResource
    value: false

@mojochao
Copy link
Author

mojochao commented Mar 25, 2020 via email

@rchenzheng
Copy link

rchenzheng commented Mar 30, 2020

This is still happening as of helmfile (v0.106.3), helm-diff (v3.1.1), and helm (v3.1.2)

What prometheus-operator chart version are you using ?@mojochao

I seem to have the same issue with 8.12.3

@giannisbetas
Copy link

helmfile: v0.109.0
helm: v3.1.2
helm-diff: 3.1.1

Chart:
prometheus-operator: 8.12.13

The issue is not solved.
In order to work around it I had to install the CRDs manually

@logston
Copy link

logston commented Apr 24, 2020

Similar. But a different chart.

Bash:

$ helmfile --version
helmfile version v0.109.1
$ helm version
version.BuildInfo{Version:"v3.2.0", GitCommit:"e11b7ce3b12db2941e90399e874513fbd24bcb71", GitTreeState:"clean", GoVersion:"go1.13.10"}
$ kubectl version
Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.0", GitCommit:"9e991415386e4cf155a24b1da15becaa390438d8", GitTreeState:"clean", BuildDate:"2020-03-25T14:58:59Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"15+", GitVersion:"v1.15.11-eks-af3caf", GitCommit:"af3caf6136cd355f467083651cc1010a499f59b1", GitTreeState:"clean", BuildDate:"2020-03-27T21:51:36Z", GoVersion:"go1.12.17", Compiler:"gc", Platform:"linux/amd64"}
$ helm plugin list
NAME    VERSION DESCRIPTION                           
diff    3.1.1   Preview helm upgrade changes as a diff

helmfile:

repositories:
- name: dynatrace
  url: https://raw.githubusercontent.com/Dynatrace/helm-charts/master/repos/stable

releases:
  - name: dynatrace-oneagent-operator
    namespace: dynatrace
    chart: dynatrace/dynatrace-oneagent-operator
    version: ~0.7.0

@mumoshu
Copy link
Collaborator

mumoshu commented Apr 24, 2020

Isn't this chart issue, rather than helm or helmfile's?

I mean, are those charts migrated to internally use the new crds directory added in Helm 3?

@Xartos
Copy link

Xartos commented Apr 24, 2020

I would rather say a helm 3 issue. Since doing helm install --dry-run don't work for charts with crds, see helm/helm#7449
And in the end that's why this fails as well, you can't verify objects that aren't yet defined in the API-server.

In my opinion the best solution would be the one suggested in that issue, but it doesn't seem to be a priority atm. So another solution would be to have a way to disable diff in helmfile when the release aren't present (when installing), like a --skip-diff-if-installing flag when doing apply or something.

@mumoshu
Copy link
Collaborator

mumoshu commented Apr 24, 2020

Thanks. Interesting idea!

Just curious, but do you think how could Helmfile determines if it's an install or upgrade?

@Xartos
Copy link

Xartos commented Apr 24, 2020

I don't know how you would do it optimally, but I guess it could do helm list and see if the release exists or something similar.

@andrewnazarov
Copy link
Contributor

But does helmfile ever do helm install --dry-run?
helm-diff plugin does helm upgrade --debug --dry-run

@Xartos
Copy link

Xartos commented Apr 24, 2020

It probably don't. The point I was trying to make is that the root cause is the same. As long as you try to validate an object that is not defined in the API-server, the validation will fail.

It can't be running that for an install right, atleast not without the --install flag?
I know that helm-diff runs helm template ... --validate which also validates the rendered manifests to the API-server and thus the same error occurs.

@baurmatt
Copy link
Contributor

We currently using the following workaround:

...
    hooks:
    # Create CRDs separately in helmfile presync hooks
    # https://github.com/roboll/helmfile/issues/1124
    # https://github.com/helm/helm/issues/7449
    # https://github.com/cloudposse/helmfiles/blob/59490fd2599d6113a14103be919985f9fbcea73a/releases/prometheus-operator.yaml
    # Hooks associated to presync events are triggered before each release is applied to the remote cluster.
    # This is the ideal event to execute any commands that may mutate the cluster state as it
    # will not be run for read-only operations like lint, diff or template.
    # These hook install the prometheuses.monitoring.coreos.com CustomResourceDefinition if needed
    - events: ["presync"]
      command: "/bin/sh"
      args: ["-c", "kubectl get crd prometheuses.monitoring.coreos.com >/dev/null 2>&1 || \
             { helm pull stable/prometheus-operator --version {{`{{ .Release.Version }}`}} && tar -Oxzf prometheus-operator-{{`{{ .Release.Version }}`}}.tgz prometheus-operator/crds/crd-prometheus.yaml | kubectl apply -f -; }"]
    # This hoook installs the alertmanagers.monitoring.coreos.com CustomResourceDefinition if needed
    - events: ["presync"]
      command: "/bin/sh"
      args: ["-c", "kubectl get crd alertmanagers.monitoring.coreos.com >/dev/null 2>&1 || \
             { helm pull stable/prometheus-operator --version {{`{{ .Release.Version }}`}} && tar -Oxzf prometheus-operator-{{`{{ .Release.Version }}`}}.tgz prometheus-operator/crds/crd-alertmanager.yaml | kubectl apply -f -; }"]
    # This hoook installs the prometheusrules.monitoring.coreos.com CustomResourceDefinition if needed
    - events: ["presync"]
      command: "/bin/sh"
      args: ["-c", "kubectl get crd prometheusrules.monitoring.coreos.com >/dev/null 2>&1 || \
             { helm pull stable/prometheus-operator --version {{`{{ .Release.Version }}`}} && tar -Oxzf prometheus-operator-{{`{{ .Release.Version }}`}}.tgz prometheus-operator/crds/crd-prometheusrules.yaml | kubectl apply -f -; }"]
    # This hoook installs the servicemonitors.monitoring.coreos.com CustomResourceDefinition if needed
    - events: ["presync"]
      command: "/bin/sh"
      args: ["-c", "kubectl get crd servicemonitors.monitoring.coreos.com >/dev/null 2>&1 || \
             { helm pull stable/prometheus-operator --version {{`{{ .Release.Version }}`}} && tar -Oxzf prometheus-operator-{{`{{ .Release.Version }}`}}.tgz prometheus-operator/crds/crd-servicemonitor.yaml | kubectl apply -f -; }"]
    # This hoook installs the podmonitors.monitoring.coreos.com CustomResourceDefinition if needed
    - events: ["presync"]
      command: "/bin/sh"
      args: ["-c", "kubectl get crd podmonitors.monitoring.coreos.com >/dev/null 2>&1 || \
             { helm pull stable/prometheus-operator --version {{`{{ .Release.Version }}`}} && tar -Oxzf prometheus-operator-{{`{{ .Release.Version }}`}}.tgz prometheus-operator/crds/crd-podmonitor.yaml | kubectl apply -f -; }"]
    # This hoook installs the thanosrulers.monitoring.coreos.com CustomResourceDefinition if needed
    - events: ["presync"]
      command: "/bin/sh"
      args: ["-c", "kubectl get crd thanosrulers.monitoring.coreos.com >/dev/null 2>&1 || \
             { helm pull stable/prometheus-operator --version {{`{{ .Release.Version }}`}} && tar -Oxzf prometheus-operator-{{`{{ .Release.Version }}`}}.tgz prometheus-operator/crds/crd-thanosrulers.yaml | kubectl apply -f -; }"]
...

It's similar to the one posted above by @agmtr but uses the CRDs directly from the chart. This increases compatibility because it uses the same version which is normally bundled with the chart - just deploys it with a presync hook.

@semoac
Copy link

semoac commented May 19, 2020

As a workaround I installed helm-diff 3.0.0-rc.7

@Barna001
Copy link

Barna001 commented Jun 4, 2020

I had the exact same problem, used "8.13.12" version for prometheus-operator and it worked :)
Helm 3 does not support crd-install hook anymore.

@bradenwright
Copy link

FWIW using helmfile sync works, but using sync skips the diff. Right now I've been using sync on my first install and diff/apply after that.

@mumoshu
Copy link
Collaborator

mumoshu commented Jul 20, 2020

I thought helm-diff recently added --disable-openapi-validation that can be used for disabling validation at all which will also makes this issue disappear.

Can we probably enhance Helmfile to add a new option under releases[] to enable it? e.g.

releases:
- name: prometheus-operator
  chart: stable/prometheus-operator
  # Set this to true if your chart contains crds or install-crd hooks.
  disableOpenAPIValidation: true

@mumoshu
Copy link
Collaborator

mumoshu commented Jul 20, 2020

databus23/helm-diff#220

@mumoshu
Copy link
Collaborator

mumoshu commented Jul 22, 2020

Apparently --disable-openapi-validation doesn't work(It can be a solution for other problems)

But I've managed to make prometheus-operator installation work with --disable-validation I've added to helm-diff today

databus23/helm-diff#228

mumoshu added a commit that referenced this issue 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.

Fixes #1124
mumoshu added a commit that referenced this issue 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.

Fixes #1124
@mumoshu
Copy link
Collaborator

mumoshu commented Jul 22, 2020

So after #1373 this should work

releases:
- name: prometheus-operator
  chart: stable/prometheus-operator
  disableValidation: true

@Xtigyro
Copy link

Xtigyro commented Feb 3, 2021

@mumoshu Thank you, man - as always you rock!!!

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 a pull request may close this issue.