diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 20ac376..510ee88 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 `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 @@ -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 reuseSource, names the live LLMService in the explicitly supplied namespace." } + reuseSource: + type: boolean + default: false + description: | + 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 } @@ -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 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." } @@ -1917,10 +1931,64 @@ 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: | + 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: + 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" } release: { type: string } namespace: { type: string } chart: { type: string, description: "`name-version`." } @@ -1972,6 +2040,10 @@ 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" } release: { type: string } namespace: { type: string } exists: { type: boolean } diff --git a/internal/llmsvc/canary.go b/internal/llmsvc/canary.go new file mode 100644 index 0000000..a26cab1 --- /dev/null +++ b/internal/llmsvc/canary.go @@ -0,0 +1,133 @@ +package llmsvc + +import ( + "encoding/json" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +const ( + CanaryPrefix = "canary.modelsphere.dev/" + AssignmentKey = CanaryPrefix + "assignment" +) + +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"` +} + +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"` +} + +// CanaryOwnership reports incomplete ownership metadata with Verified=false. +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 { + 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 +} + +// CanaryAssigned reports malformed or incomplete assignments with Verified=false. +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 +} + +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/llmsvc/canary_test.go b/internal/llmsvc/canary_test.go new file mode 100644 index 0000000..2352bfc --- /dev/null +++ b/internal/llmsvc/canary_test.go @@ -0,0 +1,95 @@ +package llmsvc + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +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"} + 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}, + {"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}, + } { + 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 && 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) + } + }) + } +} + +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") + } +} + +func TestCanaryAssignedReportsArmAndMalformedMetadata(t *testing.T) { + for _, tc := range []struct { + name string + raw string + present, verified bool + }{ + {"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{} + 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) { + t.Fatalf("got %+v", got) + } + }) + } +} diff --git a/internal/llmsvc/client.go b/internal/llmsvc/client.go index b3dd164..13585d8 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/plan/plan.go b/internal/plan/plan.go index 10b667f..1792950 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -246,7 +246,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/server/api.go b/internal/server/api.go index 44e123c..45a7e66 100644 --- a/internal/server/api.go +++ b/internal/server/api.go @@ -565,10 +565,12 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) { } type deployment struct { - 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"` @@ -588,10 +590,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"` - 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. @@ -671,8 +675,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, + Release: u.GetName(), Namespace: u.GetNamespace(), UID: string(u.GetUID()), + 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) if err != nil { diff --git a/internal/server/backend.go b/internal/server/backend.go index 47e2d38..ef2d096 100644 --- a/internal/server/backend.go +++ b/internal/server/backend.go @@ -133,7 +133,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 98213dd..f1d1824 100644 --- a/internal/server/backend_llm.go +++ b/internal/server/backend_llm.go @@ -361,16 +361,36 @@ 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.Delete(ctx, ns, release) - 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)) + } + 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 { diff --git a/internal/server/canary_review_test.go b/internal/server/canary_review_test.go new file mode 100644 index 0000000..a4b3ac1 --- /dev/null +++ b/internal/server/canary_review_test.go @@ -0,0 +1,112 @@ +package server + +import ( + "fmt" + "maps" + "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", 200, 2, ""}, + {"assigned", 200, 2, ""}, + {"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") + } + }) + } +} diff --git a/internal/server/canary_test.go b/internal/server/canary_test.go new file mode 100644 index 0000000..0e7c34a --- /dev/null +++ b/internal/server/canary_test.go @@ -0,0 +1,229 @@ +package server + +import ( + "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 TestCanaryArmReadMetadata(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) + } + } +} + +func TestCanaryAssignedArmReadMetadata(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) + } + } +} + +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]) + } + }) + } + } +} + +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 e220907..d40b0bf 100644 --- a/internal/server/deploy.go +++ b/internal/server/deploy.go @@ -30,6 +30,7 @@ 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"` + 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. @@ -168,6 +169,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.ReuseSource { + return s.composeLive(ctx, req) + } if req.FromRelease != "" { var err error if req, err = s.carryForward(ctx, req); err != nil { 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/live_plan_test.go b/internal/server/live_plan_test.go new file mode 100644 index 0000000..7ad9e90 --- /dev/null +++ b/internal/server/live_plan_test.go @@ -0,0 +1,76 @@ +package server + +import ( + "encoding/json" + "github.com/modelsphere/swiss/internal/llmsvc" + "github.com/modelsphere/swiss/internal/values" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "testing" +) + +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) + 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) + if _, err = rig.c.Create(t.Context(), u); err != nil { + t.Fatal(err) + } + 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) + } + if tuned.Source != p.Source || tuned.Chart != p.Chart || tuned.Profile != p.Profile { + t.Fatal("upgrade moved a frozen source") + } + if tuned.Values()["schedulerName"] != "retained" { + t.Fatal("lost scheduler") + } + + req.Overrides["replicaCount"] = int64(3) + 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 c4b1cf4..0491e7f 100644 --- a/internal/server/status.go +++ b/internal/server/status.go @@ -22,15 +22,17 @@ import ( ) type releaseStatus struct { - 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. @@ -53,9 +55,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"` - 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. @@ -134,6 +138,10 @@ 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) var obj *llmsvc.LLMService if decoded, err := llmsvc.FromUnstructured(u); err == nil { obj = decoded 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 )