Skip to content

BUG/MINOR: configure: pick the first valid UNIX socket from HAPROXY_MASTER_CLI - #416

Open
fabiomatavelli wants to merge 1 commit into
haproxytech:masterfrom
fabiomatavelli:fix-haproxy-master-cli-parsing
Open

BUG/MINOR: configure: pick the first valid UNIX socket from HAPROXY_MASTER_CLI#416
fabiomatavelli wants to merge 1 commit into
haproxytech:masterfrom
fabiomatavelli:fix-haproxy-master-cli-parsing

Conversation

@fabiomatavelli

Copy link
Copy Markdown

Fixes #329. This picks up the work started in #330, which was closed by its author, and addresses the review feedback left there.

Problem

Since haproxy/haproxy@8a02257, HAProxy advertises its master CLI as a ;-separated list of addresses, so HAPROXY_MASTER_CLI now looks like:

unix@/data/haproxy/run/master-socket;sockpair@7

configureAPI only stripped the unix@ prefix and passed the rest through, so the runtime client tried to dial a socket literally named /data/haproxy/run/master-socket;sockpair@7:

level=warning msg="Error setting up runtime client with master socket (1): /data/haproxy/run/master-socket;sockpair@7 : dial unix /data/haproxy/run/master-socket;sockpair@7: connect: no such file or directory"

When the Data Plane API runs from a program section this makes it exit, and HAProxy tears down the whole process tree through exit-on-failure.

Changes

misc.MasterSocketFromEnv(value string) (string, bool) parses the raw environment value:

  • splits on ; and keeps only the UNIX addresses, so any other address family HAProxy may add later is ignored instead of breaking the parsing
  • returns the first candidate that is already bound on the filesystem (os.Stat plus os.ModeSocket)
  • if none is bound yet, returns the first syntactically valid candidate anyway. runtime_options.AllowDelayedStart exists precisely to wait for a socket HAProxy has not bound yet, so a hard existence check here would regress those setups
  • returns false when there is no usable UNIX socket at all

misc.IsUnixSocketAddr now rejects the empty string, and accepts an <family>@ prefix only when the family is unix. Previously "", sockpair@7 and fd@3 all returned true.

configureAPI only overrides haproxyOptions.MasterRuntime when a socket was actually found, and logs a warning otherwise. This is what keeps a master runtime set through --master-runtime / haproxy.master_runtime from being silently replaced by an empty string.

Tests: table-driven unit tests for both functions in misc/misc_test.go, binding real UNIX sockets so the os.ModeSocket branch is genuinely exercised.

Review comments from #330

Comment Addressed by
@mjuraga: HAPROXY_MASTER_CLI is ;-separated and may hold several entries, split it and use the first valid unix@ one MasterSocketFromEnv splits and iterates, first bound socket wins
@georgijd-form3: IsUnixSocketAddr("") is true, so an unset variable wipes a user-supplied MasterRuntime IsUnixSocketAddr rejects "", and the caller only assigns when a socket was found
@mjuraga: IsUnixSocketAddr should validate the address properly, including that the socket exists Address family validation lives in IsUnixSocketAddr, the existence check lives in MasterSocketFromEnv where a filesystem lookup is meaningful and where a delayed start can still be honoured

Notes

  • abns@name now returns false from IsUnixSocketAddr. The runtime client cannot dial abstract-namespace sockets anyway, so this turns a confusing dial failure into an explicit skip.
  • Verified locally: go build ./..., go test ./..., make lint (0 issues) and make check-commit all pass.

…ASTER_CLI

Since HAProxy commit 8a02257d, HAPROXY_MASTER_CLI advertises the master
CLI as a list of addresses separated by ";", for example
"unix@/var/run/master.sock;sockpair@7". The whole value was handed over
to the runtime client, which failed to dial a socket path that does not
exist and made the Data Plane API exit. HAProxy reacts to that with
exit-on-failure and kills every process, which is the failure reported
in the issue.

Split the value, keep only the UNIX addresses and use the first one that
is already bound, falling back to the first valid candidate so that a
delayed runtime start still picks it up.

The UNIX socket check now rejects the empty string and every address
family other than "unix@". Together with the caller no longer overriding
the master runtime when no socket was found, a value set through
--master-runtime or haproxy.master_runtime is not silently replaced by
an empty string any more.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data Plane API is killed by SIGTERM in openshift

1 participant