ansible: add custom resource metrics #1723
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
size/M
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/test e2e-aws-ansible |
| fi | ||
|
|
||
| # verify that the metrics endpoint exists | ||
| if ! timeout 1m bash -c -- "until kubectl run -it --rm --restart=Never test-metrics --image=registry.access.redhat.com/ubi7/ubi-minimal:latest -- curl -sfo /dev/null http://memcached-operator-metrics:8383/metrics; do sleep 1; done"; |
There was a problem hiding this comment.
I believe you asked on slack how to test custom resource metrics.
So this is for the controller-runtime metrics, you could add the same test for the custom resource metrics, so seeing if the metrics exists on 8686 port.
Another thing you could do is, we know exactly what the metrics data will be so you could curl the endpoint and compare the result to a predefined string. If you look at the go e2e test you can see how the metrics data would look like here
operator-sdk/test/e2e/_incluster-test-code/memcached_test.go
Lines 392 to 395 in d6e1aba
Hope that makes sense, otherwise feel free to ask any questions!
There was a problem hiding this comment.
Note that the verification needs to happen after line 79, so after creating CR instance. As that is when the metric should be 1.
There was a problem hiding this comment.
I think we wanted to add the same check for the other metrics port so port 8686.
There was a problem hiding this comment.
I believe I was able to manage it the check for port 8686 and for the CR info. Not having much experience with these metrics I just used grep. Let me know what you think. Thank you for the help.
|
LGTM once Travis is passing, will take a deeper dive to see why it's failing |
| fi | ||
|
|
||
| # verify that the metrics endpoint exists | ||
| if ! timeout 1m bash -c -- "until kubectl run -it --rm --restart=Never test-metrics --image=registry.access.redhat.com/ubi7/ubi-minimal:latest -- curl -sfo /dev/null http://memcached-operator-metrics:8383/metrics; do sleep 1; done"; |
There was a problem hiding this comment.
I think we wanted to add the same check for the other metrics port so port 8686.
|
Hi David, just wanted to touch back to see if you have an idea for when you can get the last couple of nits so we can close/merge this? |
Perfect timing 😎. I've been struggling to get back to this but I have this afternoon to work on it. If you've seen no changes by Tuesday then I've messed up. |
djzager
force-pushed
the
ao-custom-metrics
branch
2 times, most recently
from
9034fac
to
4ab0ba0
Compare
djzager
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
Figuring out the check and then this will be ready. |
djzager
force-pushed
the
ao-custom-metrics
branch
from
4ab0ba0
to
f1775d3
Compare
This PR enables custom resource metrics for ansible based operators.
djzager
force-pushed
the
ao-custom-metrics
branch
from
f1775d3
to
c07cea7
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/test e2e-aws-ansible |
In the Ansible Operator tests, we should verify that the metrics reflect that an instance of the memcached resource has been created. Also add the change to the Changelog.
djzager
force-pushed
the
ao-custom-metrics
branch
from
c07cea7
to
5e2315c
Compare
|
@joelanford @theishshah I believe this is ready to go. Thank you for your patience. |
Description of the change:
This PR enables custom resource metrics for ansible based operators.
Motivation for the change:
Closes #1554