GoogleCloudPlatform / prometheus-engine

Google Cloud Managed Service for Prometheus libraries and manifests.
https://g.co/cloud/managedprometheus
Apache License 2.0
196 stars 93 forks source link

feat: add config-reloader readiness #1075

Open TheSpiritXIII opened 4 months ago

TheSpiritXIII commented 4 months ago

This change adds a readiness endpoint to the config-reloader. This helps Kubernetes be more accurate in accessing that workloads are up.

The primary reason I added it is for https://github.com/GoogleCloudPlatform/prometheus-engine/pull/1076 -- I wanted the config-reloader to start running before Prometheus is running. However, the config-reloader must hit a reload URL. As a "hack", I made it hit an endpoint of itself that does nothing. Since this endpoint is functionally the same as a readiness endpoint, I figured we might as well add a few more missing pieces to turn this into a readiness endpoint that Kubernetes can hit. This also helps make the PRs smaller and hopefully easier to review.

pintohutch commented 3 months ago

Can you fill out the description and describe what this change is doing and the motivation for it?

TheSpiritXIII commented 3 months ago

@pintohutch I updated the description. Feel free to push back on this change!

pintohutch commented 3 months ago

@TheSpiritXIII - IIUC our discussion around supporting "empty" mandatory flag/config will make this change unnecessary. Is that your understanding as well?

TheSpiritXIII commented 3 months ago

@pintohutch yes this change becomes non-critical. This PR may still provide a slightly better signal over when a workload is "ready" than we have today, but again, let's not prioritize it unless we need to.

bwplotka commented 2 months ago

LGTM, but we need to resolve the conflicts. Thanks!