cp-helm-charts icon indicating copy to clipboard operation
cp-helm-charts copied to clipboard

Add support for Prometheus Operator's ServiceMonitor resource

Open tczekajlo opened this issue 5 years ago • 8 comments

What changes were proposed in this pull request?

  • add support for ServiceMonitor resource which is provided by prometheus-operator

How was this patch tested?

helm upgrade/install on a testing cluster, k8s v1.11.8, prometheus-operator-4.3.3 chart

tczekajlo avatar Mar 14 '19 10:03 tczekajlo

It looks like @tczekajlo hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence. Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

ghost avatar Mar 14 '19 10:03 ghost

[clabot:check]

tczekajlo avatar Mar 17 '19 11:03 tczekajlo

@confluentinc It looks like @tczekajlo just signed our Contributor License Agreement. :+1:

Always at your service,

clabot

ghost avatar Mar 17 '19 11:03 ghost

Could you update the docs also

qshao-pivotal avatar Mar 22 '19 22:03 qshao-pivotal

@qshao-pivotal I've updated docs. I hope this is the correct place.

tczekajlo avatar Apr 20 '19 08:04 tczekajlo

This looks super helpful and ready to go, with documentation updates in place. Any reason why it hasn't been merged?

terryf82 avatar Oct 07 '20 04:10 terryf82

  Error: unable to build kubernetes objects from release manifest: error validating "": error validating data: ValidationError(Service.spec.ports[0].port): invalid type for io.k8s.api.core.v1.ServicePort.port: got "string", expected "integer"
  Error: plugin "diff" exited with error

I get this when running through helmfile

the fix is easy. remove the part in bold

spec: ports: - port: {{ .Values.prometheus.jmx.port | quote }} name: metrics

~~another thing to note: servicemonitor definitions must be in the same namespace as prometheus. by default this puts the servicemonitor in the helm namespace, which may or may not be the same as prometheus~~ note: Must override this value in order to use servicemonitors outside of the prometheus namespace:

    - prometheus:
        prometheusSpec:
          serviceMonitorSelectorNilUsesHelmValues: false

oct 13 2020 update: charts/cp-*/templates/service-metrics.yaml can be deleted since master already defines this service in .../service.yaml.

azhang avatar Oct 13 '20 02:10 azhang

Any reason for this to still not be merged? Seems like the checks failed because of server maintenance, could we retry the checks?

Jefski14 avatar Jan 03 '22 09:01 Jefski14