Enable prometheus metrics for controllers #609
Conversation
Danil-Grigorev
force-pushed
the
add-service-monitor-controller-metrics
branch
from
4068d89
to
a63dd19
Compare
|
/test e2e-openstack |
openshift-ci-robot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
@Danil-Grigorev can you please coordinate with openstack/metal3/ovirt providers so they are aware of this? |
|
/retest |
|
/test e2e-baremetal |
|
@Danil-Grigorev: The specified target(s) for
Use
In response to this:
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. |
- Include PR openshift/machine-api-operator#609 introducing metrics support
- Include PR openshift/machine-api-operator#609 introducing metrics support
- Include PR openshift/machine-api-operator#609 introducing metrics support
- Include PR openshift/machine-api-operator#609 introducing metrics support
|
#640 depends on this PR. |
|
how do we know this work? |
|
@Danil-Grigorev Looks like at this point we are just waiting on the baremetal PR to merge for this to be ready? That PR needs a rebase presently, can you work to try and get that merged please Also, have you taken this for a test run on a real cluster? Would be good to get some proof in the description of this PR that we are seeing metrics coming through (screenshot from prometheus?) |
I'll put the screenshots soon. The main proof that the code is functional, is that the CI tests expect all created serviceMonitor endpoints to be healthy. If the port isn't exposed, it would return 503 on every metrics collection request, thus failing the tests. |
That doesn't necessarily mean that we are exposing metrics there, we could just be displaying an nginx welcome page for all the E2E tests know 😉 It's good to be in a habit of manual testing with features like this, while our test suites are reasonably thorough, there's a lot of stuff they can't test. Also, while they're quite flaky at the moment, it helps if you've manually tested to know if it's a CI issue or it is your issue |
Danil-Grigorev
force-pushed
the
add-service-monitor-controller-metrics
branch
from
a63dd19
to
1515a41
Compare
|
GCP metric triggering on wrong machine configuration resulted in error within API call: |
It is true, but you could manually verify it at any moment. Here is an example of AWS metrics before merging openshift/cluster-api-provider-aws#324. This is also displayed in CI with tests AWS sample metric output: Sample metric firing, captured by the |
|
@enxebre @JoelSpeed I extended the description with a demo for introduced merics running in a GCP cluster. |
This change ensure compatibility with serviceMonitor resource introduced in openshift/machine-api-operator#609
|
/retest |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed 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-robot
added
the
approved
|
/retest |
|
/test e2e-openstack |
|
/lgtm |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@Danil-Grigorev: The following test failed, say
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. |
This change ensures compatibility with the ServiceMonitor resource introduced in [1] [1] openshift/machine-api-operator#609 Signed-off-by: Gal-Zaidman <gzaidman@redhat.com>
this is needed to pull in the change to metrics that happened in MAO: openshift/machine-api-operator#609
This is to comply with openshift/machine-api-operator#609. Without this change alerts are firing as nobody is listening on port 8081 and the metrics can't be exported.
this is needed to pull in the change to metrics that happened in MAO: openshift/machine-api-operator#609
This is to comply with openshift/machine-api-operator#609. Without this change alerts are firing as nobody is listening on port 8081 and the metrics can't be exported.
this is needed to pull in the change to metrics that happened in MAO: openshift/machine-api-operator#609
This is to comply with openshift/machine-api-operator#609. Without this change alerts are firing as nobody is listening on port 8081 and the metrics can't be exported.
Enables functionality introduced in #590
Depends on
Demo:
