Skip to content

Bundle ServiceMonitor CRD with the chart#11261

Merged
aanm merged 2 commits intocilium:masterfrom
diversario:bundle-service-monitor-crd
May 29, 2020
Merged

Bundle ServiceMonitor CRD with the chart#11261
aanm merged 2 commits intocilium:masterfrom
diversario:bundle-service-monitor-crd

Conversation

@diversario
Copy link
Copy Markdown
Contributor

@diversario diversario commented Apr 30, 2020

This allows installing Cilium with service monitor enabled
even if Prometheus hasn't been installed yet.

By including the ServiceMonitor CRD under crds/ directory we ensure
that Helm will install it first iff the cluster has no existing
ServiceMonitor CRD. This behavior is documented at
https://helm.sh/docs/topics/charts/#custom-resource-definitions-crds

Fixed errors such as

% helm install cilium ./cilium --namespace kube-system --set global.prometheus.enabled=true--set --set global.prometheus.serviceMonitor.enabled=true
Error: unable to build kubernetes objects from release manifest: unable to recognize "": no matches for kind "ServiceMonitor" in version "monitoring.coreos.com/v1"

This requires Helm 3; Helm 2 will ignore the crds/ directory. The CRD is copied from this commit of the helm/charts repo.

Signed-off-by: Ilya Shaisultanov ilya.shaisultanov@gmail.com

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

Fixes: #issue-number

Allow enabling ServiceMonitor without Prometheus installed.

@diversario diversario requested a review from a team April 30, 2020 16:59
Copy link
Copy Markdown
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

thank you for taking care of this

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 30, 2020

Coverage Status

Coverage decreased (-0.01%) to 36.917% when pulling f991681 on diversario:bundle-service-monitor-crd into 87f0905 on cilium:master.

@aanm aanm added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Apr 30, 2020
Comment thread install/kubernetes/cilium/crds/crd-servicemonitor.yaml Outdated
@aanm
Copy link
Copy Markdown
Member

aanm commented May 18, 2020

Discussed offline how to fix this. Waiting for implementation.

@aanm aanm self-requested a review May 28, 2020 08:57
This allows installing Cilium with service monitor enabled
even if Prometheus hasn't been installed yet.

By including the ServiceMonitor CRD under `crds/` directory we ensure
that Helm will install it first iff the cluster has no existing
ServiceMonitor CRD. This behavior is documented at
https://helm.sh/docs/topics/charts/#custom-resource-definitions-crds

This requires Helm 3; Helm 2 will ignore the crds/ directory.

Signed-off-by: Ilya Shaisultanov <ilya.shaisultanov@gmail.com>
@diversario diversario force-pushed the bundle-service-monitor-crd branch from 3a3de8f to e8ded27 Compare May 28, 2020 09:30
@diversario diversario requested a review from a team as a code owner May 28, 2020 09:30
@maintainer-s-little-helper
Copy link
Copy Markdown

Commit e8ded27aa4777a42274f0ae802399c5b255cbe47 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper Bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 28, 2020
Signed-off-by: Ilya Shaisultanov <ilya.shaisultanov@gmail.com>
@diversario diversario force-pushed the bundle-service-monitor-crd branch from e8ded27 to f991681 Compare May 28, 2020 09:45
@maintainer-s-little-helper maintainer-s-little-helper Bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 28, 2020
@aanm
Copy link
Copy Markdown
Member

aanm commented May 28, 2020

test-me-please

@maintainer-s-little-helper
Copy link
Copy Markdown

Commit b39b02eeac49be097f162ba7bb3b7a92fc21adca does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper Bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 29, 2020
@aanm aanm force-pushed the bundle-service-monitor-crd branch from b39b02e to f991681 Compare May 29, 2020 14:58
@maintainer-s-little-helper maintainer-s-little-helper Bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 29, 2020
@aanm aanm merged commit 9735010 into cilium:master May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants