fix: derive config-reloader port from http.listenAddr - #2585
Conversation
Parse ConfigReloaderExtraArgs["http.listenAddr"] into ContainerPort, probes, named scrape targets, and WaitForConfigReloadHash so hostNetwork DaemonSets can use distinct reloader ports without scheduler conflicts. Fixes VictoriaMetrics#2584
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
Please don't merge this yet - it looks unnecessarily complicated and relies on static container name, which may be undesirable |
Find the config-reloader HTTP port by its stable reloader-http name. Drop the container-name dependency and arbitrary first-port fallback.
| } | ||
|
|
||
| // withConfigReloaderMetricsResponse stubs the sidecar /metrics body in-process. httptest is | ||
| // unreliable here when loopback is broken, so the wait tests drive an injected Transport. |
There was a problem hiding this comment.
I'm not sure what "loopback is broken" means - could you provide an example? There are other places where we read metrics (i.e. vmdistributed) - do we need to make the same change there too?
I like the improvement in test UX, but the roundtrip change looks excessive
There was a problem hiding this comment.
You are right. The transport stub was local test-environment noise and does
not belong in this PR. Sorry for adding it.
I restored the existing httptest pattern and removed configReloadNewHTTPClient,
roundTripFunc, and the loopback comment. The remaining change only resolves
the configured reloader port from the PodSpec so WaitForConfigReloadHash uses
the same port as the container and probes.
Keep the existing httptest pattern and remove the local HTTP transport workaround from the config-reloader port change.
2 participants
Parse ConfigReloaderExtraArgs["http.listenAddr"] into ContainerPort, probes, named scrape targets, and WaitForConfigReloadHash so hostNetwork DaemonSets can use distinct reloader ports without scheduler conflicts.
Fixes #2584