carlos / platform Public

zdd-6 task 11: fix round 1

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude 2 weeks ago 33d2d597b52dd87fccea9f57dbf0b8709c4989df parent 7401745

4 files changed, +65 −10

internal/console/apiretry.go +4 −2
internal/console/apiretry_test.go +38 −8
internal/host/restart.go +2 −0
internal/host/restartscope_test.go +21 −0

diff --git a/internal/console/apiretry.go b/internal/console/apiretry.go

index 1b403d1c3..343dff56f 100644

--- a/internal/console/apiretry.go

+++ b/internal/console/apiretry.go

@@ -78,12 +78,14 @@ func (s *Server) handleAPIInstanceRetry(w http.ResponseWriter, r *http.Request)

}

p, _ := principalFrom(r.Context())

if !s.allowWrite(p, time.Now()) {

- jsonError(w, http.StatusTooManyRequests, "too many requests. Wait a minute and try again")

+ // TODO-COPY zdd.api.retry-limited

+ jsonError(w, http.StatusTooManyRequests, "too many requests. wait a minute and try again")

return

}

if s.cfg.Releases == nil {

+ // TODO-COPY zdd.api.retry-no-bucket

jsonError(w, http.StatusServiceUnavailable,

- "no deployment bucket on this host. This console cannot request a retry")

+ "no deployment bucket on this host. this console cannot request a retry")

return

}

