amadan

carlos / platform Public

console: a run's serial bump and run-start version use the admission's context (final review M2)

After the admission the door bumped the serial and read the channel's
version on the request context; a caller that left after its run was
admitted cost the run-start row its version. Both now run on wctx, the
detached admission context. (bumpSerial already detached its own write;
the test pins that too.)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 4228e7cb419df376111db847b7745b26d5c570b6 parent 5ff0c93
2 files changed, +79 −2
  • internal/console/apiruns.go +5 −2
  • internal/console/apiruns_test.go +74 −0
diff --git a/internal/console/apiruns.go b/internal/console/apiruns.go
index 25b24afb0..19e6921a2 100644
--- a/internal/console/apiruns.go
+++ b/internal/console/apiruns.go
@@ -308,9 +308,12 @@ func (s *Server) handleAPIRunCreate(w http.ResponseWriter, r *http.Request) {
}
return
}
- s.bumpSerial(ctx)
+ // The serial bump and the run-start row belong to the admission, so they
+ // run on its context too: a caller that left after it must not cost the
+ // box its prompt pickup or the row its version.
+ s.bumpSerial(wctx)
s.auditActivity(acct, app, actor, Activity{Action: "run-start", TargetType: "instance", Target: host,
- Detail: runStartDetail(req, s.runVersion(ctx, acct.Sqid, app, rec.Channel))})
+ Detail: runStartDetail(req, s.runVersion(wctx, acct.Sqid, app, rec.Channel))})
writeJSON(w, http.StatusAccepted, runs.Created{ID: id, Host: host, State: state})
}
diff --git a/internal/console/apiruns_test.go b/internal/console/apiruns_test.go
index e412673eb..08ef18226 100644
--- a/internal/console/apiruns_test.go
+++ b/internal/console/apiruns_test.go
@@ -9,6 +9,7 @@ import (
"io"
"mime/multipart"
"net/http"
+ "net/http/httptest"
"os"
"strings"
"sync"
@@ -491,3 +492,76 @@ func TestAPIRunCreateAdmissionWhoseAnswerWasLost(t *testing.T) {
})
}
}
+
+// leavingStore answers every call whose context has ended with that
+// context's error, as a real bucket client does, and ends the caller's
+// request once the admission's record write has landed: the client went
+// away right after its run was admitted.
+type leavingStore struct {
+ release.Store
+ recordKey string
+ leave context.CancelFunc
+}
+
+func (l *leavingStore) Get(ctx context.Context, key string) ([]byte, string, error) {
+ if err := ctx.Err(); err != nil {
+ return nil, "", err
+ }
+ return l.Store.Get(ctx, key)
+}
+
+func (l *leavingStore) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) {
+ if err := ctx.Err(); err != nil {
+ return "", err
+ }
+ etag, err := l.Store.Put(ctx, key, body, ifMatch)
+ if err == nil && key == l.recordKey {
+ l.leave()
+ }
+ return etag, err
+}
+
+// A run admitted for a client that then went away still bumps the serial,
+// so a box picks it up at once, and its run-start row still names the
+// version the instance follows: both run on the admission's own context.
+func TestAPIRunCreateFinishesTheAdmissionForAClientThatLeft(t *testing.T) {
+ fx := runFixture(t)
+ shipVersion(t, fx.consoleFixture, "shop", "v1")
+ seedChannel(t, fx.consoleFixture, "shop", "stable", "v1")
+ before, _, _ := release.ReadSerial(context.Background(), fx.rel)
+
+ var buf bytes.Buffer
+ mw := multipart.NewWriter(&buf)
+ mp, _ := mw.CreateFormField("meta")
+ if err := json.NewEncoder(mp).Encode(okMeta()); err != nil {
+ t.Fatal(err)
+ }
+ mw.Close()
+ ctx, leave := context.WithCancel(context.Background())
+ defer leave()
+ fx.srv.cfg.Releases = &leavingStore{Store: fx.rel, recordKey: provision.RecordKey(release.OpsAccount, "shop", runTestHost), leave: leave}
+ req := httptest.NewRequestWithContext(ctx, http.MethodPost, fx.runsURL(runTestHost), &buf)
+ req.Header.Set("Content-Type", mw.FormDataContentType())
+ req.Header.Set("Authorization", "Bearer "+fx.member)
+ w := httptest.NewRecorder()
+ fx.srv.ServeHTTP(w, req)
+ if w.Code != http.StatusAccepted {
+ t.Fatalf("%d %s", w.Code, w.Body.String())
+ }
+ if ctx.Err() == nil {
+ t.Fatal("the client never left: the test proves nothing")
+ }
+ if after, _, err := release.ReadSerial(context.Background(), fx.rel); err != nil || after.Serial <= before.Serial {
+ t.Errorf("serial %d -> %d, %v: not bumped", before.Serial, after.Serial, err)
+ }
+ events, _ := fx.st.Events(OpsAccountID, "shop", 10)
+ found := false
+ for _, e := range events {
+ if e.Action == "run-start" && strings.Contains(e.Detail, `"version":"v1"`) {
+ found = true
+ }
+ }
+ if !found {
+ t.Errorf("no run-start row naming v1 in %+v", events)
+ }
+}