Enable prometheus metrics scraping #156
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alebedev87 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
alebedev87
force-pushed
the
finishup-metrics
branch
from
9f8446f
to
36060d2
Compare
|
/assign @DhritiShikhar |
|
Should there be any tests for this @alebedev87 ? |
Makefile
Outdated
| @@ -156,6 +156,10 @@ deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in | |||
| cd config/manager && $(KUSTOMIZE) edit set image quay.io/openshift/origin-external-dns-operator=${IMG} | |||
| # webhook volume and service are added explicilty so that they don't land in the bundle where it's managed by OLM | |||
| cd config/default && $(KUSTOMIZE) edit add patch --path=manager_webhook_volume_patch.yaml | |||
| # add service monitor with certificate from the service serving certificate | |||
| cd config/prometheus && $(KUSTOMIZE) edit add resource monitor.yaml | |||
There was a problem hiding this comment.
Why not add the monitor resource directly or put another way include it in the default target.
There was a problem hiding this comment.
Done, added monitor.yaml as default resource, tlsConfig patch is added for make deploy only (could not do it for the bundle to not leave uncommitted changes).
|
|
||
| The ExternalDNS Operator exposes [controller-runtime metrics](https://book-v1.book.kubebuilder.io/beyond_basics/controller_metrics.html) in the format expected by [Prometheus Operator](https://github.com/prometheus-operator/prometheus-operator). | ||
| `ServiceMonitor` object is created in the operator's namespace (by default `external-dns-operator`), make sure that your instance of the Prometheus Operator is properly configured to find it. | ||
| You can check `.spec.serviceMonitorNamespaceSelector` and `.spec.serviceMonitorSelector` fields of `prometheus` resource and edit the operator's namespace or service monitor accordingly: |
There was a problem hiding this comment.
If the namespace can be add by adding a label then shouldn't we be adding the label to the namespace resource? Or is it not possible when the namespace is created by OLM.
There was a problem hiding this comment.
Yes, OLM creates the installation namespace and I didn't see a way to customize it.
Added labeling of the namespace in README when the bundle is generated.
There was a problem hiding this comment.
AFAIK the cluster prometheus should not be modified to monitor non cluster operator. Instead another prometheus instance should be used. The instructions are here.
| @@ -6,16 +6,11 @@ resources: | |||
| - ../rbac | |||
| - ../manager | |||
| - ../webhook | |||
| - ../prometheus | |||
There was a problem hiding this comment.
So since the prometheus directory is already included do you need to add it with kustomize as well?
There was a problem hiding this comment.
Not sure I understood this one. It's already in kustomize. You can deploy using make deploy or with a bundle. Both ways will create ServieMonitor and all the necessary things.
There was a problem hiding this comment.
My point is was that if the ServiceMonitor was already part of the kustomize do you need to add it again for the make deploy target.
|
Do you also want to explicity set the metrics port while initializing the operator so that the configuration matches what is currently implicit. |
alebedev87
force-pushed
the
finishup-metrics
branch
from
36060d2
to
216a8a1
Compare
alebedev87
force-pushed
the
finishup-metrics
branch
from
216a8a1
to
ed650e8
Compare
|
@DhritiShikhar : for the e2e test we'd need a reliable way of verifying the target creation in prometheus. Will try to find something. |
|
/lgtm |
|
As discussed with Dhriti, the e2e testing is possible but quite heavy. Prometheus targets are not exposed on the Kubernetes level (CRs or ConfigMaps/Secrets), they have to be retrieved from the prometheus API which most often is queried by |
|
/assign @quarterpin |
|
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
|
/assign @xenolinux |
|
/label px-approved |
openshift-ci
bot
added
the
px-approved
|
/label docs-approved Don't expect any docs update for the built-in controller-runtime metrics. |
openshift-ci
bot
added
the
docs-approved
|
@alebedev87: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Result:
