Skip to content

Bind health-check listeners on binding_ip and v4v6#904

Merged
peanball merged 1 commit intocloudfoundry:masterfrom
s4heid:ipv6
May 7, 2026
Merged

Bind health-check listeners on binding_ip and v4v6#904
peanball merged 1 commit intocloudfoundry:masterfrom
s4heid:ipv6

Conversation

@s4heid
Copy link
Copy Markdown
Contributor

@s4heid s4heid commented Apr 23, 2026

The four health-check listeners (health_check_http_url, its proxy protocol variant, and the per-tcp-proxy health_check_http_tcp-<name> pair) previously bound on :<port>, which is IPv4-wildcard only. On IPv6-only deployments (binding_ip: "::") the listeners were unreachable, causing monit host checks against localhost (IPv4) to fail and restart HAProxy.

Bind them on ha_proxy.binding_ip and append v4v6 like the other frontends so dual-stack works when ha_proxy.v4v6 is enabled.

Behavior change: when binding_ip is set to a specific IPv4 address, the health-check listeners now bind only on that address instead of all IPv4 interfaces. Operators probing health via a different NIC must update their probe targets accordingly.

peanball
peanball previously approved these changes May 4, 2026
Copy link
Copy Markdown
Contributor

@peanball peanball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Extends the logic for v4v6 to the health check endpoints, where it was so far missing.

Approving for CI.

<%- if enable_additional_health_check_proxy -%>
listen health_check_http_tcp-<%= tcp_proxy["name"] %>_proxy_protocol
bind :<%= tcp_proxy["health_check_http"] + 1 %> accept-proxy
bind <%= p("ha_proxy.binding_ip") %>:<%= tcp_proxy["health_check_http"] + 1 %> accept-proxy <%= v4v6 %>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v4v6 somewhat negates the binding IP I think. It's only used with the default address (i.e. when it's not explicitly given). Binding to 1.2.3.4:8080 v4v6 will not magically bind an IPv6 socket if I understand the HAProxy docs correctly.

Can you test whether those settings might be mutually exclusive on a real setup, or rather if they have the effect you desire?

Nevermind, the v4v6 flag is only set if the binding ip is :: (i.e. IPv6 default address) and the v4v6 config property is set:

# IPv4 and IPv6 binding (v4v6) Option {{{
v4v6 = ""
if_p("ha_proxy.v4v6") do
if p("ha_proxy.binding_ip") == "::"
v4v6 = "v4v6"
end
end
# }}}

This is just an extension of what we have in other locations that was just never applied to health checks.

@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Application Runtime Platform Working Group May 4, 2026
@peanball peanball added the run-ci Allow this PR to be tested on Concourse label May 4, 2026
Copy link
Copy Markdown
Contributor

@b1tamara b1tamara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving to run CI

The four health-check listeners (`health_check_http_url`, its PROXY protocol
variant, and the per-tcp-proxy `health_check_http_tcp-<name>` pair) previously
bound on `:<port>`, which is IPv4-wildcard only. On IPv6-only deployments
(`binding_ip: "::"`) the listeners were unreachable, causing monit host checks
against `localhost` (IPv4) to fail and restart HAProxy.

Bind them on `ha_proxy.binding_ip` and append `v4v6` like the other frontends
so dual-stack works when `ha_proxy.v4v6` is enabled.

Behavior change: when `binding_ip` is set to a specific IPv4 address, the
health-check listeners now bind only on that address instead of all IPv4
interfaces. Operators probing health via a different NIC must update their
probe targets accordingly.

Co-authored-by: jakobve <[email protected]>
Copy link
Copy Markdown
Contributor

@peanball peanball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving for CI, hopefully one final time :)

@peanball peanball merged commit 969e706 into cloudfoundry:master May 7, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Application Runtime Platform Working Group May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci Allow this PR to be tested on Concourse

Projects

Development

Successfully merging this pull request may close these issues.

3 participants