fix: strip duplicate lb- prefix from haproxy socket and cert paths (#331)

* fix: strip duplicate lb- prefix from haproxy socket and cert paths

LoadBalancerID, ListenerID and friends already carry their type prefix
(lb-/lst-), so the stats-socket, frontend cert, and HAProxyManager
config/pid filename templates double-prefixed: lb-lb-<id>.sock,
lb-lb-<id>-<lst>.pem, alb-lb-<id>.cfg.

The socket path is computed independently on both sides (gateway render
in buildHAProxyConfig, agent poll in lbagent.New); any divergence in the
lb- prefix convention between the two leaves the agent polling a socket
HAProxy never bound, spamming 'HAProxy stats unavailable: no such file'
while the data plane serves fine. Drop the redundant literal prefix so
both sides resolve to <id>.sock and the alb-/nginx cert paths match the
delivered config.

* refactor: remove unused HAProxyManager

HAProxyManager was never constructed (NewHAProxyManager had no callers).
The live ALB data plane runs in the lb-agent via reloadHAProxy with the
config generated daemon-side and shipped; this per-daemon process manager
was dead code. Remove the type, its methods, tests, and now-unused
imports (log/slog, os/exec, sync, syscall).
This commit is contained in:
Josh Siviour
2026-06-10 08:55:34 +10:00
committed by GitHub
parent 90369371ee
commit 686e374758
7 changed files with 9 additions and 385 deletions
+3 -225
View File
@@ -2,15 +2,11 @@ package handlers_elbv2
import (
"fmt"
"log/slog"
"os"
"os/exec"
"path/filepath"
"regexp"
"strconv"
"strings"
"sync"
"syscall"
"text/template"
"github.com/mulgadc/spinifex/spinifex/lbagent"
@@ -185,8 +181,8 @@ func generateALBConfig(lb *LoadBalancerRecord, listeners []*ListenerRecord, tgBy
// nginx data plane (generateNLBStreamConfig), not this builder.
func buildHAProxyConfig(lb *LoadBalancerRecord, listeners []*ListenerRecord, tgByArn map[string]*TargetGroupRecord, rulesByListener map[string][]*RuleRecord, bindAddr string, certPEMByArn map[string]string) (HAProxyConfig, error) {
socketDir := filepath.Join(os.TempDir(), "spinifex-haproxy")
// Use "lb" prefix for both ALB and NLB — must match the agent's socket path.
socketPath := filepath.Join(socketDir, fmt.Sprintf("lb-%s.sock", lb.LoadBalancerID))
// LoadBalancerID already carries the "lb-" prefix; must match the agent's socket path.
socketPath := filepath.Join(socketDir, fmt.Sprintf("%s.sock", lb.LoadBalancerID))
cfg := HAProxyConfig{
SocketPath: socketPath,
@@ -328,7 +324,7 @@ func buildHAProxyConfig(lb *LoadBalancerRecord, listeners []*ListenerRecord, tgB
// PEM file under the agent's cert dir. Daemon-rendered and agent-written paths
// must match exactly.
func frontendCertPath(lb *LoadBalancerRecord, l *ListenerRecord) string {
return filepath.Join(lbagent.CertDir, fmt.Sprintf("lb-%s-%s.pem", lb.LoadBalancerID, l.ListenerID))
return filepath.Join(lbagent.CertDir, fmt.Sprintf("%s-%s.pem", lb.LoadBalancerID, l.ListenerID))
}
// resolveFrontendCert picks a listener's default certificate, resolves its PEM
@@ -697,221 +693,3 @@ func sanitizeName(prefix, id string) string {
}
return prefix + "_" + sb.String()
}
// HAProxyManager manages HAProxy processes for ALBs on this daemon node.
type HAProxyManager struct {
configDir string // Directory for HAProxy config files
binPath string // Path to haproxy binary
mu sync.Mutex
pids map[string]int // lbID → HAProxy PID
}
// NewHAProxyManager creates a new HAProxy manager.
func NewHAProxyManager(configDir string) *HAProxyManager {
binPath := "/usr/sbin/haproxy"
if p, err := exec.LookPath("haproxy"); err == nil {
binPath = p
}
return &HAProxyManager{
configDir: configDir,
binPath: binPath,
pids: make(map[string]int),
}
}
// Available returns true if the haproxy binary is found on the system.
func (m *HAProxyManager) Available() bool {
_, err := exec.LookPath(m.binPath)
if err != nil {
// binPath might be absolute — check directly
_, err = os.Stat(m.binPath)
}
return err == nil
}
// pidFilePath returns the pidfile path for a given LB.
func (m *HAProxyManager) pidFilePath(lbID string) string {
return filepath.Join(m.configDir, fmt.Sprintf("alb-%s.pid", lbID))
}
// configFilePath returns the config path for a given LB.
func (m *HAProxyManager) configFilePath(lbID string) string {
return filepath.Join(m.configDir, fmt.Sprintf("alb-%s.cfg", lbID))
}
// IsRunning returns true if an HAProxy process is tracked and alive for this LB.
func (m *HAProxyManager) IsRunning(lbID string) bool {
m.mu.Lock()
pid, ok := m.pids[lbID]
m.mu.Unlock()
if !ok || pid <= 0 {
return false
}
// Check if process is alive
proc, err := os.FindProcess(pid)
if err != nil {
return false
}
return proc.Signal(syscall.Signal(0)) == nil
}
// Start launches a new HAProxy process for the given LB.
func (m *HAProxyManager) Start(lbID string) error {
m.mu.Lock()
defer m.mu.Unlock()
return m.startLocked(lbID)
}
// startLocked is the lock-free core of Start. Caller must hold m.mu.
func (m *HAProxyManager) startLocked(lbID string) error {
if err := os.MkdirAll(m.configDir, 0o750); err != nil {
return fmt.Errorf("create config dir: %w", err)
}
// Ensure the stats socket directory exists
socketDir := filepath.Join(os.TempDir(), "spinifex-haproxy")
if err := os.MkdirAll(socketDir, 0o750); err != nil {
return fmt.Errorf("create socket dir: %w", err)
}
configPath := m.configFilePath(lbID)
pidFile := m.pidFilePath(lbID)
cmd := exec.Command(m.binPath, "-f", configPath, "-p", pidFile, "-D")
output, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("haproxy start failed: %s: %w", strings.TrimSpace(string(output)), err)
}
// Read PID from pidfile
pid, err := m.readPidFile(pidFile)
if err != nil {
slog.Warn("HAProxy started but could not read pidfile", "lbId", lbID, "err", err)
return nil
}
m.pids[lbID] = pid
slog.Info("HAProxy started", "lbId", lbID, "pid", pid)
return nil
}
// Reload performs a graceful reload of the HAProxy process for the given LB.
// The old workers finish in-flight connections while new workers start.
func (m *HAProxyManager) Reload(lbID string) error {
m.mu.Lock()
defer m.mu.Unlock()
oldPid, ok := m.pids[lbID]
if !ok || oldPid <= 0 {
// Not running — do a fresh start instead
return m.startLocked(lbID)
}
configPath := m.configFilePath(lbID)
pidFile := m.pidFilePath(lbID)
cmd := exec.Command(m.binPath, "-f", configPath, "-p", pidFile, "-D", "-sf", strconv.Itoa(oldPid))
output, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("haproxy reload failed: %s: %w", strings.TrimSpace(string(output)), err)
}
// Read new PID
newPid, err := m.readPidFile(pidFile)
if err != nil {
slog.Warn("HAProxy reloaded but could not read new pidfile", "lbId", lbID, "err", err)
return nil
}
m.pids[lbID] = newPid
slog.Info("HAProxy reloaded", "lbId", lbID, "oldPid", oldPid, "newPid", newPid)
return nil
}
// Stop terminates the HAProxy process for the given LB and cleans up files.
func (m *HAProxyManager) Stop(lbID string) error {
m.mu.Lock()
defer m.mu.Unlock()
pid, ok := m.pids[lbID]
if ok && pid > 0 {
proc, err := os.FindProcess(pid)
if err == nil {
if err := proc.Signal(syscall.SIGTERM); err != nil {
slog.Debug("HAProxy SIGTERM failed (may already be stopped)", "lbId", lbID, "pid", pid, "err", err)
}
}
}
delete(m.pids, lbID)
// Clean up pidfile
pidFile := m.pidFilePath(lbID)
os.Remove(pidFile)
slog.Info("HAProxy stopped", "lbId", lbID, "pid", pid)
return nil
}
// StopAll terminates all tracked HAProxy processes. Called on daemon shutdown.
func (m *HAProxyManager) StopAll() {
m.mu.Lock()
lbIDs := make([]string, 0, len(m.pids))
for id := range m.pids {
lbIDs = append(lbIDs, id)
}
m.mu.Unlock()
for _, id := range lbIDs {
if err := m.Stop(id); err != nil {
slog.Warn("Failed to stop HAProxy on shutdown", "lbId", id, "err", err)
}
}
}
// readPidFile reads and parses a PID from a pidfile.
func (m *HAProxyManager) readPidFile(path string) (int, error) {
data, err := os.ReadFile(path)
if err != nil {
return 0, err
}
pid, err := strconv.Atoi(strings.TrimSpace(string(data)))
if err != nil {
return 0, fmt.Errorf("parse pid %q: %w", string(data), err)
}
return pid, nil
}
// WriteConfig writes an HAProxy config file for the given load balancer.
func (m *HAProxyManager) WriteConfig(lbID, configContent string) (string, error) {
if err := os.MkdirAll(m.configDir, 0o750); err != nil {
return "", fmt.Errorf("create config dir: %w", err)
}
path := filepath.Join(m.configDir, fmt.Sprintf("alb-%s.cfg", lbID))
if err := os.WriteFile(path, []byte(configContent), 0o600); err != nil {
return "", fmt.Errorf("write haproxy config: %w", err)
}
slog.Info("Wrote HAProxy config", "path", path, "lbId", lbID)
return path, nil
}
// RemoveConfig removes the HAProxy config file for the given load balancer.
func (m *HAProxyManager) RemoveConfig(lbID string) error {
path := filepath.Join(m.configDir, fmt.Sprintf("alb-%s.cfg", lbID))
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("remove haproxy config: %w", err)
}
return nil
}
// ValidateConfig checks if an HAProxy config file is syntactically valid.
func (m *HAProxyManager) ValidateConfig(configPath string) error {
cmd := exec.Command(m.binPath, "-c", "-f", configPath)
output, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("haproxy config validation failed: %s: %w", string(output), err)
}
return nil
}
@@ -40,7 +40,7 @@ func TestGenerateHAProxyConfigWithCerts_HTTPSRendersSslCrt(t *testing.T) {
)
require.NoError(t, err)
wantPath := filepath.Join(lbagent.CertDir, "lb-lb-abc123-lst-https.pem")
wantPath := filepath.Join(lbagent.CertDir, "lb-abc123-lst-https.pem")
assert.Contains(t, config, "bind *:443 ssl crt "+wantPath)
require.Len(t, certFiles, 1)
assert.Equal(t, certPEM, certFiles[wantPath])
+1 -155
View File
@@ -1,8 +1,6 @@
package handlers_elbv2
import (
"os"
"path/filepath"
"strings"
"testing"
@@ -51,7 +49,7 @@ func TestGenerateHAProxyConfig_SingleListenerAndBackend(t *testing.T) {
// Verify global section
assert.Contains(t, config, "stats socket")
assert.Contains(t, config, "lb-lb-abc123.sock")
assert.Contains(t, config, "lb-abc123.sock")
// Verify frontend
assert.Contains(t, config, "bind *:80")
@@ -433,158 +431,6 @@ func TestSanitizeName(t *testing.T) {
}
}
func TestHAProxyManager_Available_NotFound(t *testing.T) {
mgr := NewHAProxyManager(t.TempDir())
mgr.binPath = "/nonexistent/haproxy-fake-bin"
assert.False(t, mgr.Available())
}
func TestHAProxyManager_IsRunning_UnknownLB(t *testing.T) {
mgr := NewHAProxyManager(t.TempDir())
assert.False(t, mgr.IsRunning("lb-unknown"))
}
func TestHAProxyManager_Stop_NotRunning(t *testing.T) {
mgr := NewHAProxyManager(t.TempDir())
err := mgr.Stop("lb-not-running")
require.NoError(t, err)
}
func TestHAProxyManager_StopAll_Empty(t *testing.T) {
mgr := NewHAProxyManager(t.TempDir())
mgr.StopAll() // should not panic
}
func TestHAProxyManager_PidFilePath(t *testing.T) {
mgr := NewHAProxyManager("/tmp/haproxy-test")
assert.Equal(t, "/tmp/haproxy-test/alb-lb-123.pid", mgr.pidFilePath("lb-123"))
assert.Equal(t, "/tmp/haproxy-test/alb-lb-123.cfg", mgr.configFilePath("lb-123"))
}
func TestHAProxyManager_Start_MissingBinary(t *testing.T) {
dir := t.TempDir()
mgr := NewHAProxyManager(dir)
mgr.binPath = "/nonexistent/haproxy"
// Write a config so the path exists
_, err := mgr.WriteConfig("lb-test", "global\n")
require.NoError(t, err)
err = mgr.Start("lb-test")
require.Error(t, err)
assert.Contains(t, err.Error(), "haproxy start failed")
}
func TestHAProxyManager_Reload_NotRunning_FallsBackToStart(t *testing.T) {
dir := t.TempDir()
mgr := NewHAProxyManager(dir)
mgr.binPath = "/nonexistent/haproxy"
// Write a config so the path exists
_, err := mgr.WriteConfig("lb-test", "global\n")
require.NoError(t, err)
// Reload when not running should attempt Start (which will fail due to bad binary)
err = mgr.Reload("lb-test")
require.Error(t, err)
assert.Contains(t, err.Error(), "haproxy start failed")
}
func TestHAProxyManager_LifecycleWithFakeHAProxy(t *testing.T) {
// Create a fake haproxy script that writes its PID to the pidfile
dir := t.TempDir()
fakeHAProxy := filepath.Join(dir, "fake-haproxy")
script := `#!/bin/sh
# Parse -p flag for pidfile
PIDFILE=""
SF_PID=""
while [ $# -gt 0 ]; do
case "$1" in
-p) PIDFILE="$2"; shift 2;;
-D) shift;;
-f) shift 2;;
-c) exit 0;;
-sf) SF_PID="$2"; shift 2;;
*) shift;;
esac
done
# Kill old process if -sf was passed
if [ -n "$SF_PID" ]; then
kill "$SF_PID" 2>/dev/null || true
fi
# Start a background sleep with closed fds so parent can exit
sleep 60 </dev/null >/dev/null 2>&1 &
BGPID=$!
if [ -n "$PIDFILE" ]; then
echo "$BGPID" > "$PIDFILE"
fi
`
require.NoError(t, os.WriteFile(fakeHAProxy, []byte(script), 0o755))
configDir := filepath.Join(dir, "configs")
mgr := NewHAProxyManager(configDir)
mgr.binPath = fakeHAProxy
lbID := "lb-lifecycle-test"
// Write config
content := "global\n log stdout\n"
_, err := mgr.WriteConfig(lbID, content)
require.NoError(t, err)
// Not running yet
assert.False(t, mgr.IsRunning(lbID))
// Start
err = mgr.Start(lbID)
require.NoError(t, err)
assert.True(t, mgr.IsRunning(lbID))
// Reload
err = mgr.Reload(lbID)
require.NoError(t, err)
assert.True(t, mgr.IsRunning(lbID))
// Stop
err = mgr.Stop(lbID)
require.NoError(t, err)
assert.False(t, mgr.IsRunning(lbID))
// Pidfile should be cleaned up
_, err = os.Stat(mgr.pidFilePath(lbID))
assert.True(t, os.IsNotExist(err))
}
func TestHAProxyManager_WriteAndRemoveConfig(t *testing.T) {
dir := t.TempDir()
mgr := &HAProxyManager{configDir: dir, binPath: "/usr/sbin/haproxy"}
content := "global\n log stdout\n"
path, err := mgr.WriteConfig("lb-test", content)
require.NoError(t, err)
assert.Equal(t, filepath.Join(dir, "alb-lb-test.cfg"), path)
// Verify file contents
data, err := os.ReadFile(path)
require.NoError(t, err)
assert.Equal(t, content, string(data))
// Remove
err = mgr.RemoveConfig("lb-test")
require.NoError(t, err)
_, err = os.Stat(path)
assert.True(t, os.IsNotExist(err))
}
func TestHAProxyManager_RemoveConfig_Idempotent(t *testing.T) {
dir := t.TempDir()
mgr := &HAProxyManager{configDir: dir, binPath: "/usr/sbin/haproxy"}
err := mgr.RemoveConfig("nonexistent")
require.NoError(t, err)
}
func TestGenerateALBConfig_StillWorks(t *testing.T) {
// Regression: ALB (no Type set) should still generate HTTP-mode config
lb := &LoadBalancerRecord{
+1 -1
View File
@@ -184,7 +184,7 @@ func generateNLBStreamConfig(lb *LoadBalancerRecord, listeners []*ListenerRecord
// nginxCertPath is the stable absolute PEM path for a TLS listener under the
// agent's nginx cert dir. Daemon-rendered and agent-written paths must match.
func nginxCertPath(lb *LoadBalancerRecord, l *ListenerRecord) string {
return filepath.Join(lbagent.NginxCertDir, fmt.Sprintf("lb-%s-%s.pem", lb.LoadBalancerID, l.ListenerID))
return filepath.Join(lbagent.NginxCertDir, fmt.Sprintf("%s-%s.pem", lb.LoadBalancerID, l.ListenerID))
}
// buildNLBHealthTargets computes the active health-check targets for an NLB's
+1 -1
View File
@@ -94,7 +94,7 @@ func TestGenerateNLBStream_TLS(t *testing.T) {
config, certs, err := GenerateHAProxyConfigWithCerts(lb, []*ListenerRecord{lst}, tgByArn, nil, "10.0.3.1", certPEM)
require.NoError(t, err)
wantPath := "/etc/nginx/certs/lb-lb-tls-lst-tls.pem"
wantPath := "/etc/nginx/certs/lb-tls-lst-tls.pem"
assert.Contains(t, config, "listen 443 ssl;")
assert.Contains(t, config, "ssl_certificate "+wantPath+";")
assert.Contains(t, config, "ssl_certificate_key "+wantPath+";")
+1 -1
View File
@@ -121,7 +121,7 @@ func New(lbID, gatewayURL, accessKey, secretKey, region string) (*Agent, error)
configPath: DefaultConfigPath,
pidPath: DefaultPIDPath,
certDir: CertDir,
socketPath: fmt.Sprintf("/tmp/spinifex-haproxy/lb-%s.sock", lbID),
socketPath: fmt.Sprintf("/tmp/spinifex-haproxy/%s.sock", lbID),
accessKey: accessKey,
secretKey: secretKey,
client: client,
+1 -1
View File
@@ -68,7 +68,7 @@ func TestNew_SocketPath(t *testing.T) {
t.Fatalf("New: %v", err)
}
expected := "/tmp/spinifex-haproxy/lb-lb-sock123.sock"
expected := "/tmp/spinifex-haproxy/lb-sock123.sock"
if agent.socketPath != expected {
t.Errorf("socketPath = %q, want %q", agent.socketPath, expected)
}