-
Notifications
You must be signed in to change notification settings - Fork 112
cmd/wol-proxy: add wol-proxy #352
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.
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
Conversation
WalkthroughAdds a new wol-proxy command and README, Makefile cross-build support and target for wol-proxy, and registers a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WOLProxy as WOL Proxy
participant HealthChecker as Health Checker (bg)
participant Upstream as Upstream Server
HealthChecker->>Upstream: GET /wol-health (periodic)
Upstream-->>HealthChecker: 200 OK / no response
Client->>WOLProxy: HTTP request
WOLProxy->>WOLProxy: Check cached readiness
alt Upstream ready
WOLProxy->>Upstream: Proxy request
Upstream-->>WOLProxy: Response
WOLProxy-->>Client: Response
else Upstream not ready
rect rgb(220,240,255)
WOLProxy->>Upstream: Send WOL magic packet (to MAC)
WOLProxy->>WOLProxy: Start timeout & poll /wol-health
end
alt Ready before timeout
Upstream-->>WOLProxy: 200 OK
WOLProxy->>Upstream: Proxy request
Upstream-->>WOLProxy: Response
WOLProxy-->>Client: Response
else Timeout elapsed
rect rgb(255,230,230)
WOLProxy-->>Client: Timeout / dropped response
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (4)
proxy/proxymanager.go (1)
246-250
: Add HEAD handler and no-store to avoid proxy caches.Some health pollers use HEAD; adding it avoids 404s and extra bytes. Also mark responses non-cacheable.
pm.ginEngine.GET("/wol-health", func(c *gin.Context) { - c.String(http.StatusOK, "OK") + c.Header("Cache-Control", "no-store") + c.String(http.StatusOK, "OK") }) +pm.ginEngine.HEAD("/wol-health", func(c *gin.Context) { + c.Header("Cache-Control", "no-store") + c.String(http.StatusOK, "OK") +})cmd/wol-proxy/README.md (1)
5-7
: Clarify timeout behavior and upstream health dependency.Document that the proxy returns 408 on timeout and that upstream must expose /wol-health.
-If the server doesn't respond within the timeout period (default: 60 seconds), the request is dropped. +If the server doesn't respond within the timeout period (default: 60 seconds), the request returns HTTP 408 (Request Timeout). @@ -`GET /status` - that's it. Everything else is proxied to the upstream server. +`GET /status` - reports wol-proxy status and failure counts. Everything else is proxied to the upstream server. + +Note: The upstream llama-swap server must expose `GET /wol-health` (returns 200 OK) for readiness checks.Also applies to: 25-27
Makefile (1)
89-94
: Make Windows builds append .exe automatically and add trimpath.Improves cross-platform output naming and reproducibility.
GOOS ?= $(shell go env GOOS 2>/dev/null || echo linux) GOARCH ?= $(shell go env GOARCH 2>/dev/null || echo amd64) wol-proxy: $(BUILD_DIR) @echo "Building wol-proxy" - go build -o $(BUILD_DIR)/wol-proxy-$(GOOS)-$(GOARCH) cmd/wol-proxy/wol-proxy.go + $(eval EXE :=) + @if [ "$(GOOS)" = "windows" ]; then EXE=".exe"; fi; \ + go build -trimpath -o $(BUILD_DIR)/wol-proxy-$(GOOS)-$(GOARCH)$${EXE} cmd/wol-proxy/wol-proxy.gocmd/wol-proxy/wol-proxy.go (1)
156-179
: Optional: debounce WoL to prevent bursts on concurrent requests.Multiple simultaneous requests will all call sendMagicPacket. Consider a short debounce (e.g., only send once every N seconds) using an atomic timestamp or a mutex-guarded time.Time.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Makefile
(1 hunks)cmd/wol-proxy/README.md
(1 hunks)cmd/wol-proxy/wol-proxy.go
(1 hunks)proxy/proxymanager.go
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Fix all staticcheck-reported issues in Go code
Files:
cmd/wol-proxy/wol-proxy.go
proxy/proxymanager.go
🧬 Code graph analysis (1)
cmd/wol-proxy/wol-proxy.go (2)
proxy/logMonitor.go (4)
LevelDebug
(17-17)LevelInfo
(18-18)LevelWarn
(19-19)LevelError
(20-20)proxy/proxymanager.go (1)
New
(48-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run-tests
🔇 Additional comments (1)
proxy/proxymanager.go (1)
135-142
: Bypass for /wol-health logging is correct and early in the middleware chain.This cleanly skips the timing/log line and still lets the rest of the handlers run. LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (4)
cmd/wol-proxy/README.md (1)
19-19
: Fix typo in comment."altenerative" should be "alternative".
Apply this diff:
- # altenerative listening port + # alternative listening addresscmd/wol-proxy/wol-proxy.go (3)
86-91
: Don't log normal server shutdown as an error.
ListenAndServe
returnshttp.ErrServerClosed
whenShutdown()
is called, which is expected behavior and shouldn't be logged as an error.Apply this diff:
go func() { slog.Info("server starting on", "address", *flagListen) - if err := server.ListenAndServe(); err != nil { + if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { slog.Error("error starting server", "error", err) } }()
164-170
: Fix data race: read status and failCount under lock.The
/status
handler readsp.status
andp.failCount
without synchronization while the health check goroutine modifies them. This is a data race.Apply this diff:
if r.Method == "GET" && r.URL.Path == "/status" { + p.statusMutex.RLock() + st := p.status + failures := p.failCount + p.statusMutex.RUnlock() w.Header().Set("Content-Type", "text/plain; charset=utf-8") - w.WriteHeader(200) - fmt.Fprintf(w, "status: %s\n", string(p.status)) - fmt.Fprintf(w, "failures: %d\n", p.failCount) + w.WriteHeader(http.StatusOK) + fmt.Fprintf(w, "status: %s\n", string(st)) + fmt.Fprintf(w, "failures: %d\n", failures) return }
126-126
: Fix typo in comment."goroutien" should be "goroutine".
Apply this diff:
- // start a goroutien to check upstream status + // start a goroutine to check upstream status
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/wol-proxy/README.md
(1 hunks)cmd/wol-proxy/wol-proxy.go
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Fix all staticcheck-reported issues in Go code
Files:
cmd/wol-proxy/wol-proxy.go
🧬 Code graph analysis (1)
cmd/wol-proxy/wol-proxy.go (2)
proxy/logMonitor.go (4)
LevelDebug
(17-17)LevelInfo
(18-18)LevelWarn
(19-19)LevelError
(20-20)proxy/proxymanager.go (1)
New
(48-131)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run-tests
- GitHub Check: run-tests
Add a Wake-On-LAN proxy for llama-swap servers that suspend to save power.
Summary by CodeRabbit
New Features
Documentation
Chores