search

outscale / osc-cost

Analysis of cloud costs for 3DS Outscale
5 stars 3 forks source link

[helm] Fix for param name and probes #107

Closed albundy83 closed 1 year ago

albundy83 commented 1 year ago

Hello, I have made an error in the previous merge request, sorry for that.

The idea to pull /metrics to perform the readiness and liveness probes was a terrible idea, it totally overload the api calls (especially if you did not disable oos count) So, I remove it. Docker need to be rebuilt by the way, should I update release version also ?

albundy83 commented 1 year ago

Well, even with the tcpSocket, nc command is triggered. Until, osc-cost has a better /metrics, I should disable livenessProbe and readinessProbe

What do you think ?

ghost commented 1 year ago

Hi, Thanks for your PR.

You are right using /metrics is a bad idea because you will overload the api calls. I agree to have for the moment tcpSocket on port 8080 so that probes only check that osc-cost server doesn't crash. In the future, we can consider having a specific call to check that osc-cost exporter is healthy.

PS I have created a PR to prepare the release (helm, docker, cargo version) for your PR (https://github.com/outscale/osc-cost/pull/108)

albundy83 commented 1 year ago

Well, for the tcpSocket, as I told you, I have tried it and it seems that it also trigger the api calls :( ... That's why I was thinking about removing for moment the *probes (Your PR is perfect :) )

ghost commented 1 year ago

Ok You can disable livenessProbe and readinessProbe in this PR.

albundy83 commented 1 year ago

Ok done :)

ghost commented 1 year ago

Ok good :)