Add and remove models from the command line

The last piece of not restarting a control plane to try a model:

  llmgw ctl model add -f qwen-coder-32b.yaml
  llmgw ctl model remove qwen-coder-32b

The file is the same YAML a model takes in server.yaml, read by the same
koanf parser through the same tags, so an entry moves between the two by
being copied and a field that works in one cannot silently mean nothing in
the other.

Wired only when there is a store. A control plane without one says so
rather than accepting a model that vanishes on the next restart, which is
the kind of success that wastes an afternoon. At startup the catalog is
rebuilt from configuration plus what was stored, so models added before a
restart are still there after it — without that the whole feature would be
a slower way to lose work.

Every failure adding a model is the definition being wrong — a collision, a
missing fallback, a profile that cannot be placed — so they are 400s. A 500
would send someone to the server's logs for a mistake in their own file.

The actor comes from the caller and is kept with the row, so a catalog that
has grown over months can say where each entry came from.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

internal/control/adminapi/api.go

@@ -7,19 +7,23 @@
 package adminapi
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"time"
 
 	"gitea.mynodes.xyz/ilker/llmgw/internal/control"
+	"gitea.mynodes.xyz/ilker/llmgw/internal/control/catalog"
 	"gitea.mynodes.xyz/ilker/llmgw/internal/control/inventory"
 	"gitea.mynodes.xyz/ilker/llmgw/internal/control/reconciler"
 )
 
 // API serves the management endpoints.
 type API struct {
-	ctl *control.Controller
+	models Models
+	ctl    *control.Controller
 }
 
 // New builds the API over a controller.
@@ -28,6 +32,11 @@ func New(ctl *control.Controller) *API { return &API{ctl: ctl} }
 // Register mounts the endpoints on a mux under /api/v1.
 func (a *API) Register(mux *http.ServeMux) {
 	mux.HandleFunc("GET /api/v1/summary", a.summary)
+	// Adding a model is a write, and the only one this API has. It lives here
+	// rather than on the client-facing listener for the same reason the rest of
+	// it does: reaching this port already means holding the control plane.
+	mux.HandleFunc("POST /api/v1/models", a.addModel)
+	mux.HandleFunc("DELETE /api/v1/models/{name}", a.removeModel)
 	mux.HandleFunc("GET /api/v1/nodes", a.listNodes)
 	mux.HandleFunc("GET /api/v1/models", a.listModels)
 	mux.HandleFunc("GET /api/v1/replicas", a.listReplicas)
@@ -237,6 +246,52 @@ func (a *API) Replicas(model string) []ReplicaView {
 	return out
 }
 
+// Models is the service that owns catalog changes, when there is one.
+//
+// Optional because a control plane with no store cannot persist a model, and
+// pretending otherwise would accept a model that vanishes on restart.
+type Models interface {
+	Add(ctx context.Context, m catalog.Model, by string) error
+	Remove(ctx context.Context, name string) error
+}
+
+// SetModels supplies the catalog-changing service.
+func (a *API) SetModels(m Models) { a.models = m }
+
+func (a *API) addModel(w http.ResponseWriter, r *http.Request) {
+	if a.models == nil {
+		writeError(w, http.StatusNotImplemented,
+			"this control plane has no store, so a model added here would not survive a restart")
+		return
+	}
+	var m catalog.Model
+	if err := json.NewDecoder(io.LimitReader(r.Body, 1<<20)).Decode(&m); err != nil {
+		writeError(w, http.StatusBadRequest, "the model definition is not valid JSON: "+err.Error())
+		return
+	}
+	if err := a.models.Add(r.Context(), m, r.Header.Get("X-Llmgw-Actor")); err != nil {
+		// Every failure here is the definition being wrong, which is the
+		// caller's to fix — a collision, a missing fallback, a profile that
+		// cannot be placed. None of them are server faults.
+		writeError(w, http.StatusBadRequest, err.Error())
+		return
+	}
+	writeJSON(w, http.StatusOK, map[string]string{"added": m.Name})
+}
+
+func (a *API) removeModel(w http.ResponseWriter, r *http.Request) {
+	if a.models == nil {
+		writeError(w, http.StatusNotImplemented, "this control plane has no store")
+		return
+	}
+	name := r.PathValue("name")
+	if err := a.models.Remove(r.Context(), name); err != nil {
+		writeError(w, http.StatusBadRequest, err.Error())
+		return
+	}
+	writeJSON(w, http.StatusOK, map[string]string{"removed": name})
+}
+
 func (a *API) summary(w http.ResponseWriter, _ *http.Request) {
 	writeJSON(w, http.StatusOK, a.Summary())
 }

internal/control/adminapi/api_test.go

@@ -3,6 +3,7 @@ package adminapi
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
@@ -246,3 +247,109 @@ func TestEmptyClusterReturnsArraysNotNull(t *testing.T) {
 		}
 	}
 }
