Skip to content

OCPBUGS-67161: Replace HTTP backend liveness check with admin socket check #737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 2 commits into from
Mar 10, 2026
+36 −6
Merged

OCPBUGS-67161: Replace HTTP backend liveness check with admin socket check #737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5beca77
OCPBUGS-67161: Replace HTTP backend liveness check with admin socket …
alebedev87 Feb 24, 2026
4b76ac6
OCPBUGS-67161: Define admin socket URL in a single place
alebedev87 Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions pkg/cmd/infra/router/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {

var reloadCallbacks []func()

adminSocketURL := &url.URL{Scheme: "unix", Path: "/var/lib/haproxy/run/haproxy.sock"}
statsPort := o.StatsPort
switch {
case o.MetricsType == "haproxy" && statsPort != 0:
Expand Down Expand Up @@ -607,7 +608,7 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {

collector, err := haproxy.NewPrometheusCollector(haproxy.PrometheusOptions{
// Only template router customizers who alter the image should need this
ScrapeURI: env("ROUTER_METRICS_HAPROXY_SCRAPE_URI", ""),
ScrapeURI: env("ROUTER_METRICS_HAPROXY_SCRAPE_URI", adminSocketURL.String()),
// Only template router customizers who alter the image should need this
PidFile: env("ROUTER_METRICS_HAPROXY_PID_FILE", ""),
Timeout: timeout,
Expand Down Expand Up @@ -636,9 +637,10 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
return err
}
checkController := metrics.ControllerLive()
checkSocket := metrics.AdminSocketAvailable(adminSocketURL)
liveChecks := []healthz.HealthChecker{checkController}
if !(isTrue(env("ROUTER_BIND_PORTS_BEFORE_SYNC", ""))) {
liveChecks = append(liveChecks, checkBackend)
liveChecks = append(liveChecks, checkSocket)
}

kubeconfig, _, err := o.Config.KubeConfig()
Expand Down Expand Up @@ -735,7 +737,7 @@ func (o *TemplateRouterOptions) Run(stopCh <-chan struct{}) error {
return err
}
cmopts := templateplugin.ConfigManagerOptions{
ConnectionInfo: "unix:///var/lib/haproxy/run/haproxy.sock",
ConnectionInfo: adminSocketURL.String(),
CommitInterval: o.CommitInterval,
BlueprintRoutes: blueprintRoutes,
BlueprintRoutePoolSize: o.BlueprintRoutePoolSize,
Expand Down
3 changes: 0 additions & 3 deletions pkg/router/metrics/haproxy/haproxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -732,9 +732,6 @@ func NewPrometheusCollector(opts PrometheusOptions) (*Exporter, error) {
}

func defaultOptions(opts PrometheusOptions) PrometheusOptions {
if len(opts.ScrapeURI) == 0 {
opts.ScrapeURI = "unix:///var/lib/haproxy/run/haproxy.sock"
}
if len(opts.PidFile) == 0 {
opts.PidFile = "/var/lib/haproxy/run/haproxy.pid"
}
Expand Down
31 changes: 31 additions & 0 deletions pkg/router/metrics/health.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,34 @@ func ProxyProtocolHTTPBackendAvailable(u *url.URL) healthz.HealthChecker {
return nil
})
}

// AdminSocketAvailable returns a healthz check that verifies the
// HAProxy process is alive by sending "show version" to its admin socket and
// expecting a non-empty response.
func AdminSocketAvailable(u *url.URL) healthz.HealthChecker {
return healthz.NamedCheck("admin-socket", func(_ *http.Request) error {
conn, err := net.DialTimeout("unix", u.Path, 2*time.Second)

@jcmoraisjr jcmoraisjr Feb 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Raw socket works nice, but router already imports a haproxy client that should make the code smaller and easier to understand, how does it sound?

		client := &haproxy.HAProxyClient{ // github.com/bcicen/go-haproxy
			Addr:    "unix:///var/lib/haproxy/run/haproxy.sock",
			Timeout: 2,
		}
		out, err := client.RunCommand("show info")

@jcmoraisjr jcmoraisjr Feb 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems to be the third place configuring the admin socket, maybe there's a place to configure it just once and everyone else can reuse instead?

@alebedev87 alebedev87 Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Raw socket works nice, but router already imports a haproxy client that should make the code smaller and easier to understand, how does it sound?

Right, I was considering this client. I wasn't too sure about a need to read the whole output of show info command as it takes a little more CPU cycles and uses a little more memory. I wanted to keep the probe as "bare bones" as possible - raw socket with a few bytes read just enough to understand that the process is alive.

This seems to be the third place configuring the admin socket, maybe there's a place to configure it just once and everyone else can reuse instead?

True, let me see how I can unify this.

@jcmoraisjr jcmoraisjr Mar 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see your point. From one side our client just reads 10 bytes for a fraction of a time. On the other one I see the haproxy side having most of the processing and memory consumption, since it needs to generate and store the whole output before streaming it to our client. Maybe you want to consider instead show version as a way of having zero cpu/mem consumption on both haproxy and router side.

@alebedev87 alebedev87 Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

let me see how I can unify this.

Addressed in 48fb1bb.

@alebedev87 alebedev87 Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On the other one I see the haproxy side having most of the processing and memory consumption, since it needs to generate and store the whole output before streaming it to our client.

True, this will add up to the probe round trip.

Maybe you want to consider instead show version as a way of having zero cpu/mem consumption on both haproxy and router side.

Yes, show version is something which was proposed by Miciah too. Looking at show version output, I agree it looks less heavy as it mostly has some constants. Ok, let me change the command.

@alebedev87 alebedev87 Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, let me change the command.

Done in 5c31f66.

@jcmoraisjr jcmoraisjr Mar 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does it worth to also move to the client for a cleaner code?

@alebedev87 alebedev87 Mar 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does it worth to also move to the client for a cleaner code?

I think we can consider unifying admin socket calls for the whole codebase. Maybe after DCM goes GA and the usage of go-haproxy module will be battle tested. For the moment though, I would prefer to keep the probe implementations consistent and use the raw socket for this new one too.

if err != nil {
return err
}
defer conn.Close()

conn.SetDeadline(time.Now().Add(2 * time.Second))

if _, err := conn.Write([]byte("show version\n")); err != nil {
return err
}

buf := make([]byte, 10)
n, err := conn.Read(buf)
if err != nil {
return err
}
if n == 0 {
return fmt.Errorf("empty response from admin socket")
}

log.V(4).Info("probe succeeded", "url", u.String())
return nil
})
}