Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixed-crux-health-tmux.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Crux `GET /v1/health` now reports a `tmux` field (where tmux resolves on PATH) and answers 503, with the full health body, when tmux is missing (the body also carries `error`/`code`, so `api.Client` callers and the supervisor see "tmux not found on PATH" rather than a bare 503): every agent runs in tmux, so a server without it cannot serve traffic and was reporting healthy ([hivecommons/crux#137](https://github.com/hivecommons/crux/issues/137)). Existing fields are unchanged; probes that treated any 200 as healthy now see the failure.
12 changes: 12 additions & 0 deletions src/crux/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@ type Health struct {
// that restarted the server sees the uptime reset.
StartedAt time.Time `json:"started_at"`
UptimeSeconds float64 `json:"uptime_seconds"`
// Tmux is where tmux resolves on PATH, "" when it does not. Every agent
// runs inside tmux, so the route answers 503 (with this body) when empty.
Tmux string `json:"tmux"`
// Error and Code are set only on a non-2xx answer and share their wire
// keys with api.Error, so the 503 body also decodes as the Error that
// Client surfaces in a *StatusError: a caller polling health (the
// supervisor's WaitReady) sees "tmux not found on PATH", not a bare 503.
Error string `json:"error,omitempty"`
Code string `json:"code,omitempty"`
}

// HiveCallback is the wire form of the callback client's status.
Expand Down Expand Up @@ -254,4 +263,7 @@ const (
// holds; the runtime does not replace definitions.
CodeConflict = "conflict"
CodeRuntime = "runtime_error"
// CodeUnavailable is a server that is up but cannot serve: GET /v1/health
// answers 503 with it when tmux is missing from PATH.
CodeUnavailable = "unavailable"
)
33 changes: 33 additions & 0 deletions src/crux/api/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,3 +512,36 @@ func TestClientStatusTracksReachability(t *testing.T) {
t.Fatalf("status after success = %+v", st)
}
}

// A 503 health body carries error/code alongside the health fields, so the
// client names the cause (the supervisor's WaitReady logs this error).
func TestClientHealthUnavailableNamesCause(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", ContentTypeJSON)
w.WriteHeader(http.StatusServiceUnavailable)
_ = json.NewEncoder(w).Encode(Health{
Version: Version,
Error: "tmux not found on PATH: exec: \"tmux\": executable file not found in $PATH",
Code: CodeUnavailable,
})
}))
t.Cleanup(srv.Close)
c, err := NewClient(srv.URL, "tok")
if err != nil {
t.Fatal(err)
}
_, err = c.Health(context.Background())
var se *StatusError
if !errors.As(err, &se) || se.Status != http.StatusServiceUnavailable || se.Body.Code != CodeUnavailable {
t.Fatalf("status error: %+v (%v)", se, err)
}
if !strings.Contains(err.Error(), "tmux not found on PATH") {
t.Fatalf("error does not name tmux: %v", err)
}
// WaitReady keeps polling a 503 (it is not 401) and ends with ctx.
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
defer cancel()
if err := c.WaitReady(ctx, time.Millisecond); !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("wait ready: %v", err)
}
}
1 change: 1 addition & 0 deletions src/crux/serve/families_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,7 @@ func TestServeWithoutHostOrBackground(t *testing.T) {
if len(rt.Background()) != 0 {
t.Fatal("background loops started while disabled")
}
t.Setenv("PATH", fakeTmuxDir(t))
var h api.Health
newFamServer(t).expect(http.MethodGet, api.PathHealth, nil, http.StatusOK, &h)
if len(h.Background) != 0 {
Expand Down
47 changes: 47 additions & 0 deletions src/crux/serve/health_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package serve

import (
"net/http"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"

"github.com/hivecommons/hive/crux/api"
)

// fakeTmuxDir returns a directory holding an executable named tmux, for a
// PATH that resolves it without needing a real install.
func fakeTmuxDir(t *testing.T) string {
t.Helper()
dir := t.TempDir()
if err := os.WriteFile(filepath.Join(dir, "tmux"), []byte("#!/bin/sh\n"), 0o755); err != nil {
t.Fatal(err)
}
return dir
}

func TestHealthTmuxMissingIs503(t *testing.T) {
t.Setenv("PATH", t.TempDir())
var h api.Health
newFamServer(t).expect(http.MethodGet, api.PathHealth, nil, http.StatusServiceUnavailable, &h)
if h.Tmux != "" || h.Version != api.Version {
t.Fatalf("health: %+v", h)
}
// The body is also an api.Error, so api.Client callers see the cause.
if h.Code != api.CodeUnavailable || !strings.Contains(h.Error, "tmux not found on PATH") {
t.Fatalf("health error: %+v", h)
}
}

func TestHealthTmuxPresentIs200(t *testing.T) {
if _, err := exec.LookPath("tmux"); err != nil {
t.Skip("tmux not installed")
}
var h api.Health
newFamServer(t).expect(http.MethodGet, api.PathHealth, nil, http.StatusOK, &h)
if h.Tmux == "" {
t.Fatalf("health: %+v", h)
}
}
12 changes: 11 additions & 1 deletion src/crux/serve/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/hivecommons/hive/crux/internal/privatedir"

"github.com/hivecommons/hive/crux"
"github.com/hivecommons/hive/crux/agent/tmux"
"github.com/hivecommons/hive/crux/api"
"github.com/hivecommons/hive/crux/checkout"
"github.com/hivecommons/hive/crux/config"
Expand Down Expand Up @@ -388,7 +389,16 @@ func (s *Server) handleHealth(w http.ResponseWriter, _ *http.Request) {
hc := s.HiveCallback()
h.HiveCallback = &hc
}
s.writeJSON(w, http.StatusOK, h)
status := http.StatusOK
if path, err := tmux.LookPath(); err != nil {
// The body doubles as an api.Error so api.Client reports the cause.
status = http.StatusServiceUnavailable
h.Error = fmt.Sprintf("tmux not found on PATH: %v", err)
h.Code = api.CodeUnavailable
} else {
h.Tmux = path
}
s.writeJSON(w, status, h)
}

func (s *Server) handleList(w http.ResponseWriter, _ *http.Request) {
Expand Down
Loading