+
+type fakeModels struct {
+	added   []catalog.Model
+	actors  []string
+	removed []string
+	err     error
+}
+
+func (f *fakeModels) Add(_ context.Context, m catalog.Model, by string) error {
+	if f.err != nil {
+		return f.err
+	}
+	f.added = append(f.added, m)
+	f.actors = append(f.actors, by)
+	return nil
+}
+
+func (f *fakeModels) Remove(_ context.Context, name string) error {
+	if f.err != nil {
+		return f.err
+	}
+	f.removed = append(f.removed, name)
+	return nil
+}
+
+func TestAddingAModelOverTheAPI(t *testing.T) {
+	a, _ := setup(t, testModels())
+	fm := &fakeModels{}
+	a.SetModels(fm)
+	mux := http.NewServeMux()
+	a.Register(mux)
+
+	body := `{"name":"new","source":"/models/new","profiles":[{"name":"p","engine":"llamacpp"}]}`
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/models", strings.NewReader(body))
+	req.Header.Set("X-Llmgw-Actor", "ilker")
+	rec := httptest.NewRecorder()
+	mux.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status %d: %s", rec.Code, rec.Body.String())
+	}
+	if len(fm.added) != 1 || fm.added[0].Name != "new" {
+		t.Fatalf("added = %+v", fm.added)
+	}
+	// Who made the change, kept so a catalog that grew over months can say
+	// where each entry came from.
+	if len(fm.actors) != 1 || fm.actors[0] != "ilker" {
+		t.Errorf("actor = %v", fm.actors)
+	}
+}
+
+func TestABadModelDefinitionIsTheCallersFault(t *testing.T) {
+	// Every failure here is the definition being wrong — a collision, a missing
+	// fallback, a profile that cannot be placed. None are server faults, and
+	// returning 500 for them would send someone looking at the wrong logs.
+	a, _ := setup(t, testModels())
+	a.SetModels(&fakeModels{err: errors.New("model \"x\" collides")})
+	mux := http.NewServeMux()
+	a.Register(mux)
+
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/models", strings.NewReader(`{"name":"x"}`))
+	rec := httptest.NewRecorder()
+	mux.ServeHTTP(rec, req)
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("status %d, expected 400", rec.Code)
+	}
+	if !strings.Contains(rec.Body.String(), "collides") {
+		t.Errorf("the reason was lost: %s", rec.Body.String())
+	}
+}
+
+func TestWithoutAStoreTheAPIRefusesRatherThanForgets(t *testing.T) {
+	// Accepting a model that vanishes on the next restart is worse than saying
+	// it cannot be kept.
+	a, _ := setup(t, testModels()) // SetModels never called
+	mux := http.NewServeMux()
+	a.Register(mux)
+
+	req := httptest.NewRequest(http.MethodPost, "/api/v1/models", strings.NewReader(`{"name":"x"}`))
+	rec := httptest.NewRecorder()
+	mux.ServeHTTP(rec, req)
+	if rec.Code != http.StatusNotImplemented {
+		t.Errorf("status %d, expected 501", rec.Code)
+	}
+	if !strings.Contains(rec.Body.String(), "restart") {
+		t.Errorf("the reason does not explain the consequence: %s", rec.Body.String())
+	}
+}
+
+func TestRemovingAModelOverTheAPI(t *testing.T) {
+	a, _ := setup(t, testModels())
+	fm := &fakeModels{}
+	a.SetModels(fm)
+	mux := http.NewServeMux()
+	a.Register(mux)
+
+	req := httptest.NewRequest(http.MethodDelete, "/api/v1/models/gone", nil)
+	rec := httptest.NewRecorder()
+	mux.ServeHTTP(rec, req)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status %d: %s", rec.Code, rec.Body.String())
+	}
+	if len(fm.removed) != 1 || fm.removed[0] != "gone" {
+		t.Errorf("removed = %v", fm.removed)
+	}
+}