search

pravega / zookeeper-operator

Kubernetes Operator for Zookeeper
Apache License 2.0
368 stars 206 forks source link

Issue-581 Make metrics exposable based on helm variables #582

Closed joshsouza closed 11 months ago

joshsouza commented 11 months ago

Resolves: https://github.com/pravega/zookeeper-operator/issues/581

codecov[bot] commented 11 months ago

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (88b9307) 85.12% compared to head (794565c) 85.91%. Report is 1 commits behind head on master.

Files Patch % Lines
controllers/zookeepercluster_controller.go 70.00% 3 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #582 +/- ## ========================================== + Coverage 85.12% 85.91% +0.79% ========================================== Files 12 12 Lines 1613 1633 +20 ========================================== + Hits 1373 1403 +30 + Misses 155 145 -10 Partials 85 85 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jkhalack commented 11 months ago

Has there been any testing done to prove that the modified chart allows exposing metrics for Prometheus?

joshsouza commented 11 months ago

Has there been any testing done to prove that the modified chart allows exposing metrics for Prometheus?

I utilized helm template when writing the code to ensure the appropriate adjustments to the k8s resources, and deploying it to a local kind cluster before/after with a second curl pod deployed to test connectivity validates that without these changes I am unable to curl <pod_ip>:6000/metrics (connection refused) beforehand, and with --set metricsBindAddress=0.0.0.0 in my helm update/install parameters, I am able to successfully reach the metrics endpoint from my curl pod afterwards.

While I didn't explicitly test that Prometheus itself was working, this should be a sufficient test to verify that the changes are functioning as intended, and should allow metrics gathering by other pods in the cluster.