admin
7264f02172
gates / gates (push) Successful in 28s
v0.100.x removed the serialisation: 26.2s -> ~9.85s mean. What remained was one Gitea package SEARCH per dropdown at 0.20-3.8s depending on load, which concurrency cannot help. Memoised for 60s IN MEMORY. The TTL was ruled by the operator against the workflow that cares: a bake-and-vouch session publishes an artifact and comes straight here to select it, so a minute is short enough not to be noticed and long enough that every reload in that session is instant. NOT persisted. Gitea IS the store for both the version list and the sha; a copy in hub_settings would be a second source of truth that can drift from the registry it describes, and the operator reads the sha here to confirm what they are about to vouch. An in-memory cache dies with the process and can never be mistaken for a record. A failed resolve is NOT cached — a blip must not pin an empty dropdown for a minute. But an empty list from a package that genuinely has no versions IS cached, because 'we found nothing' and 'we could not look' are different answers (CONTEXT S-39, applied to a list instead of a figure). THE FIRST VERSION OF THIS GOT THAT WRONG: the comment said only successful resolves were cached and the code cached the empty list anyway. TestArtifactChoices_FailureIsNotCached caught it before it shipped — which is the argument for writing the test that asserts the comment, and the same class this session spent the day closing. go build/vet/test green, go test -race clean, run separately from this commit.
249 lines
8.9 KiB
Go
249 lines
8.9 KiB
Go
package web
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"io"
|
|
"log"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"sync/atomic"
|
|
"testing"
|
|
"time"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-hub/internal/gitea"
|
|
)
|
|
|
|
// artifactChoices fans the per-version sha lookups out concurrently (hub v0.100.0). The
|
|
// Configuration page took 26 SECONDS with two dropdowns because it made 42 sequential metadata
|
|
// round-trips. These tests pin the three things that must survive the change: the ORDER, the
|
|
// per-version failure isolation, and the concurrency itself.
|
|
|
|
// fakeGitea serves the two endpoints the client uses, with a controllable per-request delay so
|
|
// "concurrent" is measurable rather than asserted.
|
|
type fakeGitea struct {
|
|
versions []string
|
|
delay time.Duration
|
|
failVer string // this version's /files call 500s
|
|
|
|
inFlight int32
|
|
maxSeen int32
|
|
callCount int32
|
|
}
|
|
|
|
func (f *fakeGitea) start(t *testing.T) *gitea.Client {
|
|
t.Helper()
|
|
mux := http.NewServeMux()
|
|
// BOTH patterns: the version list is GET /api/v1/packages/admin?... with NO trailing slash,
|
|
// the per-version files call is /api/v1/packages/admin/generic/<pkg>/<ver>/files.
|
|
handler := func(w http.ResponseWriter, r *http.Request) {
|
|
n := atomic.AddInt32(&f.inFlight, 1)
|
|
for {
|
|
old := atomic.LoadInt32(&f.maxSeen)
|
|
if n <= old || atomic.CompareAndSwapInt32(&f.maxSeen, old, n) {
|
|
break
|
|
}
|
|
}
|
|
defer atomic.AddInt32(&f.inFlight, -1)
|
|
atomic.AddInt32(&f.callCount, 1)
|
|
time.Sleep(f.delay)
|
|
|
|
// .../generic/<pkg>/<version>/files → the sha metadata
|
|
if len(r.URL.Path) > 6 && r.URL.Path[len(r.URL.Path)-6:] == "/files" {
|
|
for _, v := range f.versions {
|
|
if f.failVer != "" && v == f.failVer && strings.Contains(r.URL.Path, "/"+v+"/") {
|
|
http.Error(w, "boom", http.StatusInternalServerError)
|
|
return
|
|
}
|
|
}
|
|
ver := versionFromFilesPath(r.URL.Path)
|
|
fmt.Fprintf(w, `[{"name":"felhom-agent","sha256":"sha-%s"}]`, ver)
|
|
return
|
|
}
|
|
// the version list
|
|
out := "["
|
|
for i, v := range f.versions {
|
|
if i > 0 {
|
|
out += ","
|
|
}
|
|
out += fmt.Sprintf(`{"name":"felhom-agent","type":"generic","version":%q}`, v)
|
|
}
|
|
fmt.Fprint(w, out+"]")
|
|
}
|
|
mux.HandleFunc("/api/v1/packages/admin", handler)
|
|
mux.HandleFunc("/api/v1/packages/admin/", handler)
|
|
srv := httptest.NewServer(mux)
|
|
t.Cleanup(srv.Close)
|
|
return gitea.New(srv.URL, "admin", "u", "t")
|
|
}
|
|
|
|
func versionFromFilesPath(p string) string {
|
|
// .../generic/<pkg>/<version>/files
|
|
end := len(p) - len("/files")
|
|
start := end - 1
|
|
for start > 0 && p[start-1] != '/' {
|
|
start--
|
|
}
|
|
return p[start:end]
|
|
}
|
|
|
|
func newTestWebServer(t *testing.T, c *gitea.Client) *Server {
|
|
t.Helper()
|
|
s := &Server{logger: log.New(io.Discard, "", 0)}
|
|
s.SetGiteaClient(c)
|
|
return s
|
|
}
|
|
|
|
// The dropdown is newest-first and the concurrent version must not scramble it.
|
|
func TestArtifactChoices_PreservesOrder(t *testing.T) {
|
|
f := &fakeGitea{versions: []string{"0.9.0", "0.8.0", "0.7.0", "0.6.0", "0.5.0"}}
|
|
s := newTestWebServer(t, f.start(t))
|
|
|
|
got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent")
|
|
|
|
if len(got) != 5 {
|
|
t.Fatalf("got %d choices, want 5: %+v", len(got), got)
|
|
}
|
|
// ListVersions sorts newest-first; the fan-out must not disturb that, and each sha must belong
|
|
// to ITS OWN version — a slot mix-up would show the operator a hash for a different artifact.
|
|
for i, want := range []string{"0.9.0", "0.8.0", "0.7.0", "0.6.0", "0.5.0"} {
|
|
if got[i].Version != want {
|
|
t.Errorf("choice %d = %q, want %q — the concurrent fan-out scrambled the order", i, got[i].Version, want)
|
|
}
|
|
if got[i].SHA256 != "sha-"+want {
|
|
t.Errorf("version %s carries sha %q — a sha was written into the wrong slot, which would show "+
|
|
"the operator a hash belonging to a different artifact", got[i].Version, got[i].SHA256)
|
|
}
|
|
}
|
|
}
|
|
|
|
// A single failing version drops ITSELF and nothing else — unchanged from the serial version.
|
|
func TestArtifactChoices_OneFailureDropsOnlyThatVersion(t *testing.T) {
|
|
f := &fakeGitea{versions: []string{"0.9.0", "0.8.0", "0.7.0"}, failVer: "0.8.0"}
|
|
s := newTestWebServer(t, f.start(t))
|
|
|
|
got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent")
|
|
|
|
if len(got) != 2 {
|
|
t.Fatalf("got %d choices, want 2 (the failing one dropped): %+v", len(got), got)
|
|
}
|
|
for _, c := range got {
|
|
if c.Version == "0.8.0" {
|
|
t.Error("the failing version was included")
|
|
}
|
|
}
|
|
if got[0].Version != "0.9.0" || got[1].Version != "0.7.0" {
|
|
t.Errorf("order broken around the dropped version: %+v", got)
|
|
}
|
|
}
|
|
|
|
// THE POINT OF THE CHANGE. With a per-request delay, a serial implementation takes
|
|
// len(versions) * delay; the concurrent one takes about ceil(n/8) * delay. Asserting the wall-clock
|
|
// is what makes this a test of the fix rather than of the plumbing — and the in-flight counter
|
|
// proves requests genuinely overlapped rather than the timing being luck.
|
|
func TestArtifactChoices_IsConcurrent(t *testing.T) {
|
|
const n = 16
|
|
vers := make([]string, n)
|
|
for i := range vers {
|
|
vers[i] = fmt.Sprintf("0.%d.0", 100-i) // already newest-first
|
|
}
|
|
f := &fakeGitea{versions: vers, delay: 50 * time.Millisecond}
|
|
s := newTestWebServer(t, f.start(t))
|
|
|
|
start := time.Now()
|
|
got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent")
|
|
elapsed := time.Since(start)
|
|
|
|
if len(got) != n {
|
|
t.Fatalf("got %d choices, want %d", len(got), n)
|
|
}
|
|
serial := time.Duration(n) * 50 * time.Millisecond // 800ms
|
|
if elapsed > serial/2 {
|
|
t.Errorf("took %v; a serial implementation would take ~%v and the concurrent one should be far "+
|
|
"under half of that. This is the 26-second Configuration page.", elapsed, serial)
|
|
}
|
|
if max := atomic.LoadInt32(&f.maxSeen); max < 2 {
|
|
t.Errorf("max concurrent in-flight requests was %d — the lookups did not actually overlap, so a "+
|
|
"fast wall-clock here would be luck rather than concurrency", max)
|
|
}
|
|
// and it must respect the bound rather than opening one connection per version
|
|
if max := atomic.LoadInt32(&f.maxSeen); max > 8 {
|
|
t.Errorf("max concurrent in-flight was %d, above the bound of 8 — a large package list would "+
|
|
"stampede Gitea", max)
|
|
}
|
|
}
|
|
|
|
// The cap is unchanged: at most 20 versions are offered however many exist.
|
|
func TestArtifactChoices_StillCapsAtTwenty(t *testing.T) {
|
|
vers := make([]string, 30)
|
|
for i := range vers {
|
|
vers[i] = fmt.Sprintf("0.%d.0", 200-i)
|
|
}
|
|
f := &fakeGitea{versions: vers}
|
|
s := newTestWebServer(t, f.start(t))
|
|
|
|
got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent")
|
|
if len(got) != 20 {
|
|
t.Errorf("got %d choices, want the 20 cap", len(got))
|
|
}
|
|
if atomic.LoadInt32(&f.callCount) > 21 { // 1 version list + 20 sha lookups
|
|
t.Errorf("made %d requests; the cap must limit the FAN-OUT too, not just the rendered list",
|
|
atomic.LoadInt32(&f.callCount))
|
|
}
|
|
}
|
|
|
|
// R-267: the dropdown is memoised for artifactChoicesTTL. A second load inside the window must make
|
|
// NO further Gitea calls; a failed resolve must NOT be cached, or a blip would pin an empty list in
|
|
// front of the operator for a minute.
|
|
func TestArtifactChoices_CachedWithinTTL(t *testing.T) {
|
|
f := &fakeGitea{versions: []string{"0.9.0", "0.8.0"}}
|
|
s := newTestWebServer(t, f.start(t))
|
|
|
|
first := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent")
|
|
after := atomic.LoadInt32(&f.callCount)
|
|
if after == 0 {
|
|
t.Fatal("the first resolve made no calls")
|
|
}
|
|
second := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent")
|
|
|
|
if atomic.LoadInt32(&f.callCount) != after {
|
|
t.Errorf("a second load inside the TTL hit Gitea again (%d -> %d calls) — the cache is not "+
|
|
"in the path", after, atomic.LoadInt32(&f.callCount))
|
|
}
|
|
if len(second) != len(first) || second[0].Version != first[0].Version || second[0].SHA256 != first[0].SHA256 {
|
|
t.Errorf("cached result differs from the fresh one: %+v vs %+v", second, first)
|
|
}
|
|
}
|
|
|
|
func TestArtifactChoices_FailureIsNotCached(t *testing.T) {
|
|
// every version fails → an empty list, which must NOT be remembered
|
|
f := &fakeGitea{versions: []string{"0.9.0"}, failVer: "0.9.0"}
|
|
s := newTestWebServer(t, f.start(t))
|
|
|
|
if got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent"); len(got) != 0 {
|
|
t.Fatalf("precondition: expected an empty list, got %+v", got)
|
|
}
|
|
before := atomic.LoadInt32(&f.callCount)
|
|
_ = s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent")
|
|
if atomic.LoadInt32(&f.callCount) == before {
|
|
t.Error("an empty/failed resolve was cached — a Gitea blip would then show the operator an " +
|
|
"empty dropdown for a full minute with no way to retry")
|
|
}
|
|
}
|
|
|
|
// Two packages must not share a cache slot.
|
|
func TestArtifactChoices_CacheIsPerPackage(t *testing.T) {
|
|
f := &fakeGitea{versions: []string{"0.9.0"}}
|
|
s := newTestWebServer(t, f.start(t))
|
|
a := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent")
|
|
before := atomic.LoadInt32(&f.callCount)
|
|
_ = s.artifactChoices(context.Background(), "felhom-golden", "golden.tar.zst")
|
|
if atomic.LoadInt32(&f.callCount) == before {
|
|
t.Error("a different package was served from the first package's cache entry")
|
|
}
|
|
if len(a) == 0 {
|
|
t.Error("precondition")
|
|
}
|
|
}
|