From 30a42c77a8b1a6c5bf609955dcbfe7c058766117 Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Thu, 29 Oct 2026 15:42:18 +0800 Subject: [PATCH 1/9] feat(server): protect Canary-managed model services --- internal/llmsvc/canary.go | 41 +++++++++ internal/llmsvc/canary_test.go | 39 +++++++++ internal/llmsvc/client.go | 5 ++ internal/server/api.go | 11 +-- internal/server/autoscale_loop_test.go | 9 +- internal/server/backend.go | 5 ++ internal/server/backend_llm.go | 11 ++- internal/server/backend_test.go | 41 ++++----- internal/server/canary.go | 49 +++++++++++ internal/server/canary_test.go | 110 +++++++++++++++++++++++++ internal/server/deploy.go | 7 ++ internal/server/rollback.go | 4 + internal/server/slo.go | 4 + internal/server/status.go | 8 +- 14 files changed, 303 insertions(+), 41 deletions(-) create mode 100644 internal/llmsvc/canary.go create mode 100644 internal/llmsvc/canary_test.go create mode 100644 internal/server/canary.go create mode 100644 internal/server/canary_test.go diff --git a/internal/llmsvc/canary.go b/internal/llmsvc/canary.go new file mode 100644 index 0000000..d73429a --- /dev/null +++ b/internal/llmsvc/canary.go @@ -0,0 +1,41 @@ +package llmsvc + +import ( + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +const CanaryPrefix = "canary.modelsphere.dev/" + +type CanaryOwner struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Namespace string `json:"namespace"` + Name string `json:"name"` + UID string `json:"uid"` + Verified bool `json:"verified"` +} + +// CanaryOwnership keeps orphaned or inconsistent ownership protected too. +func CanaryOwnership(u *unstructured.Unstructured) *CanaryOwner { + if u == nil { + return nil + } + labels := u.GetLabels() + for _, ref := range u.GetOwnerReferences() { + if strings.HasPrefix(ref.APIVersion, CanaryPrefix) && ref.Kind == "LLMCanary" { + return &CanaryOwner{ + APIVersion: ref.APIVersion, Kind: ref.Kind, Namespace: u.GetNamespace(), Name: ref.Name, UID: string(ref.UID), + Verified: ref.Controller != nil && *ref.Controller && ref.UID != "" && ref.Name != "" && + labels[CanaryPrefix+"canary-name"] == ref.Name && labels[CanaryPrefix+"canary-namespace"] == u.GetNamespace() && labels[CanaryPrefix+"canary-uid"] == string(ref.UID), + } + } + } + for _, key := range []string{"canary-name", "canary-namespace", "canary-uid"} { + if _, ok := labels[CanaryPrefix+key]; ok { + return &CanaryOwner{APIVersion: CanaryPrefix + "v1alpha1", Kind: "LLMCanary", Namespace: labels[CanaryPrefix+"canary-namespace"], Name: labels[CanaryPrefix+"canary-name"], UID: labels[CanaryPrefix+"canary-uid"]} + } + } + return nil +} diff --git a/internal/llmsvc/canary_test.go b/internal/llmsvc/canary_test.go new file mode 100644 index 0000000..b7e1719 --- /dev/null +++ b/internal/llmsvc/canary_test.go @@ -0,0 +1,39 @@ +package llmsvc + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestCanaryOwnershipProtectsIncompleteMetadata(t *testing.T) { + controller := true + ref := metav1.OwnerReference{APIVersion: "canary.modelsphere.dev/v1alpha1", Kind: "LLMCanary", Name: "demo", UID: "old-uid", Controller: &controller} + labels := map[string]string{CanaryPrefix + "canary-name": "demo", CanaryPrefix + "canary-namespace": "canary", CanaryPrefix + "canary-uid": "old-uid"} + for _, tc := range []struct { + name string + refs []metav1.OwnerReference + labels map[string]string + protected, verified bool + }{ + {"ordinary", nil, nil, false, false}, + {"matching", []metav1.OwnerReference{ref}, labels, true, true}, + {"removed labels", []metav1.OwnerReference{ref}, nil, true, false}, + {"removed reference", nil, labels, true, false}, + {"different UID", []metav1.OwnerReference{ref}, map[string]string{CanaryPrefix + "canary-uid": "new-uid"}, true, false}, + {"empty marker", nil, map[string]string{CanaryPrefix + "canary-name": ""}, true, false}, + {"unrelated owner", []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: "Deployment", Name: "demo", UID: "old-uid"}}, nil, false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + u := &unstructured.Unstructured{} + u.SetNamespace("canary") + u.SetOwnerReferences(tc.refs) + u.SetLabels(tc.labels) + owner := CanaryOwnership(u) + if (owner != nil) != tc.protected || (owner != nil && owner.Verified != tc.verified) { + t.Fatalf("%+v", owner) + } + }) + } +} diff --git a/internal/llmsvc/client.go b/internal/llmsvc/client.go index 9266f1d..1dcb573 100644 --- a/internal/llmsvc/client.go +++ b/internal/llmsvc/client.go @@ -87,6 +87,11 @@ func (c *Client) Delete(ctx context.Context, ns, name string) error { return c.resource(ns).Delete(ctx, name, metav1.DeleteOptions{}) } +func (c *Client) DeleteObserved(ctx context.Context, u *unstructured.Unstructured) error { + uid, rv := u.GetUID(), u.GetResourceVersion() + return c.resource(u.GetNamespace()).Delete(ctx, u.GetName(), metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid, ResourceVersion: &rv}}) +} + // Patch applies a patch and returns the updated object. func (c *Client) Patch(ctx context.Context, ns, name string, pt types.PatchType, data []byte) (*unstructured.Unstructured, error) { got, err := c.resource(ns).Patch(ctx, name, pt, data, metav1.PatchOptions{}) diff --git a/internal/server/api.go b/internal/server/api.go index db0a5c9..fabe7a5 100644 --- a/internal/server/api.go +++ b/internal/server/api.go @@ -586,10 +586,11 @@ type deployment struct { Route string `json:"route,omitempty"` // Backend is helm or llmsvc. External is an LLMService swiss did not // write. CleanupPending means both records still exist. - Backend string `json:"backend"` - External bool `json:"external,omitempty"` - CleanupPending bool `json:"cleanupPending,omitempty"` - Deletion *llmDeletionView `json:"deletion,omitempty"` + Backend string `json:"backend"` + External bool `json:"external,omitempty"` + ManagedBy *llmsvc.CanaryOwner `json:"managedBy,omitempty"` + CleanupPending bool `json:"cleanupPending,omitempty"` + Deletion *llmDeletionView `json:"deletion,omitempty"` // What the plan recorded about its catalog, for finding the configured // catalog the release belongs to. @@ -670,7 +671,7 @@ func (s *Server) deploymentFromService(ctx context.Context, ref deployRef) *depl u := ref.obj d := &deployment{ Release: u.GetName(), Namespace: u.GetNamespace(), - Backend: backendLLMSVC, External: llmsvc.IsExternal(u), CleanupPending: ref.cleanup, + Backend: backendLLMSVC, External: llmsvc.IsExternal(u), CleanupPending: ref.cleanup, ManagedBy: llmsvc.CanaryOwnership(u), } obj, err := llmsvc.FromUnstructured(u) if err != nil { diff --git a/internal/server/autoscale_loop_test.go b/internal/server/autoscale_loop_test.go index ed822f2..212c505 100644 --- a/internal/server/autoscale_loop_test.go +++ b/internal/server/autoscale_loop_test.go @@ -219,7 +219,7 @@ func runLLMLoop(t *testing.T, tc loopCase) { return true, nil, err } copied := obj.(*unstructured.Unstructured).DeepCopy() - if n >= 6 { + if n >= 7 { helm, _, _ := unstructured.NestedMap(copied.Object, "status", "helm") if helm == nil { helm = map[string]any{} @@ -243,10 +243,9 @@ func runLLMLoop(t *testing.T, tc loopCase) { t.Fatal(err) } if tc.Conflict { - // One more than the loop alone: applyLocked reads the LLMService - // once to refuse a release that is being deleted. - if gotGets != 8 { - t.Errorf("Get calls %d, want 8", gotGets) + // applyLocked checks Canary ownership and deletion before applying. + if gotGets != 9 { + t.Errorf("Get calls %d, want 9", gotGets) } if st.LastError == "" || !strings.Contains(st.LastError, tc.ErrorContains) { t.Fatalf("lastError %q, want substring %q (gets %d)", st.LastError, tc.ErrorContains, gets.Load()) diff --git a/internal/server/backend.go b/internal/server/backend.go index 9039173..858aa41 100644 --- a/internal/server/backend.go +++ b/internal/server/backend.go @@ -82,6 +82,11 @@ func failErr(code int, err error) error { } func writeStatus(w http.ResponseWriter, err error, fallback int) { + var managed *canaryManagedError + if errors.As(err, &managed) { + writeJSON(w, http.StatusConflict, map[string]any{"error": managed.Error(), "code": "CANARY_MANAGED_SERVICE", "owner": managed.Owner}) + return + } code := fallback var hf *httpFailure if errors.As(err, &hf) { diff --git a/internal/server/backend_llm.go b/internal/server/backend_llm.go index edcee7c..4b9d293 100644 --- a/internal/server/backend_llm.go +++ b/internal/server/backend_llm.go @@ -183,6 +183,9 @@ func (b llmBackend) Apply(ctx context.Context, p *plan.Plan, mode exec.Mode, o a if err != nil { return applyResult{}, failErr(http.StatusBadGateway, err) } + if err := refuseCanaryService(cur); err != nil { + return applyResult{}, err + } if llmsvc.IsExternal(cur) { return applyResult{}, fail(http.StatusConflict, "release %s is an external LLMService: swiss did not deploy it, so it cannot be upgraded", ref) } @@ -318,6 +321,9 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal if err != nil { return uninstallResult{}, failErr(http.StatusBadGateway, err) } + if err := refuseCanaryService(u); err != nil { + return uninstallResult{}, err + } // The operator's finalizer holds the object through route removal and pod // termination. This call returns once that delete is accepted; the reap // drops swiss's records when the object is finally gone. @@ -343,7 +349,7 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal } started := time.Now() - delErr := s.llms.Delete(ctx, ns, release) + delErr := s.llms.DeleteObserved(ctx, u) if apierrors.IsNotFound(delErr) { if hadPlan { if rerr := s.restorePlan(ctx, ns, release, prev); rerr != nil { @@ -361,6 +367,9 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal s.log.ErrorContext(ctx, "plan not marked failed", "namespace", ns, "release", release, "err", ferr) } } + if apierrors.IsConflict(delErr) { + return uninstallResult{}, failErr(http.StatusConflict, delErr) + } return uninstallResult{}, failErr(http.StatusInternalServerError, delErr) } // Record before tracking. The object may already be gone, and a reap that diff --git a/internal/server/backend_test.go b/internal/server/backend_test.go index d01c2c0..654c61b 100644 --- a/internal/server/backend_test.go +++ b/internal/server/backend_test.go @@ -245,9 +245,8 @@ func newGateServer(t *testing.T, kind, applyWith, helmBin, helmfileBin string) * return g } -// A served CRD that this swissd cannot read must not turn the helm paths into -// 502s. Forbidden and a discovery error both behave as an unserved CRD. -func TestLLMInvisibleMatchesUnserved(t *testing.T) { +// Read fallback remains available; mutations require an ownership check. +func TestLLMInvisibleKeepsReadsAndRefusesWrites(t *testing.T) { helmBin := failBin(t, "helm") helmfileBin := failBin(t, "helmfile") base := newGateServer(t, "unserved", backendHelm, helmBin, helmfileBin) @@ -273,26 +272,15 @@ func TestLLMInvisibleMatchesUnserved(t *testing.T) { t.Fatalf("status called get after forbidden was cached: %d", other.calls.Load()) } - bHash := gatePlan(t, base) oHash := gatePlan(t, other) - bc, bb := post(t, base.srv, "/api/install", map[string]any{"planHash": bHash}) - oc, ob := post(t, other.srv, "/api/install", map[string]any{"planHash": oHash}) - if bc != oc || bc == http.StatusBadGateway { - t.Fatalf("install base %d %v other %d %v", bc, bb, oc, ob) - } - if _, ok := base.w.written[planRef("models", "qwen-new")]; !ok { - t.Fatal("unserved install did not write the plan ConfigMap") - } - if _, ok := other.w.written[planRef("models", "qwen-new")]; !ok { - t.Fatal("hidden install did not write the plan ConfigMap") - } - bc, bb = post(t, base.srv, "/api/apply", map[string]any{"planHash": bHash}) - oc, ob = post(t, other.srv, "/api/apply", map[string]any{"planHash": oHash}) - if bc != oc || bc == http.StatusBadGateway { - t.Fatalf("upgrade base %d %v other %d %v", bc, bb, oc, ob) + for _, endpoint := range []string{"/api/install", "/api/apply"} { + code, body := post(t, other.srv, endpoint, map[string]any{"planHash": oHash}) + if code != http.StatusBadGateway || !strings.Contains(errText(body), "cannot verify service ownership") { + t.Fatalf("%s: %d %v", endpoint, code, body) + } } - if kind == "forbidden" && other.calls.Load() != 1 { - t.Fatalf("apply called the API after forbidden was cached: %d", other.calls.Load()) + if len(other.w.written) != 0 { + t.Fatal("unverified ownership wrote a plan") } _, body := get(t, other.srv, "/api/cluster") @@ -318,9 +306,8 @@ func TestLLMInvisibleMatchesUnserved(t *testing.T) { } } -// applyWith llmsvc falls back to helm when the CRD is served but not granted, -// and when discovery itself fails. -func TestLLMApplyWithHiddenFallsBackToHelm(t *testing.T) { +// An llmsvc profile cannot bypass ownership checks by falling back to helm. +func TestLLMApplyWithHiddenRefusesWrites(t *testing.T) { helmBin := failBin(t, "helm") helmfileBin := failBin(t, "helmfile") for _, kind := range []string{"forbidden", "discovery"} { @@ -328,11 +315,11 @@ func TestLLMApplyWithHiddenFallsBackToHelm(t *testing.T) { g := newGateServer(t, kind, backendLLMSVC, helmBin, helmfileBin) hash := gatePlan(t, g) code, out := post(t, g.srv, "/api/install", map[string]any{"planHash": hash}) - if code == http.StatusBadGateway { + if code != http.StatusBadGateway { t.Fatalf("install %d %v", code, out) } - if _, ok := g.w.written[planRef("models", "qwen-new")]; !ok { - t.Fatal("fallback did not write the helm plan ConfigMap") + if len(g.w.written) != 0 { + t.Fatal("unverified ownership wrote a helm plan") } _, body := get(t, g.srv, "/api/cluster") info := body["llmservice"].(map[string]any) diff --git a/internal/server/canary.go b/internal/server/canary.go new file mode 100644 index 0000000..1df4a35 --- /dev/null +++ b/internal/server/canary.go @@ -0,0 +1,49 @@ +package server + +import ( + "context" + "fmt" + "net/http" + + "github.com/modelsphere/swiss/internal/llmsvc" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +type canaryManagedError struct{ Owner *llmsvc.CanaryOwner } + +func (e *canaryManagedError) Error() string { + return fmt.Sprintf("service is managed by LLMCanary %s/%s (UID %s); manage configuration and cleanup from the experiment", e.Owner.Namespace, e.Owner.Name, e.Owner.UID) +} + +func refuseCanaryService(u *unstructured.Unstructured) error { + if owner := llmsvc.CanaryOwnership(u); owner != nil { + return &canaryManagedError{Owner: owner} + } + return nil +} + +func (s *Server) refuseCanaryWrite(ctx context.Context, ns, release string) error { + access := s.llmsvcAccess(ctx) + if access.discErr != nil || access.forbidden { + return fail(http.StatusBadGateway, "cannot verify service ownership: %s", access.why()) + } + if !access.served || s.llms == nil { + return nil + } + ns, err := s.resolveNamespace(ctx, ns, release) + if err != nil { + return failErr(http.StatusBadGateway, err) + } + u, err := s.llms.Get(ctx, ns, release) + if apierrors.IsNotFound(err) { + return nil + } + if err != nil { + if apierrors.IsForbidden(err) { + s.noteLLMSVCForbidden() + } + return fail(http.StatusBadGateway, "cannot verify service ownership: %s", err) + } + return refuseCanaryService(u) +} diff --git a/internal/server/canary_test.go b/internal/server/canary_test.go new file mode 100644 index 0000000..64b8d7c --- /dev/null +++ b/internal/server/canary_test.go @@ -0,0 +1,110 @@ +package server + +import ( + "net/http" + "testing" + + "github.com/modelsphere/swiss/internal/llmsvc" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + k8stesting "k8s.io/client-go/testing" +) + +func ownedArm() *unstructured.Unstructured { + u := llmObject("models", "demo-baseline", map[string]string{llmsvc.AnnProfile: "prod"}) + u.SetUID("child-uid") + u.SetResourceVersion("17") + controller := true + u.SetOwnerReferences([]metav1.OwnerReference{{APIVersion: "canary.modelsphere.dev/v1alpha1", Kind: "LLMCanary", Name: "demo", UID: "parent-uid", Controller: &controller}}) + u.SetLabels(map[string]string{llmsvc.CanaryPrefix + "canary-name": "demo", llmsvc.CanaryPrefix + "canary-namespace": "models", llmsvc.CanaryPrefix + "canary-uid": "parent-uid"}) + u.Object["status"] = map[string]any{"phase": "Applied", "helm": map[string]any{"revision": int64(1), "status": "deployed"}} + return u +} + +func TestLLMDeleteUsesObservedIdentity(t *testing.T) { + rig := startRig(t, llmProbe(), true, false, backendLLMSVC) + u := llmObject("models", "ordinary", nil) + u.SetUID("original-child") + u.SetResourceVersion("17") + if _, err := rig.c.Create(t.Context(), u); err != nil { + t.Fatal(err) + } + code, body := del(t, rig.srv, "/api/releases/models/ordinary") + if code != 200 { + t.Fatalf("ordinary service: %d %v", code, body) + } + found := false + for _, action := range rig.dyn.Actions() { + if action.GetVerb() != "delete" { + continue + } + found = true + pre := action.(k8stesting.DeleteAction).GetDeleteOptions().Preconditions + if pre == nil || pre.UID == nil || *pre.UID != "original-child" || pre.ResourceVersion == nil || *pre.ResourceVersion != "17" { + t.Fatalf("delete did not protect identity: %+v", pre) + } + } + if !found { + t.Fatal("ordinary service was not deleted") + } +} + +func TestCanaryArmReadAndWriteBoundaries(t *testing.T) { + rig := startRig(t, llmProbe(), true, false, backendLLMSVC) + u := ownedArm() + if _, err := rig.c.Create(t.Context(), u); err != nil { + t.Fatal(err) + } + for _, path := range []string{"/api/deployments", "/api/releases/models/demo-baseline/status"} { + code, body := get(t, rig.srv, path) + if code != 200 { + t.Fatalf("%s: %d %v", path, code, body) + } + if path == "/api/deployments" { + body = body["deployments"].([]any)[0].(map[string]any) + } + owner, _ := body["managedBy"].(map[string]any) + if owner["name"] != "demo" || owner["uid"] != "parent-uid" || owner["verified"] != true { + t.Fatal(body) + } + } + hash := rig.storePlan(t, map[string]any{"release": "demo-baseline"}) + rig.dyn.ClearActions() + checks := []struct { + path string + call func() (int, map[string]any) + }{ + {"apply", func() (int, map[string]any) { + return post(t, rig.srv, "/api/apply", map[string]any{"planHash": hash, "expectRevision": 1, "forceConflicts": true}) + }}, + {"install", func() (int, map[string]any) { + return post(t, rig.srv, "/api/install", map[string]any{"planHash": hash}) + }}, + {"rollback", func() (int, map[string]any) { + return post(t, rig.srv, "/api/releases/models/demo-baseline/rollback", map[string]any{"toRevision": 1, "expectRevision": 1}) + }}, + {"uninstall", func() (int, map[string]any) { return del(t, rig.srv, "/api/releases/models/demo-baseline") }}, + {"slo write", func() (int, map[string]any) { + return put(t, rig.srv, "/api/releases/models/demo-baseline/slo", map[string]any{"priority": 10}) + }}, + {"slo reset", func() (int, map[string]any) { return del(t, rig.srv, "/api/releases/models/demo-baseline/slo") }}, + {"slo read", func() (int, map[string]any) { return get(t, rig.srv, "/api/releases/models/demo-baseline/slo") }}, + } + for _, check := range checks { + t.Run(check.path, func(t *testing.T) { + code, body := check.call() + if code != http.StatusConflict || body["code"] != "CANARY_MANAGED_SERVICE" { + t.Fatalf("%d %v", code, body) + } + owner, _ := body["owner"].(map[string]any) + if owner["uid"] != "parent-uid" { + t.Fatal(body) + } + }) + } + for _, a := range rig.dyn.Actions() { + if a.GetVerb() == "delete" || a.GetVerb() == "update" || a.GetVerb() == "patch" || a.GetVerb() == "create" { + t.Fatalf("blocked write reached Kubernetes: %v", a) + } + } +} diff --git a/internal/server/deploy.go b/internal/server/deploy.go index a6d4ab2..eb4daa9 100644 --- a/internal/server/deploy.go +++ b/internal/server/deploy.go @@ -651,6 +651,9 @@ func (s *Server) apply(ctx context.Context, p *plan.Plan, mode exec.Mode, applyW // marker is written, so an apply cannot write its applying phase over the // marker and an uninstall cannot start in the middle of an apply. func (s *Server) applyLocked(ctx context.Context, p *plan.Plan, mode exec.Mode, applyWith string, o applyOpts) (applyResult, error) { + if err := s.refuseCanaryWrite(ctx, p.Release.Namespace, p.Release.Name); err != nil { + return applyResult{}, err + } if err := s.refuseUninstalling(ctx, p.Release.Namespace, p.Release.Name); err != nil { return applyResult{}, err } @@ -692,6 +695,10 @@ func (s *Server) handleUninstall(w http.ResponseWriter, r *http.Request) { defer cancel() ns, release := r.PathValue("namespace"), r.PathValue("release") + if err := s.refuseCanaryWrite(ctx, ns, release); err != nil { + writeStatus(w, err, http.StatusBadGateway) + return + } if !s.tryApplyLock(ns, release) { // The lock may be held by the uninstall itself, still running helm, // or by another call while an LLMService is already being deleted. diff --git a/internal/server/rollback.go b/internal/server/rollback.go index c3419fa..f93b536 100644 --- a/internal/server/rollback.go +++ b/internal/server/rollback.go @@ -200,6 +200,10 @@ func (s *Server) handleRevisionDiff(w http.ResponseWriter, r *http.Request) { // engine flags come back verbatim -- which is the point. Going back to what // worked must not mean going to whatever the catalog now says that version is. func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { + if err := s.refuseCanaryWrite(r.Context(), r.PathValue("namespace"), r.PathValue("release")); err != nil { + writeStatus(w, err, http.StatusBadGateway) + return + } ctx, cancel := contextWithTimeout(r, 15*time.Minute) defer cancel() diff --git a/internal/server/slo.go b/internal/server/slo.go index f78bc4f..0ec49c3 100644 --- a/internal/server/slo.go +++ b/internal/server/slo.go @@ -38,6 +38,10 @@ func (s *Server) handleSLO(w http.ResponseWriter, r *http.Request) { ctx, cancel := contextWithTimeout(r, 15*time.Second) defer cancel() + if err := s.refuseCanaryWrite(ctx, ns, release); err != nil { + writeStatus(w, err, http.StatusBadGateway) + return + } prof, err := s.Profile(ctx) if err != nil { writeError(w, http.StatusBadGateway, err.Error()) diff --git a/internal/server/status.go b/internal/server/status.go index c4b1cf4..5f5194e 100644 --- a/internal/server/status.go +++ b/internal/server/status.go @@ -53,9 +53,10 @@ type releaseStatus struct { // (llmPlanStatus); Service carries the operator's own words. Plan *planStatus `json:"planStatus,omitempty"` // Backend is helm or llmsvc when swiss manages the release. - Backend string `json:"backend,omitempty"` - CleanupPending bool `json:"cleanupPending,omitempty"` - Service *llmStatusView `json:"llmservice,omitempty"` + Backend string `json:"backend,omitempty"` + ManagedBy *llmsvc.CanaryOwner `json:"managedBy,omitempty"` + CleanupPending bool `json:"cleanupPending,omitempty"` + Service *llmStatusView `json:"llmservice,omitempty"` } // llmStatusView is the operator's status, in place of status.yaml. @@ -134,6 +135,7 @@ func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadGateway, err.Error()) return } + out.ManagedBy = llmsvc.CanaryOwnership(u) var obj *llmsvc.LLMService if decoded, err := llmsvc.FromUnstructured(u); err == nil { obj = decoded From f6611f5846652971fede1a612ea7674e3c0c9358 Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Thu, 29 Oct 2026 16:01:40 +0800 Subject: [PATCH 2/9] fix(llmsvc): include Canary system policy in read plans --- internal/llmsvc/canary_test.go | 21 +++++++++++++++++++++ internal/plan/plan.go | 2 +- internal/values/layers.go | 15 ++++++++------- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/internal/llmsvc/canary_test.go b/internal/llmsvc/canary_test.go index b7e1719..64825e0 100644 --- a/internal/llmsvc/canary_test.go +++ b/internal/llmsvc/canary_test.go @@ -37,3 +37,24 @@ func TestCanaryOwnershipProtectsIncompleteMetadata(t *testing.T) { }) } } + +func TestCanarySystemLayerIsEffectiveInReadPlan(t *testing.T) { + u, err := ToUnstructured(&LLMService{Spec: Spec{Chart: ChartSpec{Name: "vllm", Repo: "oci://example/charts", Version: "1.0.0"}, Layers: []Layer{ + {Name: "form", Values: map[string]any{"replicaCount": 1, "scaler": map[string]any{"enabled": true}, "modelRoute": map[string]any{"nginx": map[string]any{"route": "arm-name"}}}}, + {Name: "canary-system", Values: map[string]any{"scaler": map[string]any{"enabled": false}, "modelRoute": map[string]any{"nginx": map[string]any{"route": "production-route"}}}}, + }}}) + if err != nil { + t.Fatal(err) + } + p, err := ToPlan(u) + if err != nil { + t.Fatal(err) + } + v := p.Values() + if v["scaler"].(map[string]any)["enabled"] != false || v["modelRoute"].(map[string]any)["nginx"].(map[string]any)["route"] != "production-route" { + t.Fatalf("system policy ignored: %v", v) + } + if p.LayerOf("scaler.enabled") != "canary-system" { + t.Fatal("wrong provenance") + } +} diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 1fc0194..9550f01 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -243,7 +243,7 @@ func (p *Plan) YAML() ([]byte, error) { } // Layers are the values documents a release is applied through, in merge order. -var Layers = []string{values.LayerDefaults, values.LayerCatalog, values.LayerSite, values.LayerDerived, values.LayerForm, values.LayerEdit, values.LayerAutoscale} +var Layers = []string{values.LayerDefaults, values.LayerCatalog, values.LayerSite, values.LayerDerived, values.LayerForm, values.LayerEdit, values.LayerAutoscale, values.LayerCanarySystem} // LayerValues splits the composed values by the layer that set each path, so a // release is applied through one document per layer rather than one merged file. diff --git a/internal/values/layers.go b/internal/values/layers.go index c2bdb02..8e82e16 100644 --- a/internal/values/layers.go +++ b/internal/values/layers.go @@ -9,11 +9,12 @@ package values // not a veto but a report, so plan.Shadowed lists the values one layer set and a // later one overrode. const ( - LayerDefaults = "defaults" // the site's preferences; the variant overrides them - LayerCatalog = "catalog" // what makes this model run - LayerSite = "site" // what makes it work here - LayerDerived = "derived" // computed from catalog x site; a provenance label only - LayerForm = "form" // how much, where, how routed - LayerEdit = "edit" // typed into the plan editor - LayerAutoscale = "autoscale" // replicaCount from PD autoscaling, applied last + LayerDefaults = "defaults" // the site's preferences; the variant overrides them + LayerCatalog = "catalog" // what makes this model run + LayerSite = "site" // what makes it work here + LayerDerived = "derived" // computed from catalog x site; a provenance label only + LayerForm = "form" // how much, where, how routed + LayerEdit = "edit" // typed into the plan editor + LayerAutoscale = "autoscale" // replicaCount from PD autoscaling + LayerCanarySystem = "canary-system" // final policy imposed by the owning experiment ) From a2598a39d3bf473bf2aa326b904efb087bdfcfa2 Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Mon, 2 Nov 2026 11:56:01 +0800 Subject: [PATCH 3/9] feat(server): protect assigned Canary services and expose identities --- internal/llmsvc/canary.go | 75 +++++++++++++++++++++++++++++++++- internal/llmsvc/canary_test.go | 31 ++++++++++++++ internal/server/api.go | 17 ++++---- internal/server/autoscale.go | 8 ++++ internal/server/backend.go | 5 +++ internal/server/canary.go | 12 ++++++ internal/server/canary_test.go | 74 +++++++++++++++++++++++++++++++++ internal/server/status.go | 12 ++++-- 8 files changed, 222 insertions(+), 12 deletions(-) diff --git a/internal/llmsvc/canary.go b/internal/llmsvc/canary.go index d73429a..33796c8 100644 --- a/internal/llmsvc/canary.go +++ b/internal/llmsvc/canary.go @@ -1,12 +1,16 @@ package llmsvc import ( + "encoding/json" "strings" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -const CanaryPrefix = "canary.modelsphere.dev/" +const ( + CanaryPrefix = "canary.modelsphere.dev/" + AssignmentKey = CanaryPrefix + "assignment" +) type CanaryOwner struct { APIVersion string `json:"apiVersion"` @@ -17,6 +21,23 @@ type CanaryOwner struct { Verified bool `json:"verified"` } +type CanaryAssignment struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Namespace string `json:"namespace"` + Name string `json:"name"` + UID string `json:"uid"` + Role string `json:"role,omitempty"` + Arm string `json:"arm,omitempty"` + ConfigDigest string `json:"configDigest,omitempty"` + ConfigVersion string `json:"configVersion,omitempty"` + Verified bool `json:"verified"` +} + +func (a *CanaryAssignment) WriteBlocked() bool { + return a != nil && (!a.Verified || a.Role == "arm") +} + // CanaryOwnership keeps orphaned or inconsistent ownership protected too. func CanaryOwnership(u *unstructured.Unstructured) *CanaryOwner { if u == nil { @@ -39,3 +60,55 @@ func CanaryOwnership(u *unstructured.Unstructured) *CanaryOwner { } return nil } + +// CanaryAssigned parses canary.modelsphere.dev/assignment and keeps malformed +// or incomplete assignment metadata protected too. +func CanaryAssigned(u *unstructured.Unstructured) *CanaryAssignment { + if u == nil { + return nil + } + raw, ok := u.GetAnnotations()[AssignmentKey] + if !ok { + return nil + } + var a CanaryAssignment + if strings.TrimSpace(raw) == "" || json.Unmarshal([]byte(raw), &a) != nil { + return &CanaryAssignment{ + APIVersion: CanaryPrefix + "v1alpha1", + Kind: "LLMCanary", + Namespace: u.GetNamespace(), + Verified: false, + } + } + a.APIVersion = strings.TrimSpace(a.APIVersion) + a.Kind = strings.TrimSpace(a.Kind) + a.Namespace = strings.TrimSpace(a.Namespace) + a.Name = strings.TrimSpace(a.Name) + a.UID = strings.TrimSpace(a.UID) + a.Role = strings.TrimSpace(a.Role) + a.Arm = strings.TrimSpace(a.Arm) + a.ConfigDigest = strings.TrimSpace(a.ConfigDigest) + a.ConfigVersion = strings.TrimSpace(a.ConfigVersion) + + validRole := a.Role == "production" || a.Role == "arm" + validArm := (a.Role == "arm" && a.Arm != "") || (a.Role == "production" && a.Arm == "") + sameNS := a.Role == "production" || u.GetNamespace() == "" || a.Namespace == u.GetNamespace() + a.Verified = a.APIVersion == CanaryPrefix+"v1alpha1" && + a.Kind == "LLMCanary" && + a.Namespace != "" && + a.Name != "" && + a.UID != "" && + validRole && + validArm && + sameNS + if a.APIVersion == "" { + a.APIVersion = CanaryPrefix + "v1alpha1" + } + if a.Kind == "" { + a.Kind = "LLMCanary" + } + if a.Namespace == "" { + a.Namespace = u.GetNamespace() + } + return &a +} diff --git a/internal/llmsvc/canary_test.go b/internal/llmsvc/canary_test.go index 64825e0..2b7b5d3 100644 --- a/internal/llmsvc/canary_test.go +++ b/internal/llmsvc/canary_test.go @@ -58,3 +58,34 @@ func TestCanarySystemLayerIsEffectiveInReadPlan(t *testing.T) { t.Fatal("wrong provenance") } } + +func TestCanaryAssignedProtectsArmAndMalformedMetadata(t *testing.T) { + for _, tc := range []struct { + name string + raw string + present, verified, blocking bool + }{ + {"none", "", false, false, false}, + {"valid arm", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"arm","arm":"baseline","configDigest":"sha256:abc","configVersion":"7"}`, true, true, true}, + {"valid production", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"production"}`, true, true, false}, + {"empty annotation", " ", true, false, true}, + {"malformed json", "{not-json", true, false, true}, + {"missing uid", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","role":"arm","arm":"baseline"}`, true, false, true}, + {"arm without arm name", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"arm"}`, true, false, true}, + {"production with arm name", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"production","arm":"baseline"}`, true, false, true}, + {"cross namespace production", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"other","name":"demo","uid":"parent-uid","role":"production"}`, true, true, false}, + {"cross namespace", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"other","name":"demo","uid":"parent-uid","role":"arm","arm":"baseline"}`, true, false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + u := &unstructured.Unstructured{} + u.SetNamespace("canary") + if tc.present { + u.SetAnnotations(map[string]string{AssignmentKey: tc.raw}) + } + got := CanaryAssigned(u) + if (got != nil) != tc.present || (got != nil && got.Verified != tc.verified) || got.WriteBlocked() != tc.blocking { + t.Fatalf("got %+v, blocking=%v", got, got.WriteBlocked()) + } + }) + } +} diff --git a/internal/server/api.go b/internal/server/api.go index fabe7a5..00a958e 100644 --- a/internal/server/api.go +++ b/internal/server/api.go @@ -563,6 +563,7 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) { } type deployment struct { + UID string `json:"uid,omitempty"` Release string `json:"release"` Namespace string `json:"namespace"` Chart string `json:"chart,omitempty"` @@ -586,11 +587,12 @@ type deployment struct { Route string `json:"route,omitempty"` // Backend is helm or llmsvc. External is an LLMService swiss did not // write. CleanupPending means both records still exist. - Backend string `json:"backend"` - External bool `json:"external,omitempty"` - ManagedBy *llmsvc.CanaryOwner `json:"managedBy,omitempty"` - CleanupPending bool `json:"cleanupPending,omitempty"` - Deletion *llmDeletionView `json:"deletion,omitempty"` + Backend string `json:"backend"` + External bool `json:"external,omitempty"` + ManagedBy *llmsvc.CanaryOwner `json:"managedBy,omitempty"` + AssignedTo *llmsvc.CanaryAssignment `json:"assignedTo,omitempty"` + CleanupPending bool `json:"cleanupPending,omitempty"` + Deletion *llmDeletionView `json:"deletion,omitempty"` // What the plan recorded about its catalog, for finding the configured // catalog the release belongs to. @@ -670,8 +672,9 @@ func (s *Server) deploymentRefs(ctx context.Context) ([]deployRef, error) { func (s *Server) deploymentFromService(ctx context.Context, ref deployRef) *deployment { u := ref.obj d := &deployment{ - Release: u.GetName(), Namespace: u.GetNamespace(), - Backend: backendLLMSVC, External: llmsvc.IsExternal(u), CleanupPending: ref.cleanup, ManagedBy: llmsvc.CanaryOwnership(u), + Release: u.GetName(), Namespace: u.GetNamespace(), UID: string(u.GetUID()), + Backend: backendLLMSVC, External: llmsvc.IsExternal(u), CleanupPending: ref.cleanup, + ManagedBy: llmsvc.CanaryOwnership(u), AssignedTo: llmsvc.CanaryAssigned(u), } obj, err := llmsvc.FromUnstructured(u) if err != nil { diff --git a/internal/server/autoscale.go b/internal/server/autoscale.go index 82d5b69..34442b6 100644 --- a/internal/server/autoscale.go +++ b/internal/server/autoscale.go @@ -87,6 +87,10 @@ func (s *Server) putAutoscale(ctx context.Context, w http.ResponseWriter, r *htt writeError(w, http.StatusBadGateway, "no cluster writer") return } + if err := s.refuseCanaryWrite(ctx, ns, release); err != nil { + writeStatus(w, err, http.StatusBadGateway) + return + } b, _, err := s.backendFor(ctx, ns, release) if err != nil { writeStatus(w, err, http.StatusBadGateway) @@ -122,6 +126,10 @@ func (s *Server) deleteAutoscale(ctx context.Context, w http.ResponseWriter, ns, writeError(w, http.StatusBadGateway, "no cluster writer") return } + if err := s.refuseCanaryWrite(ctx, ns, release); err != nil { + writeStatus(w, err, http.StatusBadGateway) + return + } b, _, err := s.backendFor(ctx, ns, release) if err != nil { writeStatus(w, err, http.StatusBadGateway) diff --git a/internal/server/backend.go b/internal/server/backend.go index 858aa41..91c39ff 100644 --- a/internal/server/backend.go +++ b/internal/server/backend.go @@ -87,6 +87,11 @@ func writeStatus(w http.ResponseWriter, err error, fallback int) { writeJSON(w, http.StatusConflict, map[string]any{"error": managed.Error(), "code": "CANARY_MANAGED_SERVICE", "owner": managed.Owner}) return } + var assigned *canaryAssignedError + if errors.As(err, &assigned) { + writeJSON(w, http.StatusConflict, map[string]any{"error": assigned.Error(), "code": "CANARY_ASSIGNED_SERVICE", "assignedTo": assigned.Assignment}) + return + } code := fallback var hf *httpFailure if errors.As(err, &hf) { diff --git a/internal/server/canary.go b/internal/server/canary.go index 1df4a35..0738131 100644 --- a/internal/server/canary.go +++ b/internal/server/canary.go @@ -16,10 +16,22 @@ func (e *canaryManagedError) Error() string { return fmt.Sprintf("service is managed by LLMCanary %s/%s (UID %s); manage configuration and cleanup from the experiment", e.Owner.Namespace, e.Owner.Name, e.Owner.UID) } +type canaryAssignedError struct{ Assignment *llmsvc.CanaryAssignment } + +func (e *canaryAssignedError) Error() string { + if e.Assignment != nil && e.Assignment.Namespace != "" && e.Assignment.Name != "" { + return fmt.Sprintf("service is assigned to LLMCanary %s/%s (UID %s) as arm %s; release or delete the experiment before modifying this service", e.Assignment.Namespace, e.Assignment.Name, e.Assignment.UID, e.Assignment.Arm) + } + return "service has incomplete LLMCanary assignment metadata; refusal protects the experiment arm" +} + func refuseCanaryService(u *unstructured.Unstructured) error { if owner := llmsvc.CanaryOwnership(u); owner != nil { return &canaryManagedError{Owner: owner} } + if assigned := llmsvc.CanaryAssigned(u); assigned.WriteBlocked() { + return &canaryAssignedError{Assignment: assigned} + } return nil } diff --git a/internal/server/canary_test.go b/internal/server/canary_test.go index 64b8d7c..bce9a09 100644 --- a/internal/server/canary_test.go +++ b/internal/server/canary_test.go @@ -108,3 +108,77 @@ func TestCanaryArmReadAndWriteBoundaries(t *testing.T) { } } } + +func TestCanaryAssignedArmReadAndWriteBoundaries(t *testing.T) { + rig := startRig(t, llmProbe(), true, false, backendLLMSVC) + u := llmObject("models", "demo-existing-arm", map[string]string{ + llmsvc.AnnProfile: "prod", + llmsvc.AssignmentKey: `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"models","name":"demo","uid":"parent-uid","role":"arm","arm":"candidate","configDigest":"sha256:abc","configVersion":"3"}`, + }) + u.SetUID("arm-uid") + u.SetResourceVersion("19") + u.Object["status"] = map[string]any{"phase": "Applied", "helm": map[string]any{"revision": int64(1), "status": "deployed"}} + if _, err := rig.c.Create(t.Context(), u); err != nil { + t.Fatal(err) + } + for _, path := range []string{"/api/deployments", "/api/releases/models/demo-existing-arm/status"} { + code, body := get(t, rig.srv, path) + if code != 200 { + t.Fatalf("%s: %d %v", path, code, body) + } + if path == "/api/deployments" { + body = body["deployments"].([]any)[0].(map[string]any) + } + if body["uid"] != "arm-uid" { + t.Fatalf("missing service UID: %v", body) + } + assigned, _ := body["assignedTo"].(map[string]any) + if assigned["name"] != "demo" || assigned["uid"] != "parent-uid" || assigned["role"] != "arm" || assigned["arm"] != "candidate" || assigned["verified"] != true { + t.Fatalf("unexpected assignedTo: %v", body) + } + } + hash := rig.storePlan(t, map[string]any{"release": "demo-existing-arm"}) + rig.dyn.ClearActions() + checks := []struct { + path string + call func() (int, map[string]any) + }{ + {"apply", func() (int, map[string]any) { + return post(t, rig.srv, "/api/apply", map[string]any{"planHash": hash, "expectRevision": 1, "forceConflicts": true}) + }}, + {"install", func() (int, map[string]any) { + return post(t, rig.srv, "/api/install", map[string]any{"planHash": hash}) + }}, + {"rollback", func() (int, map[string]any) { + return post(t, rig.srv, "/api/releases/models/demo-existing-arm/rollback", map[string]any{"toRevision": 1, "expectRevision": 1}) + }}, + {"uninstall", func() (int, map[string]any) { return del(t, rig.srv, "/api/releases/models/demo-existing-arm") }}, + {"slo write", func() (int, map[string]any) { + return put(t, rig.srv, "/api/releases/models/demo-existing-arm/slo", map[string]any{"priority": 10}) + }}, + {"slo reset", func() (int, map[string]any) { return del(t, rig.srv, "/api/releases/models/demo-existing-arm/slo") }}, + {"autoscale write", func() (int, map[string]any) { + return put(t, rig.srv, "/api/releases/models/demo-existing-arm/autoscaling", map[string]any{"enabled": true, "minSlices": 1, "maxSlices": 2}) + }}, + {"autoscale delete", func() (int, map[string]any) { + return del(t, rig.srv, "/api/releases/models/demo-existing-arm/autoscaling") + }}, + } + for _, check := range checks { + t.Run(check.path, func(t *testing.T) { + code, body := check.call() + if code != http.StatusConflict || body["code"] != "CANARY_ASSIGNED_SERVICE" { + t.Fatalf("%d %v", code, body) + } + assigned, _ := body["assignedTo"].(map[string]any) + if assigned["uid"] != "parent-uid" || assigned["arm"] != "candidate" { + t.Fatal(body) + } + }) + } + for _, a := range rig.dyn.Actions() { + if a.GetVerb() == "delete" || a.GetVerb() == "update" || a.GetVerb() == "patch" || a.GetVerb() == "create" { + t.Fatalf("blocked write reached Kubernetes: %v", a) + } + } +} diff --git a/internal/server/status.go b/internal/server/status.go index 5f5194e..6298a21 100644 --- a/internal/server/status.go +++ b/internal/server/status.go @@ -22,6 +22,7 @@ import ( ) type releaseStatus struct { + UID string `json:"uid,omitempty"` Release string `json:"release"` Namespace string `json:"namespace"` Exists bool `json:"exists"` @@ -53,10 +54,11 @@ type releaseStatus struct { // (llmPlanStatus); Service carries the operator's own words. Plan *planStatus `json:"planStatus,omitempty"` // Backend is helm or llmsvc when swiss manages the release. - Backend string `json:"backend,omitempty"` - ManagedBy *llmsvc.CanaryOwner `json:"managedBy,omitempty"` - CleanupPending bool `json:"cleanupPending,omitempty"` - Service *llmStatusView `json:"llmservice,omitempty"` + Backend string `json:"backend,omitempty"` + ManagedBy *llmsvc.CanaryOwner `json:"managedBy,omitempty"` + AssignedTo *llmsvc.CanaryAssignment `json:"assignedTo,omitempty"` + CleanupPending bool `json:"cleanupPending,omitempty"` + Service *llmStatusView `json:"llmservice,omitempty"` } // llmStatusView is the operator's status, in place of status.yaml. @@ -135,7 +137,9 @@ func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadGateway, err.Error()) return } + out.UID = string(u.GetUID()) out.ManagedBy = llmsvc.CanaryOwnership(u) + out.AssignedTo = llmsvc.CanaryAssigned(u) var obj *llmsvc.LLMService if decoded, err := llmsvc.FromUnstructured(u); err == nil { obj = decoded From 932ed9f191e24de52fefcf9ba4fd5b025c6b789e Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Mon, 2 Nov 2026 14:23:38 +0800 Subject: [PATCH 4/9] fix(server): recover uninstall conflicts and preserve SLO read fallback --- README.md | 15 +++ docs/architecture.md | 9 ++ internal/llmsvc/canary.go | 6 +- internal/llmsvc/canary_test.go | 4 + internal/server/backend.go | 3 +- internal/server/backend_llm.go | 38 +++++-- internal/server/canary.go | 13 +++ internal/server/canary_review_test.go | 153 ++++++++++++++++++++++++++ internal/server/slo.go | 2 +- 9 files changed, 231 insertions(+), 12 deletions(-) create mode 100644 internal/server/canary_review_test.go diff --git a/README.md b/README.md index 4dc7b1b..3c7dd6b 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,21 @@ POST /api/plans /api/diff /api/apply /api/install (allowDeploy only) `applyWith` on the site profile chooses the backend for a new install: `helm` (the default when the field is omitted) or `llmsvc` when the LLMService CRD is served. A release already in the cluster stays on the backend that owns it. `llmsvc` requires the plan's `source.schema` to be v2; empty means v1 and is rejected with 400 and nothing is written. Helm accepts v1 and v2. The CLI `swiss install` and `swiss apply` stay on helm. See [docs/architecture.md](docs/architecture.md). +## Canary ownership checks + +Server writes verify LLMService ownership even for Helm releases. + +| LLMService access | Install, apply, rollback, uninstall, SLO writes | Reads, including SLO GET | +| --- | --- | --- | +| CRD absent or no LLMService client configured | Normal backend behavior | Normal fallback | +| Discovery fails or LLMServices access is Forbidden | HTTP 502; nothing written | Preserve read fallback | +| Canary-owned child or assigned arm observed | HTTP 409 with parent identity | List/status remain visible; SLO GET returns 409 | + +- This deliberately fails closed for writes: an invisible object cannot be distinguished from an experiment arm. Custom RBAC must grant LLMService reads even for Helm-only deployments when that API is present. The bundled chart includes this grant. +- Discovery failures and Forbidden results are cached for up to 30 seconds. Restore API access/RBAC and retry after that interval. +- Production-only assignments permit normal operations; malformed assignment or ownership metadata stays protected. +- See [concurrency limits](docs/architecture.md#canary-concurrency-limits) before adopting a live service. + ## Deploy CI publishes the image to `ghcr.io/modelsphere/swissd` and diff --git a/docs/architecture.md b/docs/architecture.md index bcf03e1..0948bcd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -71,3 +71,12 @@ form / flags ─┘ (hash) ↓ | `GET /api/nodes` | the GPU inventory: type, allocatable, in use, and what holds it | | `GET /api/profile` | the site profile as parsed, after defaults | | `GET /api/autoscaling`, `GET /api/releases/{ns}/{release}/autoscaling` | the autoscale loop's last pass per PD release with the form SLO on; in memory only, lost on restart | + +## Canary concurrency limits + +Ownership checks protect server operations but do not lock external controllers. + +- Apply and rollback recheck ownership inside the per-release apply lock; the rollback handler's earlier check is only an early rejection. +- Uninstall checks at the handler and again in the LLMService backend. Deletes retain UID and resourceVersion preconditions. A conflict triggers one fresh read, an identity/ownership recheck and one retry. Rejected conflicts, replacements or newly adopted arms restore the plan without recording a failed uninstall. +- The Helm uninstall handler's initial check and SLO checks are outside the apply lock. Helm/SLO upstream writes have no atomic LLMService ownership precondition. Adoption after a check can therefore overlap these operations; the local apply lock cannot serialize the Canary operator or another Swiss instance. +- Finish pending Swiss operations before assigning a service to an experiment. Atomic exclusion across controllers requires a shared admission/claim protocol. diff --git a/internal/llmsvc/canary.go b/internal/llmsvc/canary.go index 33796c8..967c7db 100644 --- a/internal/llmsvc/canary.go +++ b/internal/llmsvc/canary.go @@ -55,7 +55,11 @@ func CanaryOwnership(u *unstructured.Unstructured) *CanaryOwner { } for _, key := range []string{"canary-name", "canary-namespace", "canary-uid"} { if _, ok := labels[CanaryPrefix+key]; ok { - return &CanaryOwner{APIVersion: CanaryPrefix + "v1alpha1", Kind: "LLMCanary", Namespace: labels[CanaryPrefix+"canary-namespace"], Name: labels[CanaryPrefix+"canary-name"], UID: labels[CanaryPrefix+"canary-uid"]} + ns := labels[CanaryPrefix+"canary-namespace"] + if ns == "" { + ns = u.GetNamespace() + } + return &CanaryOwner{APIVersion: CanaryPrefix + "v1alpha1", Kind: "LLMCanary", Namespace: ns, Name: labels[CanaryPrefix+"canary-name"], UID: labels[CanaryPrefix+"canary-uid"]} } } return nil diff --git a/internal/llmsvc/canary_test.go b/internal/llmsvc/canary_test.go index 2b7b5d3..df1c31a 100644 --- a/internal/llmsvc/canary_test.go +++ b/internal/llmsvc/canary_test.go @@ -22,6 +22,7 @@ func TestCanaryOwnershipProtectsIncompleteMetadata(t *testing.T) { {"removed labels", []metav1.OwnerReference{ref}, nil, true, false}, {"removed reference", nil, labels, true, false}, {"different UID", []metav1.OwnerReference{ref}, map[string]string{CanaryPrefix + "canary-uid": "new-uid"}, true, false}, + {"missing namespace label", nil, map[string]string{CanaryPrefix + "canary-name": "demo"}, true, false}, {"empty marker", nil, map[string]string{CanaryPrefix + "canary-name": ""}, true, false}, {"unrelated owner", []metav1.OwnerReference{{APIVersion: "apps/v1", Kind: "Deployment", Name: "demo", UID: "old-uid"}}, nil, false, false}, } { @@ -31,6 +32,9 @@ func TestCanaryOwnershipProtectsIncompleteMetadata(t *testing.T) { u.SetOwnerReferences(tc.refs) u.SetLabels(tc.labels) owner := CanaryOwnership(u) + if owner != nil && owner.Namespace != "canary" { + t.Fatalf("owner namespace %q, want canary", owner.Namespace) + } if (owner != nil) != tc.protected || (owner != nil && owner.Verified != tc.verified) { t.Fatalf("%+v", owner) } diff --git a/internal/server/backend.go b/internal/server/backend.go index 20e2e7b..e9a6e17 100644 --- a/internal/server/backend.go +++ b/internal/server/backend.go @@ -143,7 +143,8 @@ type llmsvcAccess struct { } // visible is true when get/list may run. A discovery error and a cached -// Forbidden both hide the API, which is how a helm install keeps working. +// Forbidden both hide the API for reads. Writes separately require ownership +// verification, including when the selected backend is helm. func (a llmsvcAccess) visible() bool { return a.served && a.discErr == nil && !a.forbidden } diff --git a/internal/server/backend_llm.go b/internal/server/backend_llm.go index 78d5530..b922c3c 100644 --- a/internal/server/backend_llm.go +++ b/internal/server/backend_llm.go @@ -367,16 +367,39 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal return uninstallResult{}, fail(http.StatusInternalServerError, "plan not marked, nothing uninstalled: %s", err.Error()) } - started := time.Now() - delErr := s.llms.DeleteObserved(ctx, u) - if apierrors.IsNotFound(delErr) { + restoreRejected := func(cause error) (uninstallResult, error) { if hadPlan { - if rerr := s.restorePlan(ctx, ns, release, prev); rerr != nil { - s.log.ErrorContext(ctx, "plan not restored after delete NotFound", "namespace", ns, "release", release, "err", rerr) + if err := s.restorePlan(ctx, ns, release, prev); err != nil { + return uninstallResult{}, fail(http.StatusInternalServerError, "delete rejected (%s), plan not restored: %s", cause, err) } } s.forgetUninstalling(ns, release) - return uninstallResult{}, fail(http.StatusNotFound, "no release %s/%s", ns, release) + return uninstallResult{}, cause + } + + started := time.Now() + delErr := s.llms.DeleteObserved(ctx, u) + if apierrors.IsConflict(delErr) { + fresh, err := s.llms.Get(ctx, ns, release) + if apierrors.IsNotFound(err) { + return restoreRejected(fail(http.StatusNotFound, "no release %s/%s", ns, release)) + } + if err != nil { + return restoreRejected(failErr(http.StatusBadGateway, err)) + } + if fresh.GetUID() != u.GetUID() { + return restoreRejected(fail(http.StatusConflict, "service %s/%s was replaced; refresh before uninstalling", ns, release)) + } + if err := refuseCanaryService(fresh); err != nil { + return restoreRejected(err) + } + delErr = s.llms.DeleteObserved(ctx, fresh) + } + if apierrors.IsNotFound(delErr) { + return restoreRejected(fail(http.StatusNotFound, "no release %s/%s", ns, release)) + } + if apierrors.IsConflict(delErr) { + return restoreRejected(failErr(http.StatusConflict, delErr)) } out := llmDeleteRequested(ns, release) if delErr != nil { @@ -386,9 +409,6 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal s.log.ErrorContext(ctx, "plan not marked failed", "namespace", ns, "release", release, "err", ferr) } } - if apierrors.IsConflict(delErr) { - return uninstallResult{}, failErr(http.StatusConflict, delErr) - } return uninstallResult{}, failErr(http.StatusInternalServerError, delErr) } // Record before tracking. The object may already be gone, and a reap that diff --git a/internal/server/canary.go b/internal/server/canary.go index 0738131..4954765 100644 --- a/internal/server/canary.go +++ b/internal/server/canary.go @@ -36,8 +36,15 @@ func refuseCanaryService(u *unstructured.Unstructured) error { } func (s *Server) refuseCanaryWrite(ctx context.Context, ns, release string) error { + return s.refuseCanaryAccess(ctx, ns, release, true) +} + +func (s *Server) refuseCanaryAccess(ctx context.Context, ns, release string, write bool) error { access := s.llmsvcAccess(ctx) if access.discErr != nil || access.forbidden { + if !write { + return nil + } return fail(http.StatusBadGateway, "cannot verify service ownership: %s", access.why()) } if !access.served || s.llms == nil { @@ -45,6 +52,9 @@ func (s *Server) refuseCanaryWrite(ctx context.Context, ns, release string) erro } ns, err := s.resolveNamespace(ctx, ns, release) if err != nil { + if !write { + return nil + } return failErr(http.StatusBadGateway, err) } u, err := s.llms.Get(ctx, ns, release) @@ -55,6 +65,9 @@ func (s *Server) refuseCanaryWrite(ctx context.Context, ns, release string) erro if apierrors.IsForbidden(err) { s.noteLLMSVCForbidden() } + if !write { + return nil + } return fail(http.StatusBadGateway, "cannot verify service ownership: %s", err) } return refuseCanaryService(u) diff --git a/internal/server/canary_review_test.go b/internal/server/canary_review_test.go new file mode 100644 index 0000000..54599d7 --- /dev/null +++ b/internal/server/canary_review_test.go @@ -0,0 +1,153 @@ +package server + +import ( + "fmt" + "maps" + "net/http" + "testing" + + "github.com/modelsphere/swiss/internal/cluster" + "github.com/modelsphere/swiss/internal/llmsvc" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + k8stesting "k8s.io/client-go/testing" +) + +func TestLLMUninstallConflictRecovery(t *testing.T) { + for _, tc := range []struct { + name string + status, attempts int + code string + }{ + {"status update", 200, 2, ""}, + {"repeated conflict", 409, 2, ""}, + {"adopted", 409, 1, "CANARY_MANAGED_SERVICE"}, + {"assigned", 409, 1, "CANARY_ASSIGNED_SERVICE"}, + {"replacement", 409, 1, ""}, + {"gone", 404, 1, ""}, + {"read failure", 502, 1, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + probe := llmProbe() + labels := map[string]map[string]string{} + seedLLMPlan(t, &probe, labels, &uninstallCase{Namespace: "models", Release: "ordinary"}) + ref := planRef("models", "ordinary") + before := maps.Clone(probe.Maps[ref]) + rig := startRig(t, probe, true, false, backendLLMSVC) + base := rig.s.probe.(*cluster.Fake) + rig.s.SetWriter(&llmMirrorWriter{fakeWriter: rig.w, probe: base, labels: labels}) + u := llmObject("models", "ordinary", nil) + u.SetUID("original") + u.SetResourceVersion("17") + if _, err := rig.c.Create(t.Context(), u); err != nil { + t.Fatal(err) + } + attempts := 0 + gr := schema.GroupResource{Group: llmsvc.Group, Resource: llmsvc.Resource} + rig.dyn.PrependReactor("delete", "llmservices", func(a k8stesting.Action) (bool, runtime.Object, error) { + attempts++ + pre := a.(k8stesting.DeleteAction).GetDeleteOptions().Preconditions + rv := "17" + if attempts == 2 { + rv = "18" + } + if pre == nil || pre.UID == nil || *pre.UID != "original" || pre.ResourceVersion == nil || *pre.ResourceVersion != rv { + t.Errorf("attempt %d: unsafe delete preconditions %+v", attempts, pre) + } + if attempts > 1 && tc.name != "repeated conflict" { + return false, nil, nil + } + fresh := u.DeepCopy() + fresh.SetResourceVersion("18") + fresh.Object["status"] = map[string]any{"phase": "Applied", "observedGeneration": int64(1)} + switch tc.name { + case "adopted": + fresh.SetOwnerReferences(ownedArm().GetOwnerReferences()) + case "assigned": + fresh.SetAnnotations(map[string]string{llmsvc.AssignmentKey: `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"models","name":"demo","uid":"parent","role":"arm","arm":"baseline"}`}) + case "replacement": + fresh.SetUID("replacement") + case "gone": + if err := rig.dyn.Tracker().Delete(llmsvc.GVR, "models", "ordinary"); err != nil { + t.Error(err) + } + return true, nil, apierrors.NewConflict(gr, "ordinary", fmt.Errorf("stale")) + case "read failure": + rig.dyn.PrependReactor("get", "llmservices", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewForbidden(gr, "ordinary", fmt.Errorf("access revoked")) + }) + } + if err := rig.dyn.Tracker().Update(llmsvc.GVR, fresh, "models"); err != nil { + t.Error(err) + } + return true, nil, apierrors.NewConflict(gr, "ordinary", fmt.Errorf("stale")) + }) + code, body := del(t, rig.srv, "/api/releases/models/ordinary") + if code != tc.status || (tc.code != "" && body["code"] != tc.code) { + t.Fatalf("status %d: %v", code, body) + } + if attempts != tc.attempts { + t.Fatalf("delete attempts %d, want %d", attempts, tc.attempts) + } + if tc.status == 200 { + if runs := rig.runs(t); len(runs) != 1 || runs[0].Error != "" { + t.Fatalf("successful retry audit: %+v", runs) + } + return + } + if !maps.Equal(base.Maps[ref], before) { + t.Fatalf("plan not restored: %v", base.Maps[ref]) + } + if labels[ref][cluster.LabelPhase] != "" { + t.Fatalf("phase label not restored: %v", labels[ref]) + } + if runs := rig.runs(t); len(runs) != 0 { + t.Fatalf("rejected deletion recorded as uninstall: %+v", runs) + } + if rig.s.llmUninstalling("models", "ordinary") { + t.Fatal("rejected deletion still tracked") + } + }) + } +} + +func TestSLOReadFallbackWithInvisibleLLMService(t *testing.T) { + for _, kind := range []string{"forbidden", "discovery"} { + t.Run(kind, func(t *testing.T) { + var sent map[string]any + up := sloStub(t, true, true, &sent) + g := newGateServer(t, kind, backendHelm, failBin(t, "helm"), failBin(t, "helmfile")) + probe := llmProbe() + probe.Maps["swiss/site-profile"] = map[string]string{"profile.yaml": profileYAML + "\nscaler:\n sloAddress: " + up.URL + "\n sloTokenSecret: llm-scaler/slo-api\n"} + probe.Secrets["llm-scaler/slo-api"] = map[string]string{"token": "slo-secret"} + g.s.probe = probe + for range 2 { + code, body := get(t, g.srv, "/api/releases/models/glm/slo") + if code != http.StatusOK || body["found"] != true { + t.Fatalf("read fallback %d: %v", code, body) + } + } + for _, call := range []func() (int, map[string]any){ + func() (int, map[string]any) { + return put(t, g.srv, "/api/releases/models/glm/slo", map[string]any{"priority": 10}) + }, + func() (int, map[string]any) { return del(t, g.srv, "/api/releases/models/glm/slo") }, + func() (int, map[string]any) { return del(t, g.srv, "/api/releases/models/glm") }, + func() (int, map[string]any) { + return post(t, g.srv, "/api/releases/models/glm/rollback", map[string]any{"toRevision": 1, "expectRevision": 1}) + }, + } { + if code, body := call(); code != http.StatusBadGateway { + t.Fatalf("unverified write %d: %v", code, body) + } + } + if sent != nil || len(g.w.written) != 0 { + t.Fatal("blocked write reached SLO or cluster writer") + } + if g.calls.Load() != 1 { + t.Fatalf("API error not cached: %d calls", g.calls.Load()) + } + }) + } +} diff --git a/internal/server/slo.go b/internal/server/slo.go index 0ec49c3..fdf47e4 100644 --- a/internal/server/slo.go +++ b/internal/server/slo.go @@ -38,7 +38,7 @@ func (s *Server) handleSLO(w http.ResponseWriter, r *http.Request) { ctx, cancel := contextWithTimeout(r, 15*time.Second) defer cancel() - if err := s.refuseCanaryWrite(ctx, ns, release); err != nil { + if err := s.refuseCanaryAccess(ctx, ns, release, r.Method != http.MethodGet); err != nil { writeStatus(w, err, http.StatusBadGateway) return } From 8e472d6d8263fa5a548022c74b54fe54e23d092a Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Wed, 4 Nov 2026 19:20:53 +0800 Subject: [PATCH 5/9] feat(server): allow pinned adaptive upgrades for assigned Canary arms --- docs/design-adaptive-canary-upgrades.md | 9 ++ internal/llmsvc/adaptive.go | 67 +++++++++++++ internal/llmsvc/adaptive_test.go | 28 ++++++ internal/server/backend_llm.go | 2 +- internal/server/canary_adaptive.go | 121 ++++++++++++++++++++++++ internal/server/canary_adaptive_test.go | 74 +++++++++++++++ internal/server/deploy.go | 8 +- 7 files changed, 306 insertions(+), 3 deletions(-) create mode 100644 docs/design-adaptive-canary-upgrades.md create mode 100644 internal/llmsvc/adaptive.go create mode 100644 internal/llmsvc/adaptive_test.go create mode 100644 internal/server/canary_adaptive.go create mode 100644 internal/server/canary_adaptive_test.go diff --git a/docs/design-adaptive-canary-upgrades.md b/docs/design-adaptive-canary-upgrades.md new file mode 100644 index 0000000..dc95966 --- /dev/null +++ b/docs/design-adaptive-canary-upgrades.md @@ -0,0 +1,9 @@ +# Adaptive concurrency for assigned experiment arms + +Existing-service experiment arms permit only adaptive concurrency changes during upgrade. + +- `POST /api/plans` accepts `adaptiveOnly: true` with `fromRelease`, `namespace` and `overrides.modelRoute.nginx.values`. The values replace only `adaptive_cc` and `adaptive_cc_*` entries in the form layer. All source, chart, other layers and values remain pinned to the live plan; no catalog is resolved. +- Apply rechecks the live assignment and compares the proposed LLMService spec with adaptive form values removed. Install, rollback, scaling, SLO and uninstall retain their existing guards. +- Canary admission applies the same comparison and requires a controller-owned baseline before permitting changes. The controller captures it only after the original frozen digest matches, preserving existing experiments. +- Normalized evidence ignores adaptive route values and apply generation/hash/revision, retaining workload, identity, bindings, auth and all other configuration evidence. A tuning upgrade temporarily closes measurement windows while unapplied; it resumes the same experiment once ready. +- Console obtains fresh service status on upgrade, exposes only the adaptive controls for verified assigned arms, and blocks all other form changes. diff --git a/internal/llmsvc/adaptive.go b/internal/llmsvc/adaptive.go new file mode 100644 index 0000000..d74fa58 --- /dev/null +++ b/internal/llmsvc/adaptive.go @@ -0,0 +1,67 @@ +package llmsvc + +import ( + "k8s.io/apimachinery/pkg/runtime" + "strings" +) + +const AdaptiveBaselineKey = "canary.modelsphere.dev/adaptive-baseline" + +func AdaptiveKey(key string) bool { + return key == "adaptive_cc" || strings.HasPrefix(key, "adaptive_cc_") +} + +func WithoutAdaptiveValues(values map[string]any) map[string]any { + if values == nil { + return map[string]any{} + } + copy := runtime.DeepCopyJSONValue(values).(map[string]any) + stripAdaptive(copy, "modelRoute", "nginx", "values") + return copy +} + +func stripAdaptive(tree map[string]any, path ...string) { + if len(path) == 0 { + for key := range tree { + if AdaptiveKey(key) { + delete(tree, key) + } + } + return + } + child, ok := tree[path[0]].(map[string]any) + if !ok { + return + } + stripAdaptive(child, path[1:]...) + if len(child) == 0 { + delete(tree, path[0]) + } +} + +func WithoutAdaptiveSpec(spec map[string]any) map[string]any { + copy := runtime.DeepCopyJSONValue(spec).(map[string]any) + layers, ok := copy["layers"].([]any) + if !ok { + return copy + } + result := make([]any, 0, len(layers)) + for _, raw := range layers { + layer, ok := raw.(map[string]any) + if !ok { + result = append(result, raw) + continue + } + if layer["name"] == "form" { + if values, ok := layer["values"].(map[string]any); ok { + layer["values"] = WithoutAdaptiveValues(values) + if len(layer["values"].(map[string]any)) == 0 { + continue + } + } + } + result = append(result, layer) + } + copy["layers"] = result + return copy +} diff --git a/internal/llmsvc/adaptive_test.go b/internal/llmsvc/adaptive_test.go new file mode 100644 index 0000000..9ed3a0e --- /dev/null +++ b/internal/llmsvc/adaptive_test.go @@ -0,0 +1,28 @@ +package llmsvc + +import ( + "reflect" + "testing" +) + +func TestAdaptiveSpecScope(t *testing.T) { + before := map[string]any{"chart": map[string]any{"version": "1"}, "layers": []any{map[string]any{"name": "catalog", "values": map[string]any{"model": map[string]any{"gpus": int64(1)}}}}} + after := map[string]any{"chart": map[string]any{"version": "1"}, "layers": []any{map[string]any{"name": "catalog", "values": map[string]any{"model": map[string]any{"gpus": int64(1)}}}, map[string]any{"name": "form", "values": map[string]any{"modelRoute": map[string]any{"nginx": map[string]any{"values": map[string]any{"adaptive_cc": "false", "adaptive_cc_ttl": "30"}}}}}}} + if !reflect.DeepEqual(WithoutAdaptiveSpec(before), WithoutAdaptiveSpec(after)) { + t.Fatal("adaptive-only change is blocked") + } + form := after["layers"].([]any)[1].(map[string]any)["values"].(map[string]any) + route := form["modelRoute"].(map[string]any)["nginx"].(map[string]any)["values"].(map[string]any) + for _, key := range []string{"default_max", "expose_routed_peer", "adaptive_cc_other_path"} { + if key == "adaptive_cc_other_path" { + form[key] = "bad" + } else { + route[key] = "bad" + } + if reflect.DeepEqual(WithoutAdaptiveSpec(before), WithoutAdaptiveSpec(after)) { + t.Fatalf("allowed unrelated change %s", key) + } + delete(form, key) + delete(route, key) + } +} diff --git a/internal/server/backend_llm.go b/internal/server/backend_llm.go index b922c3c..402f453 100644 --- a/internal/server/backend_llm.go +++ b/internal/server/backend_llm.go @@ -184,7 +184,7 @@ func (b llmBackend) Apply(ctx context.Context, p *plan.Plan, mode exec.Mode, o a if err != nil { return applyResult{}, failErr(http.StatusBadGateway, err) } - if err := refuseCanaryService(cur); err != nil { + if err := refuseCanaryPlan(cur, p, mode, o.Action); err != nil { return applyResult{}, err } if llmsvc.IsExternal(cur) { diff --git a/internal/server/canary_adaptive.go b/internal/server/canary_adaptive.go new file mode 100644 index 0000000..eaa047d --- /dev/null +++ b/internal/server/canary_adaptive.go @@ -0,0 +1,121 @@ +package server + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "github.com/modelsphere/swiss/internal/exec" + "github.com/modelsphere/swiss/internal/llmsvc" + "github.com/modelsphere/swiss/internal/plan" + "github.com/modelsphere/swiss/internal/values" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "net/http" + "reflect" +) + +func (s *Server) composeAssignedAdaptive(ctx context.Context, req planRequest) (*plan.Plan, error) { + if req.FromRelease == "" || req.Namespace == "" || s.llms == nil { + return nil, fmt.Errorf("adaptive upgrade requires an existing LLMService") + } + u, err := s.llms.Get(ctx, req.Namespace, req.FromRelease) + if err != nil { + return nil, err + } + a := llmsvc.CanaryAssigned(u) + if a == nil || !a.Verified || a.Role != "arm" || llmsvc.CanaryOwnership(u) != nil { + return nil, fmt.Errorf("adaptive upgrade requires a verified assigned arm") + } + if req.OverridesYAML != "" || req.EditsYAML != "" || req.Model != "" || req.Version != "" || req.Variant != "" || req.Catalog != "" || req.ChartVersion != "" || req.Release != "" || req.ServiceID != "" || req.LocalPath != "" || len(req.GPUProducts) != 0 || req.ForceGPUProducts || req.CreateNamespace { + return nil, fmt.Errorf("assigned arms permit only adaptive concurrency changes") + } + if len(llmsvc.WithoutAdaptiveValues(map[string]any(req.Overrides))) != 0 { + return nil, fmt.Errorf("assigned arms permit only adaptive concurrency changes") + } + requested, _, err := unstructured.NestedMap(map[string]any(req.Overrides), "modelRoute", "nginx", "values") + if err != nil || len(requested) == 0 { + return nil, fmt.Errorf("adaptive concurrency values are required") + } + p, err := llmsvc.ToPlan(u) + if err != nil { + return nil, err + } + p = clonePlan(p) + form := llmsvc.WithoutAdaptiveValues(map[string]any(p.Layers[values.LayerForm])) + if err = unstructured.SetNestedMap(form, requested, "modelRoute", "nginx", "values"); err != nil { + return nil, err + } + // Restore every non-adaptive route value alongside the replacement. + original, _, _ := unstructured.NestedMap(map[string]any(p.Layers[values.LayerForm]), "modelRoute", "nginx", "values") + for key, value := range original { + if !llmsvc.AdaptiveKey(key) { + requested[key] = value + } + } + if err = unstructured.SetNestedMap(form, requested, "modelRoute", "nginx", "values"); err != nil { + return nil, err + } + p.Layers[values.LayerForm] = values.Tree(form) + if err = p.ComputeHash(); err != nil { + return nil, err + } + p.Helmfile, err = p.RenderHelmfile("") + return p, err +} + +func refuseCanaryPlan(u *unstructured.Unstructured, p *plan.Plan, mode exec.Mode, action string) error { + err := refuseCanaryService(u) + if err == nil { + return nil + } + a := llmsvc.CanaryAssigned(u) + if llmsvc.CanaryOwnership(u) != nil || a == nil || !a.Verified || a.Role != "arm" || mode != exec.Upgrade || action != "apply" { + return err + } + next, mapErr := llmsvc.FromPlan(p, action, "") + if mapErr != nil { + return mapErr + } + before, _, _ := unstructured.NestedMap(u.Object, "spec") + after, _, _ := unstructured.NestedMap(next.Object, "spec") + if !sameCanarySpec(llmsvc.WithoutAdaptiveSpec(before), llmsvc.WithoutAdaptiveSpec(after)) { + return err + } + current, mapErr := llmsvc.ToPlan(u) + if mapErr != nil { + return mapErr + } + if !reflect.DeepEqual(current.Source, p.Source) || current.Profile != p.Profile { + return err + } + if u.GetAnnotations()[llmsvc.AdaptiveBaselineKey] == "" { + return fail(http.StatusConflict, "experiment adaptive baseline is not ready; retry after the Canary controller reconciles") + } + return nil +} + +func (s *Server) refuseCanaryPlanWrite(ctx context.Context, p *plan.Plan, mode exec.Mode, action string) error { + err := s.refuseCanaryWrite(ctx, p.Release.Namespace, p.Release.Name) + if err == nil { + return nil + } + var assigned *canaryAssignedError + if !errors.As(err, &assigned) || s.llms == nil { + return err + } + u, getErr := s.llms.Get(ctx, p.Release.Namespace, p.Release.Name) + if getErr != nil { + return err + } + return refuseCanaryPlan(u, p, mode, action) +} + +func sameCanarySpec(before, after map[string]any) bool { + a, err := json.Marshal(before) + if err != nil { + return false + } + b, err := json.Marshal(after) + return err == nil && bytes.Equal(a, b) +} diff --git a/internal/server/canary_adaptive_test.go b/internal/server/canary_adaptive_test.go new file mode 100644 index 0000000..d297334 --- /dev/null +++ b/internal/server/canary_adaptive_test.go @@ -0,0 +1,74 @@ +package server + +import ( + "encoding/json" + "github.com/modelsphere/swiss/internal/exec" + "github.com/modelsphere/swiss/internal/llmsvc" + "github.com/modelsphere/swiss/internal/values" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "reflect" + "testing" +) + +func TestAssignedAdaptiveUpgradePinsEverythingElse(t *testing.T) { + rig := startRig(t, llmProbe(), true, false, backendLLMSVC) + hash := rig.storePlan(t, map[string]any{"release": "adaptive-arm"}) + p, err := rig.s.store.Plan(t.Context(), hash) + if err != nil { + t.Fatal(err) + } + p.Layers[values.LayerForm] = values.Tree{"schedulerName": "retained", "modelRoute": map[string]any{"nginx": map[string]any{"values": map[string]any{"adaptive_cc": "true", "adaptive_cc_min_frac": "0.4", "default_max": "42"}}}} + if err = p.ComputeHash(); err != nil { + t.Fatal(err) + } + u, err := llmsvc.FromPlan(p, "apply", "") + if err != nil { + t.Fatal(err) + } + raw, err := json.Marshal(u) + if err != nil { + t.Fatal(err) + } + wire, _, err := unstructured.UnstructuredJSONScheme.Decode(raw, nil, nil) + if err != nil { + t.Fatal(err) + } + u = wire.(*unstructured.Unstructured) + anns := u.GetAnnotations() + anns[llmsvc.AssignmentKey] = `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"models","name":"experiment","uid":"parent","role":"arm","arm":"baseline"}` + anns[llmsvc.AdaptiveBaselineKey] = "ready" + u.SetAnnotations(anns) + if _, err = rig.c.Create(t.Context(), u); err != nil { + t.Fatal(err) + } + req := planRequest{AdaptiveOnly: true, Namespace: "models", FromRelease: "adaptive-arm", Overrides: values.Tree{"modelRoute": map[string]any{"nginx": map[string]any{"values": map[string]any{"adaptive_cc": "false"}}}}} + tuned, err := rig.s.compose(t.Context(), req) + if err != nil { + t.Fatal(err) + } + if tuned.Source != p.Source || tuned.Chart != p.Chart || tuned.Profile != p.Profile { + t.Fatal("upgrade moved a frozen source") + } + if err = refuseCanaryPlan(u, tuned, exec.Upgrade, "apply"); err != nil { + t.Fatal(err) + } + if tuned.Values()["schedulerName"] != "retained" { + t.Fatal("lost scheduler") + } + if !reflect.DeepEqual(llmsvc.WithoutAdaptiveValues(map[string]any(p.Layers[values.LayerForm])), llmsvc.WithoutAdaptiveValues(map[string]any(tuned.Layers[values.LayerForm]))) { + t.Fatal("changed non-adaptive values") + } + for _, action := range []string{"rollback", "scale", "install"} { + if refuseCanaryPlan(u, tuned, exec.Upgrade, action) == nil { + t.Fatalf("allowed %s", action) + } + } + tuned.Layers[values.LayerForm]["replicaCount"] = int64(3) + if refuseCanaryPlan(u, tuned, exec.Upgrade, "apply") == nil { + t.Fatal("allowed scaling") + } + req.Overrides["replicaCount"] = int64(3) + if _, err = rig.s.compose(t.Context(), req); err == nil { + t.Fatal("accepted non-adaptive request") + } +} diff --git a/internal/server/deploy.go b/internal/server/deploy.go index bff0b56..3c77b5a 100644 --- a/internal/server/deploy.go +++ b/internal/server/deploy.go @@ -28,7 +28,8 @@ import ( type planRequest struct { // FromRelease recomposes a deployed release: its stored plan supplies the // form layer, the model and the variant, and only the catalog layer moves. - FromRelease string `json:"fromRelease,omitempty"` + FromRelease string `json:"fromRelease,omitempty"` + AdaptiveOnly bool `json:"adaptiveOnly,omitempty"` // Catalog names the configured catalog to compose from. Required when the // site lists several. An upgrade naming none stays on the one its release // came from; naming another moves the release there, same model id. @@ -166,6 +167,9 @@ func (s *Server) handlePlan(w http.ResponseWriter, r *http.Request) { } func (s *Server) compose(ctx context.Context, req planRequest) (*plan.Plan, error) { + if req.AdaptiveOnly { + return s.composeAssignedAdaptive(ctx, req) + } if req.FromRelease != "" { var err error if req, err = s.carryForward(ctx, req); err != nil { @@ -695,7 +699,7 @@ func (s *Server) apply(ctx context.Context, p *plan.Plan, mode exec.Mode, applyW // marker is written, so an apply cannot write its applying phase over the // marker and an uninstall cannot start in the middle of an apply. func (s *Server) applyLocked(ctx context.Context, p *plan.Plan, mode exec.Mode, applyWith string, o applyOpts) (applyResult, error) { - if err := s.refuseCanaryWrite(ctx, p.Release.Namespace, p.Release.Name); err != nil { + if err := s.refuseCanaryPlanWrite(ctx, p, mode, o.Action); err != nil { return applyResult{}, err } if err := s.refuseUninstalling(ctx, p.Release.Namespace, p.Release.Name); err != nil { From de235e89d6eca316693243776d013325aa770643 Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Thu, 5 Nov 2026 15:26:22 +0800 Subject: [PATCH 6/9] fix(server): leave Canary service protection to Console --- README.md | 15 +-- docs/architecture.md | 13 +- docs/design-adaptive-canary-upgrades.md | 13 +- internal/llmsvc/adaptive.go | 29 ----- internal/llmsvc/adaptive_test.go | 28 ---- internal/llmsvc/canary.go | 9 +- internal/llmsvc/canary_test.go | 34 ++--- internal/server/autoscale_loop_test.go | 9 +- internal/server/backend.go | 10 -- internal/server/backend_llm.go | 9 -- internal/server/backend_test.go | 41 ++++-- internal/server/canary.go | 74 ----------- internal/server/canary_adaptive.go | 72 +---------- internal/server/canary_adaptive_test.go | 19 +-- internal/server/canary_review_test.go | 45 +------ internal/server/canary_test.go | 164 +++++++++++------------- internal/server/deploy.go | 9 +- internal/server/rollback.go | 4 - internal/server/slo.go | 4 - 19 files changed, 144 insertions(+), 457 deletions(-) delete mode 100644 internal/llmsvc/adaptive_test.go delete mode 100644 internal/server/canary.go diff --git a/README.md b/README.md index 3c7dd6b..d5177c3 100644 --- a/README.md +++ b/README.md @@ -70,20 +70,9 @@ POST /api/plans /api/diff /api/apply /api/install (allowDeploy only) `applyWith` on the site profile chooses the backend for a new install: `helm` (the default when the field is omitted) or `llmsvc` when the LLMService CRD is served. A release already in the cluster stays on the backend that owns it. `llmsvc` requires the plan's `source.schema` to be v2; empty means v1 and is rejected with 400 and nothing is written. Helm accepts v1 and v2. The CLI `swiss install` and `swiss apply` stay on helm. See [docs/architecture.md](docs/architecture.md). -## Canary ownership checks +## Canary service metadata -Server writes verify LLMService ownership even for Helm releases. - -| LLMService access | Install, apply, rollback, uninstall, SLO writes | Reads, including SLO GET | -| --- | --- | --- | -| CRD absent or no LLMService client configured | Normal backend behavior | Normal fallback | -| Discovery fails or LLMServices access is Forbidden | HTTP 502; nothing written | Preserve read fallback | -| Canary-owned child or assigned arm observed | HTTP 409 with parent identity | List/status remain visible; SLO GET returns 409 | - -- This deliberately fails closed for writes: an invisible object cannot be distinguished from an experiment arm. Custom RBAC must grant LLMService reads even for Helm-only deployments when that API is present. The bundled chart includes this grant. -- Discovery failures and Forbidden results are cached for up to 30 seconds. Restore API access/RBAC and retry after that interval. -- Production-only assignments permit normal operations; malformed assignment or ownership metadata stays protected. -- See [concurrency limits](docs/architecture.md#canary-concurrency-limits) before adopting a live service. +List/status includes service UIDs and `managedBy` / `assignedTo` identities for Console. Console controls experiment-service actions; Swiss applies normal backend rules to these services without Canary-specific write or SLO restrictions. ## Deploy diff --git a/docs/architecture.md b/docs/architecture.md index 49454bd..9f7618e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -112,11 +112,10 @@ form / flags ─┘ (hash) ↓ | `GET /api/profile` | the site profile as parsed, after defaults | | `GET /api/autoscaling`, `GET /api/releases/{ns}/{release}/autoscaling` | the autoscale loop's last pass per PD release with the form SLO on; in memory only, lost on restart | -## Canary concurrency limits +## Canary service operations -Ownership checks protect server operations but do not lock external controllers. - -- Apply and rollback recheck ownership inside the per-release apply lock; the rollback handler's earlier check is only an early rejection. -- Uninstall checks at the handler and again in the LLMService backend. Deletes retain UID and resourceVersion preconditions. A conflict triggers one fresh read, an identity/ownership recheck and one retry. Rejected conflicts, replacements or newly adopted arms restore the plan without recording a failed uninstall. -- The Helm uninstall handler's initial check and SLO checks are outside the apply lock. Helm/SLO upstream writes have no atomic LLMService ownership precondition. Adoption after a check can therefore overlap these operations; the local apply lock cannot serialize the Canary operator or another Swiss instance. -- Finish pending Swiss operations before assigning a service to an experiment. Atomic exclusion across controllers requires a shared admission/claim protocol. +- List/status reports ownership and assignment metadata for Console. +- Console controls experiment-service actions. Swiss has no Canary-specific guard for apply, install, rollback, scale, uninstall, or SLO access. +- `adaptiveOnly` is an optional plan composition mode that pins the live source and other values. It requires neither a Canary assignment nor a controller baseline; normal plan requests remain available. +- LLMService deletion retains UID/resourceVersion preconditions and conflict recovery to avoid deleting a replacement object. +- Kubernetes RBAC and any separately enabled admission webhook still apply to API writes. diff --git a/docs/design-adaptive-canary-upgrades.md b/docs/design-adaptive-canary-upgrades.md index dc95966..bdcfb34 100644 --- a/docs/design-adaptive-canary-upgrades.md +++ b/docs/design-adaptive-canary-upgrades.md @@ -1,9 +1,8 @@ -# Adaptive concurrency for assigned experiment arms +# Adaptive concurrency plan composition -Existing-service experiment arms permit only adaptive concurrency changes during upgrade. +`POST /api/plans` accepts `adaptiveOnly: true` with `fromRelease`, `namespace` and `overrides.modelRoute.nginx.values`. -- `POST /api/plans` accepts `adaptiveOnly: true` with `fromRelease`, `namespace` and `overrides.modelRoute.nginx.values`. The values replace only `adaptive_cc` and `adaptive_cc_*` entries in the form layer. All source, chart, other layers and values remain pinned to the live plan; no catalog is resolved. -- Apply rechecks the live assignment and compares the proposed LLMService spec with adaptive form values removed. Install, rollback, scaling, SLO and uninstall retain their existing guards. -- Canary admission applies the same comparison and requires a controller-owned baseline before permitting changes. The controller captures it only after the original frozen digest matches, preserving existing experiments. -- Normalized evidence ignores adaptive route values and apply generation/hash/revision, retaining workload, identity, bindings, auth and all other configuration evidence. A tuning upgrade temporarily closes measurement windows while unapplied; it resumes the same experiment once ready. -- Console obtains fresh service status on upgrade, exposes only the adaptive controls for verified assigned arms, and blocks all other form changes. +- Composition starts from the live LLMService plan, replaces only `adaptive_cc` / `adaptive_cc_*` form values, and pins source, chart, other layers and values without resolving catalogs again. +- This optional mode works without a Canary assignment or controller-owned baseline. Normal compose and apply operations have no Canary-specific restrictions. +- List/status supplies ownership and assignment metadata for Console, which controls experiment-service actions. +- An independently enabled operator admission webhook can enforce additional rules on Kubernetes writes. diff --git a/internal/llmsvc/adaptive.go b/internal/llmsvc/adaptive.go index d74fa58..2718848 100644 --- a/internal/llmsvc/adaptive.go +++ b/internal/llmsvc/adaptive.go @@ -5,8 +5,6 @@ import ( "strings" ) -const AdaptiveBaselineKey = "canary.modelsphere.dev/adaptive-baseline" - func AdaptiveKey(key string) bool { return key == "adaptive_cc" || strings.HasPrefix(key, "adaptive_cc_") } @@ -38,30 +36,3 @@ func stripAdaptive(tree map[string]any, path ...string) { delete(tree, path[0]) } } - -func WithoutAdaptiveSpec(spec map[string]any) map[string]any { - copy := runtime.DeepCopyJSONValue(spec).(map[string]any) - layers, ok := copy["layers"].([]any) - if !ok { - return copy - } - result := make([]any, 0, len(layers)) - for _, raw := range layers { - layer, ok := raw.(map[string]any) - if !ok { - result = append(result, raw) - continue - } - if layer["name"] == "form" { - if values, ok := layer["values"].(map[string]any); ok { - layer["values"] = WithoutAdaptiveValues(values) - if len(layer["values"].(map[string]any)) == 0 { - continue - } - } - } - result = append(result, layer) - } - copy["layers"] = result - return copy -} diff --git a/internal/llmsvc/adaptive_test.go b/internal/llmsvc/adaptive_test.go deleted file mode 100644 index 9ed3a0e..0000000 --- a/internal/llmsvc/adaptive_test.go +++ /dev/null @@ -1,28 +0,0 @@ -package llmsvc - -import ( - "reflect" - "testing" -) - -func TestAdaptiveSpecScope(t *testing.T) { - before := map[string]any{"chart": map[string]any{"version": "1"}, "layers": []any{map[string]any{"name": "catalog", "values": map[string]any{"model": map[string]any{"gpus": int64(1)}}}}} - after := map[string]any{"chart": map[string]any{"version": "1"}, "layers": []any{map[string]any{"name": "catalog", "values": map[string]any{"model": map[string]any{"gpus": int64(1)}}}, map[string]any{"name": "form", "values": map[string]any{"modelRoute": map[string]any{"nginx": map[string]any{"values": map[string]any{"adaptive_cc": "false", "adaptive_cc_ttl": "30"}}}}}}} - if !reflect.DeepEqual(WithoutAdaptiveSpec(before), WithoutAdaptiveSpec(after)) { - t.Fatal("adaptive-only change is blocked") - } - form := after["layers"].([]any)[1].(map[string]any)["values"].(map[string]any) - route := form["modelRoute"].(map[string]any)["nginx"].(map[string]any)["values"].(map[string]any) - for _, key := range []string{"default_max", "expose_routed_peer", "adaptive_cc_other_path"} { - if key == "adaptive_cc_other_path" { - form[key] = "bad" - } else { - route[key] = "bad" - } - if reflect.DeepEqual(WithoutAdaptiveSpec(before), WithoutAdaptiveSpec(after)) { - t.Fatalf("allowed unrelated change %s", key) - } - delete(form, key) - delete(route, key) - } -} diff --git a/internal/llmsvc/canary.go b/internal/llmsvc/canary.go index 967c7db..60813f3 100644 --- a/internal/llmsvc/canary.go +++ b/internal/llmsvc/canary.go @@ -34,11 +34,7 @@ type CanaryAssignment struct { Verified bool `json:"verified"` } -func (a *CanaryAssignment) WriteBlocked() bool { - return a != nil && (!a.Verified || a.Role == "arm") -} - -// CanaryOwnership keeps orphaned or inconsistent ownership protected too. +// CanaryOwnership reports incomplete ownership metadata with Verified=false. func CanaryOwnership(u *unstructured.Unstructured) *CanaryOwner { if u == nil { return nil @@ -65,8 +61,7 @@ func CanaryOwnership(u *unstructured.Unstructured) *CanaryOwner { return nil } -// CanaryAssigned parses canary.modelsphere.dev/assignment and keeps malformed -// or incomplete assignment metadata protected too. +// CanaryAssigned reports malformed or incomplete assignments with Verified=false. func CanaryAssigned(u *unstructured.Unstructured) *CanaryAssignment { if u == nil { return nil diff --git a/internal/llmsvc/canary_test.go b/internal/llmsvc/canary_test.go index df1c31a..2352bfc 100644 --- a/internal/llmsvc/canary_test.go +++ b/internal/llmsvc/canary_test.go @@ -7,7 +7,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func TestCanaryOwnershipProtectsIncompleteMetadata(t *testing.T) { +func TestCanaryOwnershipReportsIncompleteMetadata(t *testing.T) { controller := true ref := metav1.OwnerReference{APIVersion: "canary.modelsphere.dev/v1alpha1", Kind: "LLMCanary", Name: "demo", UID: "old-uid", Controller: &controller} labels := map[string]string{CanaryPrefix + "canary-name": "demo", CanaryPrefix + "canary-namespace": "canary", CanaryPrefix + "canary-uid": "old-uid"} @@ -63,22 +63,22 @@ func TestCanarySystemLayerIsEffectiveInReadPlan(t *testing.T) { } } -func TestCanaryAssignedProtectsArmAndMalformedMetadata(t *testing.T) { +func TestCanaryAssignedReportsArmAndMalformedMetadata(t *testing.T) { for _, tc := range []struct { - name string - raw string - present, verified, blocking bool + name string + raw string + present, verified bool }{ - {"none", "", false, false, false}, - {"valid arm", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"arm","arm":"baseline","configDigest":"sha256:abc","configVersion":"7"}`, true, true, true}, - {"valid production", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"production"}`, true, true, false}, - {"empty annotation", " ", true, false, true}, - {"malformed json", "{not-json", true, false, true}, - {"missing uid", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","role":"arm","arm":"baseline"}`, true, false, true}, - {"arm without arm name", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"arm"}`, true, false, true}, - {"production with arm name", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"production","arm":"baseline"}`, true, false, true}, - {"cross namespace production", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"other","name":"demo","uid":"parent-uid","role":"production"}`, true, true, false}, - {"cross namespace", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"other","name":"demo","uid":"parent-uid","role":"arm","arm":"baseline"}`, true, false, true}, + {"none", "", false, false}, + {"valid arm", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"arm","arm":"baseline","configDigest":"sha256:abc","configVersion":"7"}`, true, true}, + {"valid production", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"production"}`, true, true}, + {"empty annotation", " ", true, false}, + {"malformed json", "{not-json", true, false}, + {"missing uid", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","role":"arm","arm":"baseline"}`, true, false}, + {"arm without arm name", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"arm"}`, true, false}, + {"production with arm name", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"canary","name":"demo","uid":"parent-uid","role":"production","arm":"baseline"}`, true, false}, + {"cross namespace production", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"other","name":"demo","uid":"parent-uid","role":"production"}`, true, true}, + {"cross namespace", `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"other","name":"demo","uid":"parent-uid","role":"arm","arm":"baseline"}`, true, false}, } { t.Run(tc.name, func(t *testing.T) { u := &unstructured.Unstructured{} @@ -87,8 +87,8 @@ func TestCanaryAssignedProtectsArmAndMalformedMetadata(t *testing.T) { u.SetAnnotations(map[string]string{AssignmentKey: tc.raw}) } got := CanaryAssigned(u) - if (got != nil) != tc.present || (got != nil && got.Verified != tc.verified) || got.WriteBlocked() != tc.blocking { - t.Fatalf("got %+v, blocking=%v", got, got.WriteBlocked()) + if (got != nil) != tc.present || (got != nil && got.Verified != tc.verified) { + t.Fatalf("got %+v", got) } }) } diff --git a/internal/server/autoscale_loop_test.go b/internal/server/autoscale_loop_test.go index ee1dd6f..82f4817 100644 --- a/internal/server/autoscale_loop_test.go +++ b/internal/server/autoscale_loop_test.go @@ -194,7 +194,7 @@ func runLLMLoop(t *testing.T, tc loopCase) { return true, nil, err } copied := obj.(*unstructured.Unstructured).DeepCopy() - if n >= 7 { + if n >= 6 { helm, _, _ := unstructured.NestedMap(copied.Object, "status", "helm") if helm == nil { helm = map[string]any{} @@ -219,9 +219,10 @@ func runLLMLoop(t *testing.T, tc loopCase) { } assertLoopLog(t, logs, tc) if tc.Conflict { - // applyLocked checks Canary ownership and deletion before applying. - if gotGets != 9 { - t.Errorf("Get calls %d, want 9", gotGets) + // One more than the loop alone: applyLocked reads the LLMService + // once to refuse a release that is being deleted. + if gotGets != 8 { + t.Errorf("Get calls %d, want 8", gotGets) } if got.GetAnnotations()[llmsvc.AnnAction] == "autoscale" { t.Fatalf("conflict updated the object: %v", got.GetAnnotations()) diff --git a/internal/server/backend.go b/internal/server/backend.go index e9a6e17..ef2d096 100644 --- a/internal/server/backend.go +++ b/internal/server/backend.go @@ -85,16 +85,6 @@ func failErr(code int, err error) error { } func writeStatus(w http.ResponseWriter, err error, fallback int) { - var managed *canaryManagedError - if errors.As(err, &managed) { - writeJSON(w, http.StatusConflict, map[string]any{"error": managed.Error(), "code": "CANARY_MANAGED_SERVICE", "owner": managed.Owner}) - return - } - var assigned *canaryAssignedError - if errors.As(err, &assigned) { - writeJSON(w, http.StatusConflict, map[string]any{"error": assigned.Error(), "code": "CANARY_ASSIGNED_SERVICE", "assignedTo": assigned.Assignment}) - return - } code := fallback var hf *httpFailure if errors.As(err, &hf) { diff --git a/internal/server/backend_llm.go b/internal/server/backend_llm.go index 402f453..f1d1824 100644 --- a/internal/server/backend_llm.go +++ b/internal/server/backend_llm.go @@ -184,9 +184,6 @@ func (b llmBackend) Apply(ctx context.Context, p *plan.Plan, mode exec.Mode, o a if err != nil { return applyResult{}, failErr(http.StatusBadGateway, err) } - if err := refuseCanaryPlan(cur, p, mode, o.Action); err != nil { - return applyResult{}, err - } if llmsvc.IsExternal(cur) { return applyResult{}, fail(http.StatusConflict, "release %s is an external LLMService: swiss did not deploy it, so it cannot be upgraded", ref) } @@ -340,9 +337,6 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal if err != nil { return uninstallResult{}, failErr(http.StatusBadGateway, err) } - if err := refuseCanaryService(u); err != nil { - return uninstallResult{}, err - } // The operator's finalizer holds the object through route removal and pod // termination. This call returns once that delete is accepted; the reap // drops swiss's records when the object is finally gone. @@ -390,9 +384,6 @@ func (b llmBackend) Uninstall(ctx context.Context, ns, release string) (uninstal if fresh.GetUID() != u.GetUID() { return restoreRejected(fail(http.StatusConflict, "service %s/%s was replaced; refresh before uninstalling", ns, release)) } - if err := refuseCanaryService(fresh); err != nil { - return restoreRejected(err) - } delErr = s.llms.DeleteObserved(ctx, fresh) } if apierrors.IsNotFound(delErr) { diff --git a/internal/server/backend_test.go b/internal/server/backend_test.go index 654c61b..d01c2c0 100644 --- a/internal/server/backend_test.go +++ b/internal/server/backend_test.go @@ -245,8 +245,9 @@ func newGateServer(t *testing.T, kind, applyWith, helmBin, helmfileBin string) * return g } -// Read fallback remains available; mutations require an ownership check. -func TestLLMInvisibleKeepsReadsAndRefusesWrites(t *testing.T) { +// A served CRD that this swissd cannot read must not turn the helm paths into +// 502s. Forbidden and a discovery error both behave as an unserved CRD. +func TestLLMInvisibleMatchesUnserved(t *testing.T) { helmBin := failBin(t, "helm") helmfileBin := failBin(t, "helmfile") base := newGateServer(t, "unserved", backendHelm, helmBin, helmfileBin) @@ -272,15 +273,26 @@ func TestLLMInvisibleKeepsReadsAndRefusesWrites(t *testing.T) { t.Fatalf("status called get after forbidden was cached: %d", other.calls.Load()) } + bHash := gatePlan(t, base) oHash := gatePlan(t, other) - for _, endpoint := range []string{"/api/install", "/api/apply"} { - code, body := post(t, other.srv, endpoint, map[string]any{"planHash": oHash}) - if code != http.StatusBadGateway || !strings.Contains(errText(body), "cannot verify service ownership") { - t.Fatalf("%s: %d %v", endpoint, code, body) - } + bc, bb := post(t, base.srv, "/api/install", map[string]any{"planHash": bHash}) + oc, ob := post(t, other.srv, "/api/install", map[string]any{"planHash": oHash}) + if bc != oc || bc == http.StatusBadGateway { + t.Fatalf("install base %d %v other %d %v", bc, bb, oc, ob) + } + if _, ok := base.w.written[planRef("models", "qwen-new")]; !ok { + t.Fatal("unserved install did not write the plan ConfigMap") + } + if _, ok := other.w.written[planRef("models", "qwen-new")]; !ok { + t.Fatal("hidden install did not write the plan ConfigMap") + } + bc, bb = post(t, base.srv, "/api/apply", map[string]any{"planHash": bHash}) + oc, ob = post(t, other.srv, "/api/apply", map[string]any{"planHash": oHash}) + if bc != oc || bc == http.StatusBadGateway { + t.Fatalf("upgrade base %d %v other %d %v", bc, bb, oc, ob) } - if len(other.w.written) != 0 { - t.Fatal("unverified ownership wrote a plan") + if kind == "forbidden" && other.calls.Load() != 1 { + t.Fatalf("apply called the API after forbidden was cached: %d", other.calls.Load()) } _, body := get(t, other.srv, "/api/cluster") @@ -306,8 +318,9 @@ func TestLLMInvisibleKeepsReadsAndRefusesWrites(t *testing.T) { } } -// An llmsvc profile cannot bypass ownership checks by falling back to helm. -func TestLLMApplyWithHiddenRefusesWrites(t *testing.T) { +// applyWith llmsvc falls back to helm when the CRD is served but not granted, +// and when discovery itself fails. +func TestLLMApplyWithHiddenFallsBackToHelm(t *testing.T) { helmBin := failBin(t, "helm") helmfileBin := failBin(t, "helmfile") for _, kind := range []string{"forbidden", "discovery"} { @@ -315,11 +328,11 @@ func TestLLMApplyWithHiddenRefusesWrites(t *testing.T) { g := newGateServer(t, kind, backendLLMSVC, helmBin, helmfileBin) hash := gatePlan(t, g) code, out := post(t, g.srv, "/api/install", map[string]any{"planHash": hash}) - if code != http.StatusBadGateway { + if code == http.StatusBadGateway { t.Fatalf("install %d %v", code, out) } - if len(g.w.written) != 0 { - t.Fatal("unverified ownership wrote a helm plan") + if _, ok := g.w.written[planRef("models", "qwen-new")]; !ok { + t.Fatal("fallback did not write the helm plan ConfigMap") } _, body := get(t, g.srv, "/api/cluster") info := body["llmservice"].(map[string]any) diff --git a/internal/server/canary.go b/internal/server/canary.go deleted file mode 100644 index 4954765..0000000 --- a/internal/server/canary.go +++ /dev/null @@ -1,74 +0,0 @@ -package server - -import ( - "context" - "fmt" - "net/http" - - "github.com/modelsphere/swiss/internal/llmsvc" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" -) - -type canaryManagedError struct{ Owner *llmsvc.CanaryOwner } - -func (e *canaryManagedError) Error() string { - return fmt.Sprintf("service is managed by LLMCanary %s/%s (UID %s); manage configuration and cleanup from the experiment", e.Owner.Namespace, e.Owner.Name, e.Owner.UID) -} - -type canaryAssignedError struct{ Assignment *llmsvc.CanaryAssignment } - -func (e *canaryAssignedError) Error() string { - if e.Assignment != nil && e.Assignment.Namespace != "" && e.Assignment.Name != "" { - return fmt.Sprintf("service is assigned to LLMCanary %s/%s (UID %s) as arm %s; release or delete the experiment before modifying this service", e.Assignment.Namespace, e.Assignment.Name, e.Assignment.UID, e.Assignment.Arm) - } - return "service has incomplete LLMCanary assignment metadata; refusal protects the experiment arm" -} - -func refuseCanaryService(u *unstructured.Unstructured) error { - if owner := llmsvc.CanaryOwnership(u); owner != nil { - return &canaryManagedError{Owner: owner} - } - if assigned := llmsvc.CanaryAssigned(u); assigned.WriteBlocked() { - return &canaryAssignedError{Assignment: assigned} - } - return nil -} - -func (s *Server) refuseCanaryWrite(ctx context.Context, ns, release string) error { - return s.refuseCanaryAccess(ctx, ns, release, true) -} - -func (s *Server) refuseCanaryAccess(ctx context.Context, ns, release string, write bool) error { - access := s.llmsvcAccess(ctx) - if access.discErr != nil || access.forbidden { - if !write { - return nil - } - return fail(http.StatusBadGateway, "cannot verify service ownership: %s", access.why()) - } - if !access.served || s.llms == nil { - return nil - } - ns, err := s.resolveNamespace(ctx, ns, release) - if err != nil { - if !write { - return nil - } - return failErr(http.StatusBadGateway, err) - } - u, err := s.llms.Get(ctx, ns, release) - if apierrors.IsNotFound(err) { - return nil - } - if err != nil { - if apierrors.IsForbidden(err) { - s.noteLLMSVCForbidden() - } - if !write { - return nil - } - return fail(http.StatusBadGateway, "cannot verify service ownership: %s", err) - } - return refuseCanaryService(u) -} diff --git a/internal/server/canary_adaptive.go b/internal/server/canary_adaptive.go index eaa047d..1114b39 100644 --- a/internal/server/canary_adaptive.go +++ b/internal/server/canary_adaptive.go @@ -1,21 +1,15 @@ package server import ( - "bytes" "context" - "encoding/json" - "errors" "fmt" - "github.com/modelsphere/swiss/internal/exec" "github.com/modelsphere/swiss/internal/llmsvc" "github.com/modelsphere/swiss/internal/plan" "github.com/modelsphere/swiss/internal/values" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "net/http" - "reflect" ) -func (s *Server) composeAssignedAdaptive(ctx context.Context, req planRequest) (*plan.Plan, error) { +func (s *Server) composeAdaptive(ctx context.Context, req planRequest) (*plan.Plan, error) { if req.FromRelease == "" || req.Namespace == "" || s.llms == nil { return nil, fmt.Errorf("adaptive upgrade requires an existing LLMService") } @@ -23,15 +17,11 @@ func (s *Server) composeAssignedAdaptive(ctx context.Context, req planRequest) ( if err != nil { return nil, err } - a := llmsvc.CanaryAssigned(u) - if a == nil || !a.Verified || a.Role != "arm" || llmsvc.CanaryOwnership(u) != nil { - return nil, fmt.Errorf("adaptive upgrade requires a verified assigned arm") - } if req.OverridesYAML != "" || req.EditsYAML != "" || req.Model != "" || req.Version != "" || req.Variant != "" || req.Catalog != "" || req.ChartVersion != "" || req.Release != "" || req.ServiceID != "" || req.LocalPath != "" || len(req.GPUProducts) != 0 || req.ForceGPUProducts || req.CreateNamespace { - return nil, fmt.Errorf("assigned arms permit only adaptive concurrency changes") + return nil, fmt.Errorf("adaptiveOnly permits only adaptive concurrency changes") } if len(llmsvc.WithoutAdaptiveValues(map[string]any(req.Overrides))) != 0 { - return nil, fmt.Errorf("assigned arms permit only adaptive concurrency changes") + return nil, fmt.Errorf("adaptiveOnly permits only adaptive concurrency changes") } requested, _, err := unstructured.NestedMap(map[string]any(req.Overrides), "modelRoute", "nginx", "values") if err != nil || len(requested) == 0 { @@ -63,59 +53,3 @@ func (s *Server) composeAssignedAdaptive(ctx context.Context, req planRequest) ( p.Helmfile, err = p.RenderHelmfile("") return p, err } - -func refuseCanaryPlan(u *unstructured.Unstructured, p *plan.Plan, mode exec.Mode, action string) error { - err := refuseCanaryService(u) - if err == nil { - return nil - } - a := llmsvc.CanaryAssigned(u) - if llmsvc.CanaryOwnership(u) != nil || a == nil || !a.Verified || a.Role != "arm" || mode != exec.Upgrade || action != "apply" { - return err - } - next, mapErr := llmsvc.FromPlan(p, action, "") - if mapErr != nil { - return mapErr - } - before, _, _ := unstructured.NestedMap(u.Object, "spec") - after, _, _ := unstructured.NestedMap(next.Object, "spec") - if !sameCanarySpec(llmsvc.WithoutAdaptiveSpec(before), llmsvc.WithoutAdaptiveSpec(after)) { - return err - } - current, mapErr := llmsvc.ToPlan(u) - if mapErr != nil { - return mapErr - } - if !reflect.DeepEqual(current.Source, p.Source) || current.Profile != p.Profile { - return err - } - if u.GetAnnotations()[llmsvc.AdaptiveBaselineKey] == "" { - return fail(http.StatusConflict, "experiment adaptive baseline is not ready; retry after the Canary controller reconciles") - } - return nil -} - -func (s *Server) refuseCanaryPlanWrite(ctx context.Context, p *plan.Plan, mode exec.Mode, action string) error { - err := s.refuseCanaryWrite(ctx, p.Release.Namespace, p.Release.Name) - if err == nil { - return nil - } - var assigned *canaryAssignedError - if !errors.As(err, &assigned) || s.llms == nil { - return err - } - u, getErr := s.llms.Get(ctx, p.Release.Namespace, p.Release.Name) - if getErr != nil { - return err - } - return refuseCanaryPlan(u, p, mode, action) -} - -func sameCanarySpec(before, after map[string]any) bool { - a, err := json.Marshal(before) - if err != nil { - return false - } - b, err := json.Marshal(after) - return err == nil && bytes.Equal(a, b) -} diff --git a/internal/server/canary_adaptive_test.go b/internal/server/canary_adaptive_test.go index d297334..6485e00 100644 --- a/internal/server/canary_adaptive_test.go +++ b/internal/server/canary_adaptive_test.go @@ -2,7 +2,6 @@ package server import ( "encoding/json" - "github.com/modelsphere/swiss/internal/exec" "github.com/modelsphere/swiss/internal/llmsvc" "github.com/modelsphere/swiss/internal/values" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -10,7 +9,7 @@ import ( "testing" ) -func TestAssignedAdaptiveUpgradePinsEverythingElse(t *testing.T) { +func TestAdaptiveUpgradePinsEverythingElse(t *testing.T) { rig := startRig(t, llmProbe(), true, false, backendLLMSVC) hash := rig.storePlan(t, map[string]any{"release": "adaptive-arm"}) p, err := rig.s.store.Plan(t.Context(), hash) @@ -34,10 +33,6 @@ func TestAssignedAdaptiveUpgradePinsEverythingElse(t *testing.T) { t.Fatal(err) } u = wire.(*unstructured.Unstructured) - anns := u.GetAnnotations() - anns[llmsvc.AssignmentKey] = `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"models","name":"experiment","uid":"parent","role":"arm","arm":"baseline"}` - anns[llmsvc.AdaptiveBaselineKey] = "ready" - u.SetAnnotations(anns) if _, err = rig.c.Create(t.Context(), u); err != nil { t.Fatal(err) } @@ -49,24 +44,12 @@ func TestAssignedAdaptiveUpgradePinsEverythingElse(t *testing.T) { if tuned.Source != p.Source || tuned.Chart != p.Chart || tuned.Profile != p.Profile { t.Fatal("upgrade moved a frozen source") } - if err = refuseCanaryPlan(u, tuned, exec.Upgrade, "apply"); err != nil { - t.Fatal(err) - } if tuned.Values()["schedulerName"] != "retained" { t.Fatal("lost scheduler") } if !reflect.DeepEqual(llmsvc.WithoutAdaptiveValues(map[string]any(p.Layers[values.LayerForm])), llmsvc.WithoutAdaptiveValues(map[string]any(tuned.Layers[values.LayerForm]))) { t.Fatal("changed non-adaptive values") } - for _, action := range []string{"rollback", "scale", "install"} { - if refuseCanaryPlan(u, tuned, exec.Upgrade, action) == nil { - t.Fatalf("allowed %s", action) - } - } - tuned.Layers[values.LayerForm]["replicaCount"] = int64(3) - if refuseCanaryPlan(u, tuned, exec.Upgrade, "apply") == nil { - t.Fatal("allowed scaling") - } req.Overrides["replicaCount"] = int64(3) if _, err = rig.s.compose(t.Context(), req); err == nil { t.Fatal("accepted non-adaptive request") diff --git a/internal/server/canary_review_test.go b/internal/server/canary_review_test.go index 54599d7..a4b3ac1 100644 --- a/internal/server/canary_review_test.go +++ b/internal/server/canary_review_test.go @@ -3,7 +3,6 @@ package server import ( "fmt" "maps" - "net/http" "testing" "github.com/modelsphere/swiss/internal/cluster" @@ -22,8 +21,8 @@ func TestLLMUninstallConflictRecovery(t *testing.T) { }{ {"status update", 200, 2, ""}, {"repeated conflict", 409, 2, ""}, - {"adopted", 409, 1, "CANARY_MANAGED_SERVICE"}, - {"assigned", 409, 1, "CANARY_ASSIGNED_SERVICE"}, + {"adopted", 200, 2, ""}, + {"assigned", 200, 2, ""}, {"replacement", 409, 1, ""}, {"gone", 404, 1, ""}, {"read failure", 502, 1, ""}, @@ -111,43 +110,3 @@ func TestLLMUninstallConflictRecovery(t *testing.T) { }) } } - -func TestSLOReadFallbackWithInvisibleLLMService(t *testing.T) { - for _, kind := range []string{"forbidden", "discovery"} { - t.Run(kind, func(t *testing.T) { - var sent map[string]any - up := sloStub(t, true, true, &sent) - g := newGateServer(t, kind, backendHelm, failBin(t, "helm"), failBin(t, "helmfile")) - probe := llmProbe() - probe.Maps["swiss/site-profile"] = map[string]string{"profile.yaml": profileYAML + "\nscaler:\n sloAddress: " + up.URL + "\n sloTokenSecret: llm-scaler/slo-api\n"} - probe.Secrets["llm-scaler/slo-api"] = map[string]string{"token": "slo-secret"} - g.s.probe = probe - for range 2 { - code, body := get(t, g.srv, "/api/releases/models/glm/slo") - if code != http.StatusOK || body["found"] != true { - t.Fatalf("read fallback %d: %v", code, body) - } - } - for _, call := range []func() (int, map[string]any){ - func() (int, map[string]any) { - return put(t, g.srv, "/api/releases/models/glm/slo", map[string]any{"priority": 10}) - }, - func() (int, map[string]any) { return del(t, g.srv, "/api/releases/models/glm/slo") }, - func() (int, map[string]any) { return del(t, g.srv, "/api/releases/models/glm") }, - func() (int, map[string]any) { - return post(t, g.srv, "/api/releases/models/glm/rollback", map[string]any{"toRevision": 1, "expectRevision": 1}) - }, - } { - if code, body := call(); code != http.StatusBadGateway { - t.Fatalf("unverified write %d: %v", code, body) - } - } - if sent != nil || len(g.w.written) != 0 { - t.Fatal("blocked write reached SLO or cluster writer") - } - if g.calls.Load() != 1 { - t.Fatalf("API error not cached: %d calls", g.calls.Load()) - } - }) - } -} diff --git a/internal/server/canary_test.go b/internal/server/canary_test.go index 791c924..c9ec15a 100644 --- a/internal/server/canary_test.go +++ b/internal/server/canary_test.go @@ -1,7 +1,6 @@ package server import ( - "net/http" "testing" "github.com/modelsphere/swiss/internal/llmsvc" @@ -49,7 +48,7 @@ func TestLLMDeleteUsesObservedIdentity(t *testing.T) { } } -func TestCanaryArmReadAndWriteBoundaries(t *testing.T) { +func TestCanaryArmReadMetadata(t *testing.T) { rig := startRig(t, llmProbe(), true, false, backendLLMSVC) u := ownedArm() if _, err := rig.c.Create(t.Context(), u); err != nil { @@ -68,48 +67,9 @@ func TestCanaryArmReadAndWriteBoundaries(t *testing.T) { t.Fatal(body) } } - hash := rig.storePlan(t, map[string]any{"release": "demo-baseline"}) - rig.dyn.ClearActions() - checks := []struct { - path string - call func() (int, map[string]any) - }{ - {"apply", func() (int, map[string]any) { - return post(t, rig.srv, "/api/apply", map[string]any{"planHash": hash, "expectRevision": 1, "forceConflicts": true}) - }}, - {"install", func() (int, map[string]any) { - return post(t, rig.srv, "/api/install", map[string]any{"planHash": hash}) - }}, - {"rollback", func() (int, map[string]any) { - return post(t, rig.srv, "/api/releases/models/demo-baseline/rollback", map[string]any{"toRevision": 1, "expectRevision": 1}) - }}, - {"uninstall", func() (int, map[string]any) { return del(t, rig.srv, "/api/releases/models/demo-baseline") }}, - {"slo write", func() (int, map[string]any) { - return put(t, rig.srv, "/api/releases/models/demo-baseline/slo", map[string]any{"priority": 10}) - }}, - {"slo reset", func() (int, map[string]any) { return del(t, rig.srv, "/api/releases/models/demo-baseline/slo") }}, - {"slo read", func() (int, map[string]any) { return get(t, rig.srv, "/api/releases/models/demo-baseline/slo") }}, - } - for _, check := range checks { - t.Run(check.path, func(t *testing.T) { - code, body := check.call() - if code != http.StatusConflict || body["code"] != "CANARY_MANAGED_SERVICE" { - t.Fatalf("%d %v", code, body) - } - owner, _ := body["owner"].(map[string]any) - if owner["uid"] != "parent-uid" { - t.Fatal(body) - } - }) - } - for _, a := range rig.dyn.Actions() { - if a.GetVerb() == "delete" || a.GetVerb() == "update" || a.GetVerb() == "patch" || a.GetVerb() == "create" { - t.Fatalf("blocked write reached Kubernetes: %v", a) - } - } } -func TestCanaryAssignedArmReadAndWriteBoundaries(t *testing.T) { +func TestCanaryAssignedArmReadMetadata(t *testing.T) { rig := startRig(t, llmProbe(), true, false, backendLLMSVC) u := llmObject("models", "demo-existing-arm", map[string]string{ llmsvc.AnnProfile: "prod", @@ -137,56 +97,76 @@ func TestCanaryAssignedArmReadAndWriteBoundaries(t *testing.T) { t.Fatalf("unexpected assignedTo: %v", body) } } - hash := rig.storePlan(t, map[string]any{"release": "demo-existing-arm"}) - rig.dyn.ClearActions() - checks := []struct { - path string - call func() (int, map[string]any) - }{ - {"apply", func() (int, map[string]any) { - return post(t, rig.srv, "/api/apply", map[string]any{"planHash": hash, "expectRevision": 1, "forceConflicts": true}) - }}, - {"install", func() (int, map[string]any) { - return post(t, rig.srv, "/api/install", map[string]any{"planHash": hash}) - }}, - {"rollback", func() (int, map[string]any) { - return post(t, rig.srv, "/api/releases/models/demo-existing-arm/rollback", map[string]any{"toRevision": 1, "expectRevision": 1}) - }}, - {"uninstall", func() (int, map[string]any) { return del(t, rig.srv, "/api/releases/models/demo-existing-arm") }}, - {"slo write", func() (int, map[string]any) { - return put(t, rig.srv, "/api/releases/models/demo-existing-arm/slo", map[string]any{"priority": 10}) - }}, - {"slo reset", func() (int, map[string]any) { return del(t, rig.srv, "/api/releases/models/demo-existing-arm/slo") }}, - } - for _, check := range checks { - t.Run(check.path, func(t *testing.T) { - code, body := check.call() - if code != http.StatusConflict || body["code"] != "CANARY_ASSIGNED_SERVICE" { - t.Fatalf("%d %v", code, body) - } - assigned, _ := body["assignedTo"].(map[string]any) - if assigned["uid"] != "parent-uid" || assigned["arm"] != "candidate" { - t.Fatal(body) - } - }) - } - for _, method := range []string{http.MethodPut, http.MethodDelete} { - req, err := http.NewRequest(method, rig.srv.URL+"/api/releases/models/demo-existing-arm/autoscaling", nil) - if err != nil { - t.Fatal(err) - } - resp, err := rig.srv.Client().Do(req) - if err != nil { - t.Fatal(err) - } - resp.Body.Close() - if resp.StatusCode != http.StatusMethodNotAllowed { - t.Fatalf("removed autoscaling mutation %s: %d", method, resp.StatusCode) - } - } - for _, a := range rig.dyn.Actions() { - if a.GetVerb() == "delete" || a.GetVerb() == "update" || a.GetVerb() == "patch" || a.GetVerb() == "create" { - t.Fatalf("blocked write reached Kubernetes: %v", a) +} + +func TestCanaryMetadataKeepsNormalServiceOperations(t *testing.T) { + for _, kind := range []string{"owned", "assigned", "malformed", "orphaned"} { + for _, operation := range []string{"apply", "install", "rollback", "uninstall", "slo read", "slo write", "slo reset"} { + t.Run(kind+"/"+operation, func(t *testing.T) { + statuses := make([]int, 0, 2) + for _, marked := range []bool{false, true} { + var sent map[string]any + up := sloStub(t, true, true, &sent) + probe := llmProbe() + probe.Maps["swiss/site-profile"] = map[string]string{"profile.yaml": probe.Maps["swiss/site-profile"]["profile.yaml"] + "\nscaler:\n sloAddress: " + up.URL + "\n sloTokenSecret: llm-scaler/slo-api\n"} + probe.Secrets["llm-scaler/slo-api"] = map[string]string{"token": "slo-secret"} + rig := startRig(t, probe, true, true, backendLLMSVC) + u := ownedArm() + u.SetName("glm") + if !marked || kind != "owned" { + u.SetOwnerReferences(nil) + u.SetLabels(nil) + } + if marked { + switch kind { + case "assigned": + anns := u.GetAnnotations() + anns[llmsvc.AssignmentKey] = `{"apiVersion":"canary.modelsphere.dev/v1alpha1","kind":"LLMCanary","namespace":"models","name":"demo","uid":"parent-uid","role":"arm","arm":"baseline"}` + u.SetAnnotations(anns) + case "malformed": + anns := u.GetAnnotations() + anns[llmsvc.AssignmentKey] = "{broken" + u.SetAnnotations(anns) + case "orphaned": + u.SetLabels(map[string]string{llmsvc.CanaryPrefix + "canary-name": "demo"}) + } + } + if _, err := rig.c.Create(t.Context(), u); err != nil { + t.Fatal(err) + } + hash := rig.storePlan(t, map[string]any{"release": "glm"}) + var code int + var body map[string]any + switch operation { + case "apply": + code, body = post(t, rig.srv, "/api/apply", map[string]any{"planHash": hash, "expectRevision": 1}) + case "install": + code, body = post(t, rig.srv, "/api/install", map[string]any{"planHash": hash}) + case "rollback": + code, body = post(t, rig.srv, "/api/releases/models/glm/rollback", map[string]any{"toRevision": 1, "expectRevision": 1}) + case "uninstall": + code, body = del(t, rig.srv, "/api/releases/models/glm") + case "slo read": + code, body = get(t, rig.srv, "/api/releases/models/glm/slo") + case "slo write": + code, body = put(t, rig.srv, "/api/releases/models/glm/slo", map[string]any{"priority": 10}) + case "slo reset": + code, body = del(t, rig.srv, "/api/releases/models/glm/slo") + } + if body["code"] == "CANARY_MANAGED_SERVICE" || body["code"] == "CANARY_ASSIGNED_SERVICE" { + t.Fatalf("Canary metadata blocked operation: %d %v", code, body) + } + if operation == "apply" || operation == "uninstall" || operation == "slo read" || operation == "slo write" || operation == "slo reset" { + if code < 200 || code >= 300 { + t.Fatalf("marked=%v: %d %v", marked, code, body) + } + } + statuses = append(statuses, code) + } + if statuses[0] != statuses[1] { + t.Fatalf("ordinary=%d marked=%d", statuses[0], statuses[1]) + } + }) } } } diff --git a/internal/server/deploy.go b/internal/server/deploy.go index 09729e8..b873a2e 100644 --- a/internal/server/deploy.go +++ b/internal/server/deploy.go @@ -169,7 +169,7 @@ func (s *Server) handlePlan(w http.ResponseWriter, r *http.Request) { func (s *Server) compose(ctx context.Context, req planRequest) (*plan.Plan, error) { if req.AdaptiveOnly { - return s.composeAssignedAdaptive(ctx, req) + return s.composeAdaptive(ctx, req) } if req.FromRelease != "" { var err error @@ -840,9 +840,6 @@ func (s *Server) apply(ctx context.Context, p *plan.Plan, mode exec.Mode, applyW // marker is written, so an apply cannot write its applying phase over the // marker and an uninstall cannot start in the middle of an apply. func (s *Server) applyLocked(ctx context.Context, p *plan.Plan, mode exec.Mode, applyWith string, o applyOpts) (applyResult, error) { - if err := s.refuseCanaryPlanWrite(ctx, p, mode, o.Action); err != nil { - return applyResult{}, err - } if err := s.refuseUninstalling(ctx, p.Release.Namespace, p.Release.Name); err != nil { return applyResult{}, err } @@ -879,10 +876,6 @@ func (s *Server) handleUninstall(w http.ResponseWriter, r *http.Request) { defer cancel() ns, release := r.PathValue("namespace"), r.PathValue("release") - if err := s.refuseCanaryWrite(ctx, ns, release); err != nil { - writeStatus(w, err, http.StatusBadGateway) - return - } if !s.tryApplyLock(ns, release) { // The lock may be held by the uninstall itself, still running helm, // or by another call while an LLMService is already being deleted. diff --git a/internal/server/rollback.go b/internal/server/rollback.go index 786d4b9..2e0a0dc 100644 --- a/internal/server/rollback.go +++ b/internal/server/rollback.go @@ -217,10 +217,6 @@ func (s *Server) handleRevisionDiff(w http.ResponseWriter, r *http.Request) { // engine flags come back verbatim -- which is the point. Going back to what // worked must not mean going to whatever the catalog now says that version is. func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) { - if err := s.refuseCanaryWrite(r.Context(), r.PathValue("namespace"), r.PathValue("release")); err != nil { - writeStatus(w, err, http.StatusBadGateway) - return - } ctx, cancel := contextWithTimeout(r, 15*time.Minute) defer cancel() diff --git a/internal/server/slo.go b/internal/server/slo.go index fdf47e4..f78bc4f 100644 --- a/internal/server/slo.go +++ b/internal/server/slo.go @@ -38,10 +38,6 @@ func (s *Server) handleSLO(w http.ResponseWriter, r *http.Request) { ctx, cancel := contextWithTimeout(r, 15*time.Second) defer cancel() - if err := s.refuseCanaryAccess(ctx, ns, release, r.Method != http.MethodGet); err != nil { - writeStatus(w, err, http.StatusBadGateway) - return - } prof, err := s.Profile(ctx) if err != nil { writeError(w, http.StatusBadGateway, err.Error()) From 64ad950b756330a288cd442db6d68624318b192a Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Thu, 5 Nov 2026 15:45:29 +0800 Subject: [PATCH 7/9] test(e2e): pin the CPU mock catalog fixture --- docs/e2e.md | 6 +- test/e2e/catalog_test.go | 25 ++++++++ test/e2e/e2e_suite_test.go | 24 +++++++- test/e2e/testdata/catalog/index-v2.json | 33 +++++++++++ test/e2e/testdata/catalog/mock-cpu.yaml | 76 +++++++++++++++++++++++++ test/e2e/testdata/profile.yaml | 2 +- 6 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 test/e2e/catalog_test.go create mode 100644 test/e2e/testdata/catalog/index-v2.json create mode 100644 test/e2e/testdata/catalog/mock-cpu.yaml diff --git a/docs/e2e.md b/docs/e2e.md index f9e370a..2cbd485 100644 --- a/docs/e2e.md +++ b/docs/e2e.md @@ -16,7 +16,7 @@ A failing `go test` stops the Make recipe before cleanup, so the cluster stays u ## Run -Prerequisites: Go, docker, kind, kubectl, helm, and outbound HTTPS (the catalog, the chart repo, ghcr.io, Docker Hub and raw.githubusercontent.com are fetched at run time). No GPU. The image build needs the `web/console` submodule. +Prerequisites: Go, docker, kind, kubectl, helm, and outbound HTTPS (the catalog, the chart repo, ghcr.io, Docker Hub and raw.githubusercontent.com are fetched at run time). No GPU. The default catalog is a checked-in fixture, so changes to the public catalog cannot remove the test model. Set `CATALOG` and `MODEL` together to exercise an external catalog. The image build needs the `web/console` submodule. ```sh git submodule update --init @@ -40,7 +40,7 @@ The node image and kube-proxy mode live in `test/e2e/kind-config.yaml`. `make se | --- | --- | | swissd | real: built from the checkout, loaded with `kind load`, installed from `helm/swiss` with `test/e2e/testdata/values.yaml` (SQLite on a local-path PVC, fixed login). Image and `rbac.namespaces` are set from `IMG` and `MODEL_NS` | | site profile | real: `test/e2e/testdata/profile.yaml`, with catalog URL, chart repo and namespace filled from the same env as the helm install, saved through `PUT /api/profile` | -| model catalog | real: the public v2 index, `$CATALOG` | +| model catalog | real: the pinned schema-v2 CPU mock fixture mounted from a ConfigMap; `$CATALOG` overrides it | | engine charts | real: `vllm` from the helm repo, fetched by swissd's helmfile | | model server | mocked: the catalog's `mock-llm` entry, [mock-llmd](https://github.com/modelsphere/mock-llmd) on the vllm chart, CPU only | | llm-operator | real: `dist/install.yaml` at `$LLM_OPERATOR_VERSION`. Serves LLMService and runs the LLMScaler controller | @@ -83,7 +83,7 @@ The autoscale spec applies the DisaggregatedSet CRD, runs an nginx stub for deci | `SKIP_BUILD` | unset. `true` or `1` skips the image build and still `kind load`s it | | `IMG` | `swissd:e2e` | | `IMAGE_REPO`, `IMAGE_TAG` | unset. When `IMAGE_REPO` is set it wins over `IMG`; `IMAGE_TAG` defaults to `e2e` | -| `CATALOG` | `https://modelsphere.github.io/model-catalog/` | +| `CATALOG` | `/etc/swiss-e2e-catalog`: the versioned mock fixture in `test/e2e/testdata/catalog`, mounted in swissd | | `CHART_REPO` | `https://modelsphere.github.io/helm-charts`, for the engine chart and the CRD charts | | `LLM_OPERATOR_VERSION` | `v0.5.0` | | `AUTOCONFIG_CHART_VERSION` | `0.4.1` | diff --git a/test/e2e/catalog_test.go b/test/e2e/catalog_test.go new file mode 100644 index 0000000..f9950b3 --- /dev/null +++ b/test/e2e/catalog_test.go @@ -0,0 +1,25 @@ +package e2e + +import ( + "context" + "testing" + + "github.com/modelsphere/swiss/internal/catalog" +) + +func TestPinnedMockCatalog(t *testing.T) { + c, err := catalog.OpenWithSchema(context.Background(), "testdata/catalog", "v2") + if err != nil { + t.Fatal(err) + } + resolved, err := c.Resolve(context.Background(), "mock-llm", "", "") + if err != nil { + t.Fatal(err) + } + if resolved.Variant.Requires.GPUs != 0 || !resolved.Variant.Weightless() { + t.Fatal("e2e fixture must run without GPUs or model weights") + } + if resolved.Variant.Image.Repository != "ghcr.io/modelsphere/mock-llmd" || resolved.Variant.Image.Tag != "0.1.0" { + t.Fatalf("unexpected mock image: %+v", resolved.Variant.Image) + } +} diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 980a971..9cdd003 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -39,6 +39,7 @@ const ( llmOperatorURL = "https://raw.githubusercontent.com/modelsphere/llm-operator/%s/dist/install.yaml" defaultReadySeconds = 300 defaultLocalPort = 18080 + defaultCatalog = "/etc/swiss-e2e-catalog" ) type suiteConfig struct { @@ -138,7 +139,7 @@ func loadConfig() suiteConfig { kubectlBin: envOr("KUBECTL", "kubectl"), helmBin: envOr("HELM", "helm"), cluster: envOr("KIND_CLUSTER", "swiss-test-e2e"), - catalog: envOr("CATALOG", "https://modelsphere.github.io/model-catalog/"), + catalog: envOr("CATALOG", defaultCatalog), chartRepo: envOr("CHART_REPO", "https://modelsphere.github.io/helm-charts"), llmOperator: envOr("LLM_OPERATOR_VERSION", "v0.5.0"), autoconfig: envOr("AUTOCONFIG_CHART_VERSION", "0.4.1"), @@ -235,6 +236,27 @@ func installSwiss(image string) { ) Expect(err).NotTo(HaveOccurred(), out) + if cfg.catalog == defaultCatalog { + By("mounting the pinned CPU-only e2e catalog") + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + files, err := os.ReadDir(filepath.Join(root, "test/e2e/testdata/catalog")) + Expect(err).NotTo(HaveOccurred()) + data := map[string]string{} + for _, file := range files { + raw, err := os.ReadFile(filepath.Join(root, "test/e2e/testdata/catalog", file.Name())) + Expect(err).NotTo(HaveOccurred()) + data[file.Name()] = string(raw) + } + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "swiss-e2e-catalog", Namespace: cfg.swissNS}, Data: data} + if _, err = kube.CoreV1().ConfigMaps(cfg.swissNS).Create(ctx, cm, metav1.CreateOptions{}); apierrors.IsAlreadyExists(err) { + _, err = kube.CoreV1().ConfigMaps(cfg.swissNS).Update(ctx, cm, metav1.UpdateOptions{}) + } + Expect(err).NotTo(HaveOccurred()) + patch := `{"spec":{"template":{"spec":{"containers":[{"name":"swissd","volumeMounts":[{"name":"e2e-catalog","mountPath":"` + defaultCatalog + `","readOnly":true}]}],"volumes":[{"name":"e2e-catalog","configMap":{"name":"swiss-e2e-catalog"}}]}}}}` + mustKubectl("-n", cfg.swissNS, "patch", "deployment/"+swissRelease, "--type=strategic", "-p", patch) + } + // A reused cluster keeps the old pod when only the image content changed. By("restarting swiss") mustKubectl("-n", cfg.swissNS, "rollout", "restart", "deploy/"+swissRelease) diff --git a/test/e2e/testdata/catalog/index-v2.json b/test/e2e/testdata/catalog/index-v2.json new file mode 100644 index 0000000..9d70019 --- /dev/null +++ b/test/e2e/testdata/catalog/index-v2.json @@ -0,0 +1,33 @@ +{ + "apiVersion": "catalog.swiss/v2", + "count": 1, + "models": [ + { + "name": "mock-llm", + "displayName": "Mock LLM", + "defaultVariant": "mock-cpu", + "variants": [ + { + "id": "mock-cpu", + "version": "1.0.0", + "servedName": "mock-llm", + "engine": "mock", + "chart": { + "name": "vllm", + "version": ">=0.7.6" + }, + "image": { + "repository": "ghcr.io/modelsphere/mock-llmd", + "tag": "0.1.0" + }, + "requires": { + "gpus": 0, + "topology": "single-node" + }, + "path": "mock-cpu.yaml", + "digest": "sha256:e53524e11262d99c96fe93e29d5da5ad8f6225bda36d8c9a1f8642eb852c1712" + } + ] + } + ] +} diff --git a/test/e2e/testdata/catalog/mock-cpu.yaml b/test/e2e/testdata/catalog/mock-cpu.yaml new file mode 100644 index 0000000..221ea0f --- /dev/null +++ b/test/e2e/testdata/catalog/mock-cpu.yaml @@ -0,0 +1,76 @@ +apiVersion: catalog.swiss/v2 +kind: Variant +model: mock-llm +id: mock-cpu +version: 1.0.0 +servedName: mock-llm +description: "mock-llmd on CPU: no GPU, no weights. For tests only -- the responses are synthetic." +link: https://github.com/modelsphere/mock-llmd +engine: mock +# mock-llmd has no chart of its own: it runs on the vllm chart, in place of +# vLLM, through commandOverride below. +chart: + name: vllm + version: ">=0.7.6" +image: + repository: ghcr.io/modelsphere/mock-llmd + tag: 0.1.0 +requires: + # CPU only: the chart leaves nvidia.com/gpu off the pod. + gpus: 0 + topology: single-node +values: + # The image's own ENTRYPOINT instead of `vllm serve`. + commandOverride: [] + model: + # No weights, so no model volume: the model has no source.hf to derive a path from. + localPath: "" + # The checks below assume a vLLM image -- a shell for modelCheck, python3 for + # the preStop drain, vLLM's /metrics for hangWatcher. The mock image is + # distroless and has none of them. + modelCheck: + enabled: false + hangWatcher: + enabled: false + lifecycle: + preStop: + enabled: false + shm: + enabled: false + # mock-llmd's default port, so it needs no PORT in env. + service: + port: 8081 + env: + # What /v1/models advertises; requests for any model name are answered. + - name: MOCK_MODEL_IDS + value: mock-llm + # mock-llmd has no /health; /v1/models answers as soon as it listens. + startupProbe: + httpGet: + path: /v1/models + port: http + periodSeconds: 2 + timeoutSeconds: 2 + failureThreshold: 30 + readinessProbe: + httpGet: + path: /v1/models + port: http + periodSeconds: 5 + timeoutSeconds: 2 + failureThreshold: 3 + livenessProbe: + httpGet: + path: /v1/models + port: http + periodSeconds: 10 + timeoutSeconds: 2 + failureThreshold: 3 + resources: + requests: + cpu: 100m + memory: 128Mi + limits: + memory: 512Mi + # Nothing to drain: the mock exits on SIGTERM. + terminationGracePeriodSeconds: 30 diff --git a/test/e2e/testdata/profile.yaml b/test/e2e/testdata/profile.yaml index e84268d..71de417 100644 --- a/test/e2e/testdata/profile.yaml +++ b/test/e2e/testdata/profile.yaml @@ -1,7 +1,7 @@ # Site profile for the kind e2e, saved through PUT /api/profile. # @CATALOG@, @CHART_REPO@ and @MODEL_NS@ are replaced when the suite renders it. # -# mock-llm is published in the v2 index only. It runs on CPU with no weights, +# The pinned mock-llm fixture uses the v2 index. It runs on CPU with no weights, # so the model path is never mounted. CART stays off: the test talks to the # engine directly. name: kind-e2e From 8c9fefb42831c357f3574414217f27824e3983b2 Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Thu, 5 Nov 2026 16:41:01 +0800 Subject: [PATCH 8/9] docs(api): describe Canary metadata and adaptive plans in OpenAPI --- README.md | 4 -- docs/architecture.md | 8 ---- docs/design-adaptive-canary-upgrades.md | 8 ---- docs/openapi.yaml | 60 ++++++++++++++++++++++++- 4 files changed, 58 insertions(+), 22 deletions(-) delete mode 100644 docs/design-adaptive-canary-upgrades.md diff --git a/README.md b/README.md index d5177c3..4dc7b1b 100644 --- a/README.md +++ b/README.md @@ -70,10 +70,6 @@ POST /api/plans /api/diff /api/apply /api/install (allowDeploy only) `applyWith` on the site profile chooses the backend for a new install: `helm` (the default when the field is omitted) or `llmsvc` when the LLMService CRD is served. A release already in the cluster stays on the backend that owns it. `llmsvc` requires the plan's `source.schema` to be v2; empty means v1 and is rejected with 400 and nothing is written. Helm accepts v1 and v2. The CLI `swiss install` and `swiss apply` stay on helm. See [docs/architecture.md](docs/architecture.md). -## Canary service metadata - -List/status includes service UIDs and `managedBy` / `assignedTo` identities for Console. Console controls experiment-service actions; Swiss applies normal backend rules to these services without Canary-specific write or SLO restrictions. - ## Deploy CI publishes the image to `ghcr.io/modelsphere/swissd` and diff --git a/docs/architecture.md b/docs/architecture.md index ac431a4..5b36266 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -195,11 +195,3 @@ form / flags ─┘ (hash) ↓ | `GET /api/nodes` | the GPU inventory: type, allocatable, in use, and what holds it | | `GET /api/profile` | the site profile as parsed, after defaults | | `GET /api/autoscaling`, `GET /api/releases/{ns}/{release}/autoscaling` | the autoscale loop's last pass per PD release with the form SLO on; in memory only, lost on restart | - -## Canary service operations - -- List/status reports ownership and assignment metadata for Console. -- Console controls experiment-service actions. Swiss has no Canary-specific guard for apply, install, rollback, scale, uninstall, or SLO access. -- `adaptiveOnly` is an optional plan composition mode that pins the live source and other values. It requires neither a Canary assignment nor a controller baseline; normal plan requests remain available. -- LLMService deletion retains UID/resourceVersion preconditions and conflict recovery to avoid deleting a replacement object. -- Kubernetes RBAC and any separately enabled admission webhook still apply to API writes. diff --git a/docs/design-adaptive-canary-upgrades.md b/docs/design-adaptive-canary-upgrades.md deleted file mode 100644 index bdcfb34..0000000 --- a/docs/design-adaptive-canary-upgrades.md +++ /dev/null @@ -1,8 +0,0 @@ -# Adaptive concurrency plan composition - -`POST /api/plans` accepts `adaptiveOnly: true` with `fromRelease`, `namespace` and `overrides.modelRoute.nginx.values`. - -- Composition starts from the live LLMService plan, replaces only `adaptive_cc` / `adaptive_cc_*` form values, and pins source, chart, other layers and values without resolving catalogs again. -- This optional mode works without a Canary assignment or controller-owned baseline. Normal compose and apply operations have no Canary-specific restrictions. -- List/status supplies ownership and assignment metadata for Console, which controls experiment-service actions. -- An independently enabled operator admission webhook can enforce additional rules on Kubernetes writes. diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 20ac376..49fca08 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -674,6 +674,8 @@ paths: With `fromRelease` it recomposes that release: its stored plan supplies the form layer, model and variant, and only what the request names moves. + With `adaptiveOnly: true`, it instead reads the live LLMService plan and + replaces only adaptive concurrency form values, without resolving a catalog. operationId: composePlan requestBody: required: true @@ -1805,7 +1807,18 @@ components: `overrides`; the named fields below exist because their key path or validation is swissd's business, not the form's. properties: - fromRelease: { type: string, description: "Recompose this release; release, namespace, model and variant come from its plan." } + fromRelease: { type: string, description: "Recompose this release; release, namespace, model and variant come from its plan. With adaptiveOnly, names the live LLMService in the explicitly supplied namespace." } + adaptiveOnly: + type: boolean + default: false + description: | + Compose from the live LLMService named by `fromRelease` and `namespace`. + Both fields and nonempty `overrides.modelRoute.nginx.values` are required. + Only `adaptive_cc` and keys beginning with `adaptive_cc_` may be supplied + in overrides; they replace the previous adaptive form values. Other + request fields must be omitted or empty. The source, chart and all + other values, including the canary-system layer, stay pinned. + No Canary assignment or baseline is required. False uses normal composition. catalog: { type: string, description: "Deploy: the catalog to compose from. Upgrade: a different catalog moves the release (same HF model and engine only)." } model: { type: string, description: "Required unless `fromRelease`." } version: { type: string } @@ -1861,7 +1874,7 @@ components: profile: { type: string } layers: type: object - description: "One values document per layer, applied in order: `defaults`, `catalog`, `site`, `derived`, `form`, `edit`, `autoscale`. `autoscale` is applied last and wins `replicaCount`." + description: "One values document per layer, applied in order: `defaults`, `catalog`, `site`, `derived`, `form`, `edit`, `autoscale`, `canary-system`. Later layers override earlier ones." properties: defaults: { type: object } catalog: { type: object } @@ -1870,6 +1883,7 @@ components: form: { type: object } edit: { type: object } autoscale: { type: object, description: "Only `replicaCount`. swissd writes it when the plan is disaggregated (`pd.enabled` true, `pd.colocate` not true) and the form layer has `sloRequirement.enabled` true, clamping the decision into [1, 64]." } + canary-system: { type: object, description: "Final values imposed by the Canary controller, preserved when reading a live LLMService plan and when composing an adaptive-only upgrade." } createNamespace: { type: boolean, default: false, deprecated: true, description: "Use `applyFlags.createNamespace`." } helmfile: { type: string, description: "The helmfile these values are applied through. Not hashed." } warnings: { type: array, items: { type: string }, description: "Accepted but worth seeing. Not hashed." } @@ -1917,10 +1931,49 @@ components: status: { type: string, examples: [applied] } statusError: { type: string, description: "The release is live but its status record was not updated." } + CanaryOwner: + type: object + description: | + Canary ownership reported from LLMService owner references or labels. + Omitted when no ownership metadata exists. Incomplete or inconsistent + metadata is returned with verified=false and may contain empty identity + fields. This metadata does not impose Swiss write or SLO restrictions. + required: [apiVersion, kind, namespace, name, uid, verified] + properties: + apiVersion: { type: string, examples: [canary.modelsphere.dev/v1alpha1] } + kind: { type: string, examples: [LLMCanary] } + namespace: { type: string, description: "The owning experiment's namespace." } + name: { type: string, description: "The owning experiment's name." } + uid: { type: string, description: "The owning experiment's UID." } + verified: { type: boolean, description: "True when the controller owner reference has a name and UID and the Canary labels match it. Does not verify that the experiment still exists." } + + CanaryAssignment: + type: object + description: | + The LLMService's canary.modelsphere.dev/assignment annotation. + Omitted when the annotation is absent. Malformed or incomplete metadata + is returned with verified=false and may contain empty identity fields + or unrecognized values. This metadata does not impose Swiss write or SLO restrictions. + required: [apiVersion, kind, namespace, name, uid, verified] + properties: + apiVersion: { type: string, examples: [canary.modelsphere.dev/v1alpha1] } + kind: { type: string, examples: [LLMCanary] } + namespace: { type: string, description: "The assigned experiment's namespace." } + name: { type: string, description: "The assigned experiment's name." } + uid: { type: string, description: "The assigned experiment's UID." } + role: { type: string, description: "production or arm for a verified assignment; omitted when empty.", examples: [production, arm] } + arm: { type: string, description: "Required for a verified arm assignment; empty and omitted for a verified production assignment." } + configDigest: { type: string, description: "Assigned experiment configuration digest, when recorded." } + configVersion: { type: string, description: "Assigned experiment configuration version, when recorded." } + verified: { type: boolean, description: "True when identity, role, arm and namespace fields pass assignment validation. Does not verify that the experiment still exists or that its configuration is current." } + Deployment: type: object required: [release, namespace, backend] properties: + uid: { type: string, description: "Kubernetes UID of the LLMService; omitted for Helm releases or when unavailable." } + managedBy: { $ref: "#/components/schemas/CanaryOwner" } + assignedTo: { $ref: "#/components/schemas/CanaryAssignment" } release: { type: string } namespace: { type: string } chart: { type: string, description: "`name-version`." } @@ -1972,6 +2025,9 @@ components: type: object required: [release, namespace, exists, revision, pods, ready, total, authPrefix] properties: + uid: { type: string, description: "Kubernetes UID of the LLMService; omitted for Helm releases or when unavailable." } + managedBy: { $ref: "#/components/schemas/CanaryOwner" } + assignedTo: { $ref: "#/components/schemas/CanaryAssignment" } release: { type: string } namespace: { type: string } exists: { type: boolean } From 7be48b94c768305ead7dda79905b68c103c1ab08 Mon Sep 17 00:00:00 2001 From: wenyunqing Date: Thu, 5 Nov 2026 17:36:38 +0800 Subject: [PATCH 9/9] feat(server): expose Canary states without adaptive guards --- docs/openapi.yaml | 40 +++++++++---- internal/llmsvc/adaptive.go | 38 ------------- internal/llmsvc/canary.go | 20 +++++++ internal/server/api.go | 13 +++-- internal/server/canary_adaptive.go | 55 ------------------ internal/server/canary_test.go | 57 +++++++++++++++++++ internal/server/deploy.go | 8 +-- internal/server/live_plan.go | 51 +++++++++++++++++ ...ary_adaptive_test.go => live_plan_test.go} | 35 +++++++++--- internal/server/status.go | 22 +++---- 10 files changed, 206 insertions(+), 133 deletions(-) delete mode 100644 internal/llmsvc/adaptive.go delete mode 100644 internal/server/canary_adaptive.go create mode 100644 internal/server/live_plan.go rename internal/server/{canary_adaptive_test.go => live_plan_test.go} (51%) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 49fca08..510ee88 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -674,8 +674,8 @@ paths: With `fromRelease` it recomposes that release: its stored plan supplies the form layer, model and variant, and only what the request names moves. - With `adaptiveOnly: true`, it instead reads the live LLMService plan and - replaces only adaptive concurrency form values, without resolving a catalog. + With `reuseSource: true`, it instead reads the live LLMService plan and + merges arbitrary form overrides and optional edit values, without resolving a catalog. operationId: composePlan requestBody: required: true @@ -1807,18 +1807,18 @@ components: `overrides`; the named fields below exist because their key path or validation is swissd's business, not the form's. properties: - fromRelease: { type: string, description: "Recompose this release; release, namespace, model and variant come from its plan. With adaptiveOnly, names the live LLMService in the explicitly supplied namespace." } - adaptiveOnly: + fromRelease: { type: string, description: "Recompose this release; release, namespace, model and variant come from its plan. With reuseSource, names the live LLMService in the explicitly supplied namespace." } + reuseSource: type: boolean default: false description: | - Compose from the live LLMService named by `fromRelease` and `namespace`. - Both fields and nonempty `overrides.modelRoute.nginx.values` are required. - Only `adaptive_cc` and keys beginning with `adaptive_cc_` may be supplied - in overrides; they replace the previous adaptive form values. Other - request fields must be omitted or empty. The source, chart and all - other values, including the canary-system layer, stay pinned. - No Canary assignment or baseline is required. False uses normal composition. + Reuse the live LLMService plan named by `fromRelease` and `namespace`. + Both identity fields are required. Merge arbitrary `overridesYAML` + and `overrides` into its form layer; null clears a value. A nonempty + `editsYAML` replaces the edit layer. The source, chart and remaining + layers are reused without catalog resolution. Catalog and source + selection fields are unused in this mode. No Canary or adaptive + concurrency restrictions are enforced; false uses normal composition. catalog: { type: string, description: "Deploy: the catalog to compose from. Upgrade: a different catalog moves the release (same HF model and engine only)." } model: { type: string, description: "Required unless `fromRelease`." } version: { type: string } @@ -1883,7 +1883,7 @@ components: form: { type: object } edit: { type: object } autoscale: { type: object, description: "Only `replicaCount`. swissd writes it when the plan is disaggregated (`pd.enabled` true, `pd.colocate` not true) and the form layer has `sloRequirement.enabled` true, clamping the decision into [1, 64]." } - canary-system: { type: object, description: "Final values imposed by the Canary controller, preserved when reading a live LLMService plan and when composing an adaptive-only upgrade." } + canary-system: { type: object, description: "Final values imposed by the Canary controller, preserved when reading a live LLMService plan and when reusing its source." } createNamespace: { type: boolean, default: false, deprecated: true, description: "Use `applyFlags.createNamespace`." } helmfile: { type: string, description: "The helmfile these values are applied through. Not hashed." } warnings: { type: array, items: { type: string }, description: "Accepted but worth seeing. Not hashed." } @@ -1931,6 +1931,20 @@ components: status: { type: string, examples: [applied] } statusError: { type: string, description: "The release is live but its status record was not updated." } + CanaryState: + type: string + enum: [standalone, referenced, owned] + description: | + LLMService relationship derived from Kubernetes metadata. Owned means + Canary ownership labels or an ownerReference are present; referenced + means an assignment annotation is present without ownership metadata; + standalone means neither is present. Ownership takes precedence. + Incomplete labels and malformed annotations retain their relationship + with verified=false in managedBy or assignedTo. Owned services follow + the Canary lifecycle through ownerReferences; referenced services are + retained when the experiment ends. Omitted on Helm releases or when + no LLMService exists. This field imposes no Swiss write restrictions. + CanaryOwner: type: object description: | @@ -1971,6 +1985,7 @@ components: type: object required: [release, namespace, backend] properties: + canaryState: { $ref: "#/components/schemas/CanaryState" } uid: { type: string, description: "Kubernetes UID of the LLMService; omitted for Helm releases or when unavailable." } managedBy: { $ref: "#/components/schemas/CanaryOwner" } assignedTo: { $ref: "#/components/schemas/CanaryAssignment" } @@ -2025,6 +2040,7 @@ components: type: object required: [release, namespace, exists, revision, pods, ready, total, authPrefix] properties: + canaryState: { $ref: "#/components/schemas/CanaryState" } uid: { type: string, description: "Kubernetes UID of the LLMService; omitted for Helm releases or when unavailable." } managedBy: { $ref: "#/components/schemas/CanaryOwner" } assignedTo: { $ref: "#/components/schemas/CanaryAssignment" } diff --git a/internal/llmsvc/adaptive.go b/internal/llmsvc/adaptive.go deleted file mode 100644 index 2718848..0000000 --- a/internal/llmsvc/adaptive.go +++ /dev/null @@ -1,38 +0,0 @@ -package llmsvc - -import ( - "k8s.io/apimachinery/pkg/runtime" - "strings" -) - -func AdaptiveKey(key string) bool { - return key == "adaptive_cc" || strings.HasPrefix(key, "adaptive_cc_") -} - -func WithoutAdaptiveValues(values map[string]any) map[string]any { - if values == nil { - return map[string]any{} - } - copy := runtime.DeepCopyJSONValue(values).(map[string]any) - stripAdaptive(copy, "modelRoute", "nginx", "values") - return copy -} - -func stripAdaptive(tree map[string]any, path ...string) { - if len(path) == 0 { - for key := range tree { - if AdaptiveKey(key) { - delete(tree, key) - } - } - return - } - child, ok := tree[path[0]].(map[string]any) - if !ok { - return - } - stripAdaptive(child, path[1:]...) - if len(child) == 0 { - delete(tree, path[0]) - } -} diff --git a/internal/llmsvc/canary.go b/internal/llmsvc/canary.go index 60813f3..a26cab1 100644 --- a/internal/llmsvc/canary.go +++ b/internal/llmsvc/canary.go @@ -111,3 +111,23 @@ func CanaryAssigned(u *unstructured.Unstructured) *CanaryAssignment { } return &a } + +type CanaryState string + +const ( + CanaryStandalone CanaryState = "standalone" + CanaryReferenced CanaryState = "referenced" + CanaryOwned CanaryState = "owned" +) + +func CanaryRelationship(u *unstructured.Unstructured) CanaryState { + if CanaryOwnership(u) != nil { + return CanaryOwned + } + if u != nil { + if _, ok := u.GetAnnotations()[AssignmentKey]; ok { + return CanaryReferenced + } + } + return CanaryStandalone +} diff --git a/internal/server/api.go b/internal/server/api.go index 2edec61..45a7e66 100644 --- a/internal/server/api.go +++ b/internal/server/api.go @@ -565,11 +565,12 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) { } type deployment struct { - UID string `json:"uid,omitempty"` - Release string `json:"release"` - Namespace string `json:"namespace"` - Chart string `json:"chart,omitempty"` - Status string `json:"status,omitempty"` + CanaryState llmsvc.CanaryState `json:"canaryState,omitempty"` + UID string `json:"uid,omitempty"` + Release string `json:"release"` + Namespace string `json:"namespace"` + Chart string `json:"chart,omitempty"` + Status string `json:"status,omitempty"` // Revision is helm's. Omitted when there is none, so an uninstalling row // whose release is already gone does not report 0. Revision int `json:"revision,omitempty"` @@ -675,7 +676,7 @@ func (s *Server) deploymentFromService(ctx context.Context, ref deployRef) *depl u := ref.obj d := &deployment{ Release: u.GetName(), Namespace: u.GetNamespace(), UID: string(u.GetUID()), - Backend: backendLLMSVC, External: llmsvc.IsExternal(u), CleanupPending: ref.cleanup, + Backend: backendLLMSVC, External: llmsvc.IsExternal(u), CleanupPending: ref.cleanup, CanaryState: llmsvc.CanaryRelationship(u), ManagedBy: llmsvc.CanaryOwnership(u), AssignedTo: llmsvc.CanaryAssigned(u), } obj, err := llmsvc.FromUnstructured(u) diff --git a/internal/server/canary_adaptive.go b/internal/server/canary_adaptive.go deleted file mode 100644 index 1114b39..0000000 --- a/internal/server/canary_adaptive.go +++ /dev/null @@ -1,55 +0,0 @@ -package server - -import ( - "context" - "fmt" - "github.com/modelsphere/swiss/internal/llmsvc" - "github.com/modelsphere/swiss/internal/plan" - "github.com/modelsphere/swiss/internal/values" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" -) - -func (s *Server) composeAdaptive(ctx context.Context, req planRequest) (*plan.Plan, error) { - if req.FromRelease == "" || req.Namespace == "" || s.llms == nil { - return nil, fmt.Errorf("adaptive upgrade requires an existing LLMService") - } - u, err := s.llms.Get(ctx, req.Namespace, req.FromRelease) - if err != nil { - return nil, err - } - if req.OverridesYAML != "" || req.EditsYAML != "" || req.Model != "" || req.Version != "" || req.Variant != "" || req.Catalog != "" || req.ChartVersion != "" || req.Release != "" || req.ServiceID != "" || req.LocalPath != "" || len(req.GPUProducts) != 0 || req.ForceGPUProducts || req.CreateNamespace { - return nil, fmt.Errorf("adaptiveOnly permits only adaptive concurrency changes") - } - if len(llmsvc.WithoutAdaptiveValues(map[string]any(req.Overrides))) != 0 { - return nil, fmt.Errorf("adaptiveOnly permits only adaptive concurrency changes") - } - requested, _, err := unstructured.NestedMap(map[string]any(req.Overrides), "modelRoute", "nginx", "values") - if err != nil || len(requested) == 0 { - return nil, fmt.Errorf("adaptive concurrency values are required") - } - p, err := llmsvc.ToPlan(u) - if err != nil { - return nil, err - } - p = clonePlan(p) - form := llmsvc.WithoutAdaptiveValues(map[string]any(p.Layers[values.LayerForm])) - if err = unstructured.SetNestedMap(form, requested, "modelRoute", "nginx", "values"); err != nil { - return nil, err - } - // Restore every non-adaptive route value alongside the replacement. - original, _, _ := unstructured.NestedMap(map[string]any(p.Layers[values.LayerForm]), "modelRoute", "nginx", "values") - for key, value := range original { - if !llmsvc.AdaptiveKey(key) { - requested[key] = value - } - } - if err = unstructured.SetNestedMap(form, requested, "modelRoute", "nginx", "values"); err != nil { - return nil, err - } - p.Layers[values.LayerForm] = values.Tree(form) - if err = p.ComputeHash(); err != nil { - return nil, err - } - p.Helmfile, err = p.RenderHelmfile("") - return p, err -} diff --git a/internal/server/canary_test.go b/internal/server/canary_test.go index c9ec15a..0e7c34a 100644 --- a/internal/server/canary_test.go +++ b/internal/server/canary_test.go @@ -170,3 +170,60 @@ func TestCanaryMetadataKeepsNormalServiceOperations(t *testing.T) { } } } + +func TestCanaryRelationshipsFollowLiveMetadata(t *testing.T) { + rig := startRig(t, llmProbe(), true, false, backendLLMSVC) + u := ownedArm() + u.SetLabels(nil) + u.SetOwnerReferences(nil) + live, err := rig.c.Create(t.Context(), u) + if err != nil { + t.Fatal(err) + } + for _, step := range []struct { + state string + assignment bool + ownership bool + }{ + {"standalone", false, false}, + {"referenced", true, false}, + {"owned", true, true}, + {"referenced", true, false}, + {"standalone", false, false}, + } { + labels := map[string]string{} + if step.ownership { + labels[llmsvc.CanaryPrefix+"canary-name"] = "demo" + } + live.SetLabels(labels) + anns := live.GetAnnotations() + if step.assignment { + anns[llmsvc.AssignmentKey] = "{incomplete" + } else { + delete(anns, llmsvc.AssignmentKey) + } + live.SetAnnotations(anns) + live, err = rig.c.Update(t.Context(), live) + if err != nil { + t.Fatal(err) + } + for _, path := range []string{"/api/deployments", "/api/releases/models/demo-baseline/status"} { + code, body := get(t, rig.srv, path) + if code != 200 { + t.Fatalf("%s: %d %v", path, code, body) + } + if path == "/api/deployments" { + body = body["deployments"].([]any)[0].(map[string]any) + } + if body["canaryState"] != step.state { + t.Fatalf("%s: want %s, got %v", path, step.state, body) + } + if step.state == "owned" && body["managedBy"].(map[string]any)["verified"] != false { + t.Fatal("incomplete ownership was verified") + } + if step.state == "referenced" && body["assignedTo"].(map[string]any)["verified"] != false { + t.Fatal("malformed assignment was verified") + } + } + } +} diff --git a/internal/server/deploy.go b/internal/server/deploy.go index c8e983f..d40b0bf 100644 --- a/internal/server/deploy.go +++ b/internal/server/deploy.go @@ -29,8 +29,8 @@ import ( type planRequest struct { // FromRelease recomposes a deployed release: its stored plan supplies the // form layer, the model and the variant, and only the catalog layer moves. - FromRelease string `json:"fromRelease,omitempty"` - AdaptiveOnly bool `json:"adaptiveOnly,omitempty"` + FromRelease string `json:"fromRelease,omitempty"` + ReuseSource bool `json:"reuseSource,omitempty"` // Catalog names the configured catalog to compose from. Required when the // site lists several. An upgrade naming none stays on the one its release // came from; naming another moves the release there, same model id. @@ -169,8 +169,8 @@ func (s *Server) handlePlan(w http.ResponseWriter, r *http.Request) { } func (s *Server) compose(ctx context.Context, req planRequest) (*plan.Plan, error) { - if req.AdaptiveOnly { - return s.composeAdaptive(ctx, req) + if req.ReuseSource { + return s.composeLive(ctx, req) } if req.FromRelease != "" { var err error diff --git a/internal/server/live_plan.go b/internal/server/live_plan.go new file mode 100644 index 0000000..261b090 --- /dev/null +++ b/internal/server/live_plan.go @@ -0,0 +1,51 @@ +package server + +import ( + "context" + "fmt" + + "github.com/modelsphere/swiss/internal/llmsvc" + "github.com/modelsphere/swiss/internal/plan" + "github.com/modelsphere/swiss/internal/values" + "gopkg.in/yaml.v3" +) + +func (s *Server) composeLive(ctx context.Context, req planRequest) (*plan.Plan, error) { + if req.FromRelease == "" || req.Namespace == "" || s.llms == nil { + return nil, fmt.Errorf("reuseSource requires an existing LLMService and its namespace") + } + u, err := s.llms.Get(ctx, req.Namespace, req.FromRelease) + if err != nil { + return nil, err + } + p, err := llmsvc.ToPlan(u) + if err != nil { + return nil, err + } + p = clonePlan(p) + form := p.Layers[values.LayerForm] + if form == nil { + form = values.Tree{} + } + if req.OverridesYAML != "" { + var extra values.Tree + if err = yaml.Unmarshal([]byte(req.OverridesYAML), &extra); err != nil { + return nil, fmt.Errorf("advanced overrides: %w", err) + } + values.Merge(form, extra, values.LayerForm, nil) + } + values.Merge(form, req.Overrides, values.LayerForm, nil) + p.Layers[values.LayerForm] = form + if req.EditsYAML != "" { + var edits values.Tree + if err = yaml.Unmarshal([]byte(req.EditsYAML), &edits); err != nil { + return nil, fmt.Errorf("edits: %w", err) + } + p.Layers[values.LayerEdit] = edits + } + if err = p.ComputeHash(); err != nil { + return nil, err + } + p.Helmfile, err = p.RenderHelmfile("") + return p, err +} diff --git a/internal/server/canary_adaptive_test.go b/internal/server/live_plan_test.go similarity index 51% rename from internal/server/canary_adaptive_test.go rename to internal/server/live_plan_test.go index 6485e00..7ad9e90 100644 --- a/internal/server/canary_adaptive_test.go +++ b/internal/server/live_plan_test.go @@ -5,11 +5,11 @@ import ( "github.com/modelsphere/swiss/internal/llmsvc" "github.com/modelsphere/swiss/internal/values" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "reflect" + "testing" ) -func TestAdaptiveUpgradePinsEverythingElse(t *testing.T) { +func TestLivePlanReusesSourceWithoutCanaryRestrictions(t *testing.T) { rig := startRig(t, llmProbe(), true, false, backendLLMSVC) hash := rig.storePlan(t, map[string]any{"release": "adaptive-arm"}) p, err := rig.s.store.Plan(t.Context(), hash) @@ -36,7 +36,7 @@ func TestAdaptiveUpgradePinsEverythingElse(t *testing.T) { if _, err = rig.c.Create(t.Context(), u); err != nil { t.Fatal(err) } - req := planRequest{AdaptiveOnly: true, Namespace: "models", FromRelease: "adaptive-arm", Overrides: values.Tree{"modelRoute": map[string]any{"nginx": map[string]any{"values": map[string]any{"adaptive_cc": "false"}}}}} + req := planRequest{ReuseSource: true, Namespace: "models", FromRelease: "adaptive-arm", Overrides: values.Tree{"modelRoute": map[string]any{"nginx": map[string]any{"values": map[string]any{"adaptive_cc": "false"}}}}} tuned, err := rig.s.compose(t.Context(), req) if err != nil { t.Fatal(err) @@ -47,11 +47,30 @@ func TestAdaptiveUpgradePinsEverythingElse(t *testing.T) { if tuned.Values()["schedulerName"] != "retained" { t.Fatal("lost scheduler") } - if !reflect.DeepEqual(llmsvc.WithoutAdaptiveValues(map[string]any(p.Layers[values.LayerForm])), llmsvc.WithoutAdaptiveValues(map[string]any(tuned.Layers[values.LayerForm]))) { - t.Fatal("changed non-adaptive values") - } + req.Overrides["replicaCount"] = int64(3) - if _, err = rig.s.compose(t.Context(), req); err == nil { - t.Fatal("accepted non-adaptive request") + req.OverridesYAML = "schedulerName: changed\n" + req.EditsYAML = "resources:\n limits:\n memory: 2Gi\n" + tuned, err = rig.s.compose(t.Context(), req) + if err != nil { + t.Fatal(err) + } + if tuned.Values()["replicaCount"] != int64(3) || tuned.Values()["schedulerName"] != "changed" { + t.Fatalf("ordinary overrides were restricted: %v", tuned.Values()) + } + if memory, _ := values.Get(tuned.Values(), "resources.limits.memory"); memory != "2Gi" { + t.Fatal("ordinary edit values were restricted") + } + req.Overrides = values.Tree{"modelRoute": map[string]any{"nginx": map[string]any{"values": map[string]any{"adaptive_cc_min_frac": nil, "default_max": "99"}}}} + tuned, err = rig.s.compose(t.Context(), req) + if err != nil { + t.Fatal(err) + } + route, _, _ := unstructured.NestedMap(map[string]any(tuned.Values()), "modelRoute", "nginx", "values") + if route["adaptive_cc_min_frac"] != nil { + t.Fatal("null did not clear a stored option") + } + if route["default_max"] != "99" { + t.Fatal("non-adaptive route setting was restricted") } } diff --git a/internal/server/status.go b/internal/server/status.go index 6298a21..0491e7f 100644 --- a/internal/server/status.go +++ b/internal/server/status.go @@ -22,16 +22,17 @@ import ( ) type releaseStatus struct { - UID string `json:"uid,omitempty"` - Release string `json:"release"` - Namespace string `json:"namespace"` - Exists bool `json:"exists"` - Revision int `json:"revision"` - HelmStatus string `json:"helmStatus,omitempty"` - Pods []cluster.Pod `json:"pods"` - Ready int `json:"ready"` - Total int `json:"total"` - Route string `json:"route,omitempty"` + CanaryState llmsvc.CanaryState `json:"canaryState,omitempty"` + UID string `json:"uid,omitempty"` + Release string `json:"release"` + Namespace string `json:"namespace"` + Exists bool `json:"exists"` + Revision int `json:"revision"` + HelmStatus string `json:"helmStatus,omitempty"` + Pods []cluster.Pod `json:"pods"` + Ready int `json:"ready"` + Total int `json:"total"` + Route string `json:"route,omitempty"` // URL is that route on the site's gateway, when the profile names one. URL string `json:"url,omitempty"` // Model is what the engine advertises, so a caller can name it in a request. @@ -137,6 +138,7 @@ func (s *Server) handleStatus(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadGateway, err.Error()) return } + out.CanaryState = llmsvc.CanaryRelationship(u) out.UID = string(u.GetUID()) out.ManagedBy = llmsvc.CanaryOwnership(u) out.AssignedTo = llmsvc.CanaryAssigned(u)