Skip to content

Commit 971d77b

Browse files
committed
fix: restore HTTP-based health server, make it opt-in
Revert the direct-upstream health probe introduced in #5, which opened a second TCP connection to the Modbus device on every check. Many Modbus devices only accept one concurrent connection, so the probe interfered with normal proxy operation in production. Restore the original passive HTTP health server that checks internal connection state without touching upstream. Make it opt-in via HEALTH_LISTEN so bare-binary users running multiple instances (the root cause of #4) no longer hit port conflicts. The Dockerfile sets HEALTH_LISTEN=:8080 so Docker users get health checks automatically. Closes #4
1 parent 9b12f3f commit 971d77b

File tree

5 files changed

+59
-83
lines changed

5 files changed

+59
-83
lines changed

Dockerfile

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@ FROM scratch
3131

3232
COPY --from=builder /app/mbproxy /mbproxy
3333

34-
HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \
34+
ENV HEALTH_LISTEN=:8080
35+
EXPOSE 8080
36+
37+
HEALTHCHECK --interval=5s --timeout=3s --start-period=10s --retries=3 \
3538
CMD ["/mbproxy", "-health"]
3639

3740
ENTRYPOINT ["/mbproxy"]

cmd/mbproxy/main.go

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ import (
88
"os"
99
"os/signal"
1010
"syscall"
11+
"time"
1112

1213
"github.com/tma/mbproxy/internal/config"
14+
"github.com/tma/mbproxy/internal/health"
1315
"github.com/tma/mbproxy/internal/logging"
14-
"github.com/tma/mbproxy/internal/modbus"
1516
"github.com/tma/mbproxy/internal/proxy"
1617
)
1718

@@ -20,7 +21,12 @@ func main() {
2021
flag.Parse()
2122

2223
if *healthCheck {
23-
if err := runHealthCheck(); err != nil {
24+
addr := os.Getenv("HEALTH_LISTEN")
25+
if addr == "" {
26+
fmt.Fprintln(os.Stderr, "HEALTH_LISTEN is not set")
27+
os.Exit(1)
28+
}
29+
if err := health.CheckHealth(addr); err != nil {
2430
fmt.Fprintln(os.Stderr, err)
2531
os.Exit(1)
2632
}
@@ -48,6 +54,22 @@ func main() {
4854
os.Exit(1)
4955
}
5056

57+
// Start health server if configured
58+
var hs *health.Server
59+
if cfg.HealthListen != "" {
60+
hs = health.NewServer(cfg.HealthListen, p, logger)
61+
hsLn, err := hs.Listen()
62+
if err != nil {
63+
logger.Error("failed to start health server", "error", err)
64+
os.Exit(1)
65+
}
66+
go func() {
67+
if err := hs.Serve(hsLn); err != nil {
68+
logger.Error("health server error", "error", err)
69+
}
70+
}()
71+
}
72+
5173
// Start proxy in background
5274
errCh := make(chan error, 1)
5375
go func() {
@@ -68,31 +90,18 @@ func main() {
6890
// Graceful shutdown
6991
cancel()
7092

93+
if hs != nil {
94+
shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second)
95+
defer shutdownCancel()
96+
if err := hs.Shutdown(shutdownCtx); err != nil {
97+
logger.Error("health server shutdown error", "error", err)
98+
}
99+
}
100+
71101
if err := p.Shutdown(cfg.ShutdownTimeout); err != nil {
72102
logger.Error("shutdown error", "error", err)
73103
os.Exit(1)
74104
}
75105

76106
logger.Info("shutdown complete")
77107
}
78-
79-
func runHealthCheck() error {
80-
cfg, err := config.Load()
81-
if err != nil {
82-
return err
83-
}
84-
85-
return checkUpstreamHealth(cfg, logging.New(cfg.LogLevel))
86-
}
87-
88-
func checkUpstreamHealth(cfg *config.Config, logger *slog.Logger) (err error) {
89-
client := modbus.NewClient(cfg.Upstream, cfg.Timeout, cfg.RequestDelay, cfg.ConnectDelay, logger)
90-
defer func() {
91-
closeErr := client.Close()
92-
if err == nil && closeErr != nil {
93-
err = closeErr
94-
}
95-
}()
96-
97-
return client.Connect()
98-
}

cmd/mbproxy/main_test.go

Lines changed: 0 additions & 59 deletions
This file was deleted.

internal/config/config.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type Config struct {
3030
RequestDelay time.Duration
3131
ConnectDelay time.Duration
3232
ShutdownTimeout time.Duration
33+
HealthListen string
3334
LogLevel string
3435
}
3536

@@ -46,6 +47,7 @@ func Load() (*Config, error) {
4647
RequestDelay: 0,
4748
ConnectDelay: 0,
4849
ShutdownTimeout: 30 * time.Second,
50+
HealthListen: os.Getenv("HEALTH_LISTEN"),
4951
LogLevel: GetEnv("LOG_LEVEL", "INFO"),
5052
}
5153

internal/config/config_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ func TestLoad_Defaults(t *testing.T) {
1919
os.Unsetenv("MODBUS_READONLY")
2020
os.Unsetenv("MODBUS_TIMEOUT")
2121
os.Unsetenv("MODBUS_SHUTDOWN_TIMEOUT")
22+
os.Unsetenv("HEALTH_LISTEN")
2223
os.Unsetenv("LOG_LEVEL")
2324

2425
cfg, err := Load()
@@ -56,6 +57,9 @@ func TestLoad_Defaults(t *testing.T) {
5657
if cfg.ShutdownTimeout != 30*time.Second {
5758
t.Errorf("expected 30s shutdown timeout, got %v", cfg.ShutdownTimeout)
5859
}
60+
if cfg.HealthListen != "" {
61+
t.Errorf("expected empty health listen, got %s", cfg.HealthListen)
62+
}
5963
if cfg.LogLevel != "INFO" {
6064
t.Errorf("expected INFO log level, got %s", cfg.LogLevel)
6165
}
@@ -192,3 +196,20 @@ func TestLoad_InvalidDuration(t *testing.T) {
192196
os.Unsetenv(envVar)
193197
}
194198
}
199+
200+
func TestLoad_HealthListenCustom(t *testing.T) {
201+
os.Setenv("MODBUS_UPSTREAM", "localhost:502")
202+
os.Setenv("HEALTH_LISTEN", ":9090")
203+
defer func() {
204+
os.Unsetenv("MODBUS_UPSTREAM")
205+
os.Unsetenv("HEALTH_LISTEN")
206+
}()
207+
208+
cfg, err := Load()
209+
if err != nil {
210+
t.Fatalf("unexpected error: %v", err)
211+
}
212+
if cfg.HealthListen != ":9090" {
213+
t.Errorf("expected :9090, got %s", cfg.HealthListen)
214+
}
215+
}

0 commit comments

Comments
 (0)