var req struct {

diff --git a/internal/console/apiretry_test.go b/internal/console/apiretry_test.go

index d113db12e..24a48a30b 100644

--- a/internal/console/apiretry_test.go

+++ b/internal/console/apiretry_test.go

@@ -3,7 +3,9 @@ package console

import (

"context"

"encoding/json"

+ "fmt"

"net/http"

+ "strings"

"testing"

"time"

@@ -39,8 +41,8 @@ func TestAPIInstanceRetryWritesARetryRequest(t *testing.T) {

token := cliToken(t, fx, "paul@keymail.dev", "code-host-retry")

u := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/kass/instances/" + host + "/retry"

- if code, _, body := rawReq(t, http.MethodPost, u, token, `{}`); code != http.StatusConflict {

- t.Fatalf("no report = %d %s, want 409", code, body)

+ if code, _, body := rawReq(t, http.MethodPost, u, token, `{}`); code != http.StatusConflict || !strings.Contains(body, fmt.Sprintf(apiRetryNotHeld, host)) {

+ t.Fatalf("no report = %d %s, want 409 naming %s as not held", code, body, host)

}

writeHeldBox(t, fx, host, "kass", "v2")

code, _, body := rawReq(t, http.MethodPost, u, token, `{}`)

@@ -70,13 +72,16 @@ func TestAPIInstanceRetryIgnoresAnotherAppsReport(t *testing.T) {

writeHeldBox(t, fx, host, "otherapp", "z2")

token := cliToken(t, fx, "paul@keymail.dev", "code-host-retry-x")

u := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/kass/instances/" + host + "/retry"

- if code, _, body := rawReq(t, http.MethodPost, u, token, `{}`); code != http.StatusConflict {

- t.Fatalf("another app's hold = %d %s, want 409", code, body)

+ if code, _, body := rawReq(t, http.MethodPost, u, token, `{}`); code != http.StatusConflict || !strings.Contains(body, fmt.Sprintf(apiRetryNotHeld, host)) {

+ t.Fatalf("another app's hold = %d %s, want 409 naming %s as not held", code, body, host)

}

-// A box that could not arm its scheduler reports no per-host state; the host is

-// not held, whatever else its view says.

+// A box that could not arm its scheduler reports no per-host state: its view is

+// built with Unavailable and no HeldBy, so the host is not held. This pins that

+// whole path to the 409. It cannot tell retryHostBuild's own Unavailable check

+// from the HeldBy check, because loadBoxJoin never builds a view carrying both;

+// the check is a guard for a join that someday does.

func TestAPIInstanceRetryRefusesAnUnavailableBox(t *testing.T) {

fx := newConsoleFixture(t)

const host = "web.kass.carlosapp.dev"

@@ -91,7 +96,32 @@ func TestAPIInstanceRetryRefusesAnUnavailableBox(t *testing.T) {

}

token := cliToken(t, fx, "paul@keymail.dev", "code-host-retry-u")

u := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/kass/instances/" + host + "/retry"

- if code, _, body := rawReq(t, http.MethodPost, u, token, `{}`); code != http.StatusConflict {

- t.Fatalf("unavailable box = %d %s, want 409", code, body)

+ code, _, body := rawReq(t, http.MethodPost, u, token, `{}`)

+ if code != http.StatusConflict || !strings.Contains(body, fmt.Sprintf(apiRetryNotHeld, host)) {

+ t.Fatalf("unavailable box = %d %s, want 409 naming %s as not held", code, body, host)

+ }

+// A box that reports the host and holds nothing back: there is nothing to

+// retry, and no request may be written.

+func TestAPIInstanceRetryRefusesAHostThatIsNotHeld(t *testing.T) {

+ fx := newConsoleFixture(t)

+ const host = "web.kass.carlosapp.dev"

+ sibling(t, fx, "kass", host, "stable")

+ box := status.CollectBox(nil, status.Probes{MachineID: "box1"}, time.Now())

+ box.Metrics = &status.BoxMetrics{Rollout: &status.RolloutReport{Hosts: map[string]status.HostRollout{

+ host: {Account: fx.sqid, App: "kass", Installed: "v2", Serving: "v2", Good: "v2", Awake: true},

+ }}}

+ if err := status.WriteBox(context.Background(), fx.rel, box); err != nil {

+ t.Fatal(err)

+ }

+ token := cliToken(t, fx, "paul@keymail.dev", "code-host-retry-n")

+ u := fx.ts.URL + "/api/cli/apps/" + fx.sqid + "/kass/instances/" + host + "/retry"

+ code, _, body := rawReq(t, http.MethodPost, u, token, `{}`)

+ if code != http.StatusConflict || !strings.Contains(body, fmt.Sprintf(apiRetryNotHeld, host)) {

+ t.Fatalf("a healthy host = %d %s, want 409 naming %s as not held", code, body, host)

+ }

+ if g, found, _ := release.LoadRestartScopes(context.Background(), fx.rel, fx.sqid, "kass"); found && len(g.Requests) != 0 {

+ t.Fatalf("a retry was written for a host that is not held: %+v", g.Requests)

}

diff --git a/internal/host/restart.go b/internal/host/restart.go

index e41a637cf..59d63153f 100644

--- a/internal/host/restart.go

+++ b/internal/host/restart.go

@@ -856,6 +856,8 @@ func restartRequestLeg(ctx context.Context, routes []registry.Route, store relea

}

if mem.retryHost(host) {

log.Printf("agent: retry requested for %s: its failure hold is cleared and its next attempt runs as a canary", host)

+ } else {

+ log.Printf("agent: retry for %s: nothing held to clear", host)

}

scoped = withoutRetries(scoped)

diff --git a/internal/host/restartscope_test.go b/internal/host/restartscope_test.go

index b4fba8244..3fd6ca894 100644

--- a/internal/host/restartscope_test.go

+++ b/internal/host/restartscope_test.go

@@ -680,3 +680,24 @@ func TestRestartLegSeparatesARetryFromARestartInOneWindow(t *testing.T) {

t.Fatalf("retried %v, bounced %v; want the retry to a.example.com and the bounce of b.example.com only", retried, bounced)

}

+// A retry for a host the manager holds nothing for is logged as such, not

+// as a hold cleared.

+func TestRestartLegLogsARetryWithNothingHeld(t *testing.T) {

+ ctx, s := genKit(t)

+ dd := t.TempDir()

+ routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")}

+ mem := newRestartMemory()

+ mem.retryHost = func(string) bool { return false }

+ bounce := func(h string) bool { t.Errorf("%s was bounced", h); return true }

+ restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem)

+ askScoped(t, ctx, s, "bab", "eleven", release.RestartScopeRetry, "a.example.com")

+ var logbuf bytes.Buffer

+ log.SetOutput(&logbuf)

+ defer log.SetOutput(os.Stderr)

+ restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem)

+ got := logbuf.String()

+ if !strings.Contains(got, "retry for a.example.com: nothing held to clear") || strings.Contains(got, "hold is cleared") {

+ t.Fatalf("log = %q", got)

+ }