fix: support configuration of HTTP server address #1365
Conversation
|
This probably should be changed such that Prometheus's endpoint is available on all interfaces, but the remaining endpoints are localhost only. |
With this commit, callers can bind the HTTP server for Prometheus metrics separately from the HTTP server that provides health checks.
cmd/root_test.go
Outdated
| c.SetArgs([]string{"--prometheus", "my-project:my-region:my-instance"}) | ||
| c.SetArgs([]string{ | ||
| "--prometheus", | ||
| "--prometheus-address", "localhost", |
There was a problem hiding this comment.
You need a test where this isn't localhost. That's the default value
cmd/root.go
Outdated
| // Start the HTTP server if anything requiring HTTP is specified. | ||
| if needsHTTPServer { | ||
| server := &http.Server{ | ||
| srv := &http.Server{ | ||
| Addr: fmt.Sprintf("localhost:%s", cmd.httpPort), |
There was a problem hiding this comment.
Should we be putting 0.0.0.0 here?
cmd/root.go
Outdated
| @@ -78,6 +78,8 @@ type Command struct { | |||
| telemetryPrefix string | |||
| prometheus bool | |||
| prometheusNamespace string | |||
| prometheusAddress string | |||
| prometheusPort string | |||
There was a problem hiding this comment.
[minor] Since it’s prometheusPort, what about healthCheckPort instead of httpPort?
cmd/root.go
Outdated
| @@ -78,6 +78,8 @@ type Command struct { | |||
| telemetryPrefix string | |||
| prometheus bool | |||
| prometheusNamespace string | |||
| prometheusAddress string | |||
There was a problem hiding this comment.
We also need httpAddress, right? Otherwise, the probe will fail (am I missing something?).
This reverts commit 7361af4.
|
After going back and forth on this, I think the convention is to bind HTTP probes to 0.0.0.0 (both lo and eth0) in a kubernetes context. Since people might want to run this on a GCE VM, I'll leave the default to localhost, but otherwise support binding to both interfaces. See the docs where it says that the kublet defaults to the pod ID for HTTP probes, |
In Kubernetes, the convention is to bind HTTP probes and Prometheus endpoints to 0.0.0.0 (both lo and eth0). Since people might want to run this code on a GCE VM, default to localhost, but otherwise support binding to both interfaces. This is a port of GoogleCloudPlatform/cloud-sql-proxy#1365.
In Kubernetes, the convention is to bind HTTP probes and Prometheus endpoints to 0.0.0.0 (both lo and eth0). Since people might want to run this code on a GCE VM, default to localhost, but otherwise support binding to both interfaces. This is a port of GoogleCloudPlatform/cloud-sql-proxy#1365.
In Kubernetes, the convention is to bind HTTP probes and Prometheus endpoints to
0.0.0.0 (both lo and eth0). Since people might want to run this code on a GCE
VM, default to localhost, but otherwise support binding to both interfaces.
Fixes #1359.