diff --git a/spinifex/handlers/elbv2/haproxy.go b/spinifex/handlers/elbv2/haproxy.go index 8e24fd5c8..f56321028 100644 --- a/spinifex/handlers/elbv2/haproxy.go +++ b/spinifex/handlers/elbv2/haproxy.go @@ -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 -} diff --git a/spinifex/handlers/elbv2/haproxy_certs_test.go b/spinifex/handlers/elbv2/haproxy_certs_test.go index f6575c600..8df8170d8 100644 --- a/spinifex/handlers/elbv2/haproxy_certs_test.go +++ b/spinifex/handlers/elbv2/haproxy_certs_test.go @@ -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]) diff --git a/spinifex/handlers/elbv2/haproxy_test.go b/spinifex/handlers/elbv2/haproxy_test.go index f52e68303..bfce81e86 100644 --- a/spinifex/handlers/elbv2/haproxy_test.go +++ b/spinifex/handlers/elbv2/haproxy_test.go @@ -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 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{ diff --git a/spinifex/handlers/elbv2/nginx.go b/spinifex/handlers/elbv2/nginx.go index 754772e28..46daab107 100644 --- a/spinifex/handlers/elbv2/nginx.go +++ b/spinifex/handlers/elbv2/nginx.go @@ -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 diff --git a/spinifex/handlers/elbv2/nginx_test.go b/spinifex/handlers/elbv2/nginx_test.go index ee4ec13bb..5b2976f3d 100644 --- a/spinifex/handlers/elbv2/nginx_test.go +++ b/spinifex/handlers/elbv2/nginx_test.go @@ -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+";") diff --git a/spinifex/lbagent/agent.go b/spinifex/lbagent/agent.go index 648004f43..3e7ad797c 100644 --- a/spinifex/lbagent/agent.go +++ b/spinifex/lbagent/agent.go @@ -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, diff --git a/spinifex/lbagent/agent_test.go b/spinifex/lbagent/agent_test.go index 38125975c..43e4f9417 100644 --- a/spinifex/lbagent/agent_test.go +++ b/spinifex/lbagent/agent_test.go @@ -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) }