From 9a0cd9ef0eeeb53551094b9eb991e12e3ea8c9e3 Mon Sep 17 00:00:00 2001 From: imneov Date: Sat, 7 Nov 2026 16:00:23 +0800 Subject: [PATCH] feat(api): replace complete user deployment values Accept complete userValues documents while retaining upgrade identity checks, autoscale and experiment system layers. Advertise support through the cluster capability and keep legacy request behavior. --- docs/architecture.md | 14 ++ docs/design/user-values.md | 35 ++++ docs/openapi.yaml | 12 ++ internal/compose/compose.go | 6 +- internal/server/api.go | 20 ++- internal/server/autoscale.go | 11 +- internal/server/deploy.go | 260 +--------------------------- internal/server/deploy_request.go | 130 ++++++++++++++ internal/server/upgrade.go | 129 ++++++++++++++ internal/server/user_values.go | 43 +++++ internal/server/user_values_test.go | 170 ++++++++++++++++++ 11 files changed, 564 insertions(+), 266 deletions(-) create mode 100644 docs/design/user-values.md create mode 100644 internal/server/deploy_request.go create mode 100644 internal/server/upgrade.go create mode 100644 internal/server/user_values.go create mode 100644 internal/server/user_values_test.go diff --git a/docs/architecture.md b/docs/architecture.md index 5b36266..579d3f1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -195,3 +195,17 @@ 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 | + +### Complete user configuration + +`userValues` on plan and dry-run compose requests represents one complete user document; see [the API design](design/user-values.md). + +| Boundary | Behavior | +|---|---| +| Capability | `/api/cluster` advertises `capabilities.userValues` | +| User layers | Replace retained form/edit settings; preserve release identity and explicitly supplied request metadata | +| Clearing | `{}` removes previous user settings and inherits the target baseline | +| System layers | Upgrade composition carries autoscale and canary-system; their precedence remains authoritative | +| Autoscale opt-in | Read the explicit form/edit SLO choice together; catalog/site defaults alone do not opt a release in | +| Validation | Refuse ambiguous legacy inputs, service identity changes and a fixed replica count alongside an enabled scaler | +| Compatibility | Requests omitting `userValues` keep the legacy merge behavior | diff --git a/docs/design/user-values.md b/docs/design/user-values.md new file mode 100644 index 0000000..3776ead --- /dev/null +++ b/docs/design/user-values.md @@ -0,0 +1,35 @@ +# Complete user values for deployment composition + +A configuration editor owns one complete document. Legacy upgrade requests merge retained form keys, which cannot represent removing an override while preserving later system layers. + +## Request and composition + +| Contract | Behavior | +|---|---| +| `GET /api/cluster` → `capabilities.userValues: true` | The server supports complete user documents | +| `POST /api/plans` or dry-run compose with `userValues: {...}` | Replace the previous user form/edit configuration with this complete mapping | +| `userValues: {}` | Remove prior user settings and inherit the target baseline | +| `userValues` omitted | Preserve legacy merge behavior | +| Mixed `userValues` and legacy `overrides`, `overridesYAML`, `editsYAML`, or `reuseSource` | Reject the ambiguous request | + +```text +source / target / release metadata -> normal upgrade validation +complete user document -----------> edit layer + | + autoscale / system layers -> plan -> diff -> apply +``` + +- Store the complete document in the existing edit layer, with its existing validation/ownership semantics. Release identity and request metadata remain in the form/derived layers. +- `fromRelease` still validates model moves, resolves the running chart and carries system layers. It replaces retained user layers when `userValues` is present. +- The autoscale opt-in reads the explicit user form/edit layers together, so the same SLO choice has the same effect from structured controls or YAML. Catalog/site defaults alone do not opt a release in. +- Pin an upgrade's service identity. A conflicting `serviceId` inside the document is rejected. +- Preview uses the same composition path, including `fromRelease`; it never reconstructs the final layer order in the client. +- Authentication, deployment authorization and diff/apply revision checks remain the existing ones. Composition performs no deployment write. + +## Compatibility + +Clients require the advertised capability before submitting this request. Older clients retain their current behavior; older servers must be upgraded before the unified editor is enabled. No chart setting, CRD, database migration or permission change is required. + +## Validation + +Exercise installation, clearing an upgrade document, identity checks, preservation of unrelated user values, live autoscale carry/removal, ambiguous input rejection, and parity between planning and dry-run values. Record the results locally before publishing any change. diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 1d34431..e9d56ad 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -1602,6 +1602,10 @@ components: items: { $ref: "#/components/schemas/CatalogInfo" } version: { type: string, description: "swissd's version." } allowDeploy: { type: boolean } + capabilities: + type: object + properties: + userValues: { type: boolean, description: "Accepts complete user documents in PlanRequest.userValues." } sites: type: array items: { $ref: "#/components/schemas/Site" } @@ -1954,6 +1958,14 @@ components: forceGpuProducts: { type: boolean, default: false, description: "Accept products the variant does not list, if this cluster has them." } gpuResource: { type: string, description: "The resource to request, one the site profile's nodes.gpus lists for the variant's vendor. Empty picks it from the products' nodes, else the vendor's default. An upgrade sending no form keeps the resource the release runs on." } overrides: { type: object, description: "Form-layer values. On an upgrade, merged over the release's form layer." } + userValues: + type: object + description: | + Complete user configuration, stored in the edit layer. On an upgrade, + replaces retained user form/edit values while keeping release identity + and later system layers. An empty mapping clears user settings. + Cannot be combined with overrides, overridesYAML, editsYAML or reuseSource. + Requires capabilities.userValues from GET /api/cluster. overridesYAML: { type: string, description: "Form-layer values as YAML text." } editsYAML: { type: string, description: "The edit layer, exempt from ownership. The autoscale layer is applied after it." } createNamespace: { type: boolean, default: false, deprecated: true, description: "Use `applyFlags.createNamespace`." } diff --git a/internal/compose/compose.go b/internal/compose/compose.go index ee4fda8..19b455a 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -48,7 +48,8 @@ type Input struct { Schema string // Autoscale is the PD autoscaling layer - Autoscale values.Tree + Autoscale values.Tree + CanarySystem values.Tree // GPUResource is the extended resource the variant's pods request on this // cluster, resolved from inventory. Empty is the vendor's default in the // profile's GPU table. @@ -143,6 +144,9 @@ func Compose(in Input) (*plan.Plan, error) { if len(in.Autoscale) > 0 { layers[values.LayerAutoscale] = in.Autoscale } + if len(in.CanarySystem) > 0 { + layers[values.LayerCanarySystem] = in.CanarySystem + } p := &plan.Plan{ APIVersion: plan.APIVersion, diff --git a/internal/server/api.go b/internal/server/api.go index c9abac8..7409ea5 100644 --- a/internal/server/api.go +++ b/internal/server/api.go @@ -80,11 +80,12 @@ type clusterInfo struct { CatalogFrom string `json:"catalogFrom,omitempty"` CatalogRef string `json:"catalogRef,omitempty"` // Catalogs is every catalog a page may select, in the profile's order. - Catalogs []catalogInfo `json:"catalogs,omitempty"` - Version string `json:"version"` - AllowDeploy bool `json:"allowDeploy"` - Sites []site.Site `json:"sites,omitempty"` - Warnings []string `json:"warnings,omitempty"` + Catalogs []catalogInfo `json:"catalogs,omitempty"` + Version string `json:"version"` + AllowDeploy bool `json:"allowDeploy"` + Capabilities map[string]bool `json:"capabilities"` + Sites []site.Site `json:"sites,omitempty"` + Warnings []string `json:"warnings,omitempty"` // LLMService is whether new installs can use the operator, and which // backend they will use. LLMService *llmClusterInfo `json:"llmservice,omitempty"` @@ -134,10 +135,11 @@ func (s *Server) handleCluster(w http.ResponseWriter, r *http.Request) { repos, from := s.Catalogs(ctx) info := clusterInfo{ - Profile: s.cfg.Cluster.Profile.Ref(), - CatalogFrom: from, - Version: s.version, - AllowDeploy: s.cfg.Server.AllowDeploy, + Profile: s.cfg.Cluster.Profile.Ref(), + CatalogFrom: from, + Version: s.version, + AllowDeploy: s.cfg.Server.AllowDeploy, + Capabilities: map[string]bool{"userValues": true}, } if p, err := s.Profile(ctx); err == nil { // One instance, one profile. Its name is the cluster name. diff --git a/internal/server/autoscale.go b/internal/server/autoscale.go index b334f11..d53b197 100644 --- a/internal/server/autoscale.go +++ b/internal/server/autoscale.go @@ -12,16 +12,19 @@ import ( ) // autoscaleOn is the whole gate. A release scales when its plan is -// disaggregated and the form layer turned the SLO requirement on. +// disaggregated and the user configuration turned the SLO requirement on. func autoscaleOn(p *plan.Plan) bool { - return disaggregated(p) && formSLO(p) + return disaggregated(p) && userSLO(p) } -func formSLO(p *plan.Plan) bool { +func userSLO(p *plan.Plan) bool { if p == nil { return false } - on, ok := formBool(p.Layers[values.LayerForm], "sloRequirement.enabled") + user := values.Tree{} + values.Merge(user, p.Layers[values.LayerForm], values.LayerForm, nil) + values.Merge(user, p.Layers[values.LayerEdit], values.LayerEdit, nil) + on, ok := formBool(user, "sloRequirement.enabled") return ok && on } diff --git a/internal/server/deploy.go b/internal/server/deploy.go index 6e94fc8..438b423 100644 --- a/internal/server/deploy.go +++ b/internal/server/deploy.go @@ -26,126 +26,6 @@ import ( "gopkg.in/yaml.v3" ) -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. - Catalog string `json:"catalog,omitempty"` - Model string `json:"model"` - Version string `json:"version,omitempty"` - Variant string `json:"variant,omitempty"` - Release string `json:"release,omitempty"` - Namespace string `json:"namespace,omitempty"` - // ServiceID is the identity modelRoute, sloRequirement and the scaler all - // key off, and LocalPath overrides the site's path template. Both are form - // values; they are named here rather than left to Overrides because a - // deploy form should not have to know the key path. - ServiceID string `json:"serviceId,omitempty"` - LocalPath string `json:"localPath,omitempty"` - // GPUProducts narrows the deploy to these accelerator products, any one of - // which will do. Named here rather than left to Overrides because the node - // label they match on depends on the variant's vendor, which is the - // catalog's business and not the form's. - GPUProducts []string `json:"gpuProducts,omitempty"` - // ForceGPUProducts accepts gpuProducts the variant's catalog entry does not - // list, provided each one is on this cluster under the resource the variant's - // own products resolve to. - // The variant's GPU count, image and values are used unchanged. - ForceGPUProducts bool `json:"forceGpuProducts,omitempty"` - // GPUResource picks the extended resource to request among those the site - // profile's nodes.gpus lists for the variant's vendor, in place of the one - // its products' nodes report or the vendor's default. An upgrade sending - // no form keeps the resource the release runs on. - GPUResource string `json:"gpuResource,omitempty"` - Overrides values.Tree `json:"overrides,omitempty"` - // OverridesYAML is the advanced section: a values fragment typed by hand. - // Parsed here rather than in the browser so there is one parser, and the - // ownership check still decides what it may contain. - OverridesYAML string `json:"overridesYAML,omitempty"` - // EditsYAML is the plan editor: applied after every layer, exempt from - // ownership, and labelled "edit" wherever the plan is shown. - EditsYAML string `json:"editsYAML,omitempty"` - // CreateNamespace composes helmfile's createNamespace into the plan, on top - // of the site profile's own setting. helm is what creates the namespace; - // this is where the plan says so. - // - // Deprecated: applyFlags.createNamespace on the install. - CreateNamespace bool `json:"createNamespace,omitempty"` - // ChartVersion must fall in the variant's chart.version. Empty resolves as - // chart.Resolve does. - ChartVersion string `json:"chartVersion,omitempty"` - - // Set by carryForward, never by the caller. - runningChart plan.ChartRef - movedFrom *plan.Plan - // autoscale is the live replicaCount layer, copied while the previous plan - // is autoscaling and the new form keeps the rule. Unexported so a request - // body cannot set it. - autoscale values.Tree -} - -type applyRequest struct { - PlanHash string `json:"planHash"` - // ExpectRevision is the live helm revision the diff was computed against. - // It is the whole of the optimistic lock: the revision is helm's, held in - // the cluster, so two operators who diffed the same release cannot both - // apply. A version column in swissd's own database would have been a lock - // on a copy rather than on the thing being changed. - // - // Zero -- absent -- asserts nothing, which is what an apply that skipped the - // diff sends. The diff is optional, so the lock it carries is optional with - // it; the caller is giving up the concurrency check, not evading one. - ExpectRevision int `json:"expectRevision,omitempty"` - // Note is why this is being applied, in the operator's own words. Optional, - // recorded in the audit log and beside the release, and read by nothing -- - // a diff says what changed, and only a person can say why. - Note string `json:"note,omitempty"` - // ForceConflicts lets the upgrade take fields another manager owns, which - // is what a hand `kubectl edit` leaves behind. Per apply, never stored in - // the plan -- see exec.ApplyOptions. - // - // Deprecated: applyFlags.forceConflicts, which wins when sent. - ForceConflicts bool `json:"forceConflicts,omitempty"` - ApplyFlags applyFlags `json:"applyFlags,omitempty"` - // ApplyWith is a new install's backend, helm or llmsvc; empty is the site - // profile's. An existing release keeps its own. - ApplyWith string `json:"applyWith,omitempty"` -} - -// applyFlags are per-request options for diff, install and apply, never stored -// in the plan. Each endpoint takes the ones that apply to it and ignores the rest. -type applyFlags struct { - SkipSchemaValidation bool `json:"skipSchemaValidation,omitempty"` - // A pointer so that a false sent here overrides the deprecated top-level - // forceConflicts. - ForceConflicts *bool `json:"forceConflicts,omitempty"` - CreateNamespace bool `json:"createNamespace,omitempty"` -} - -// opts is what an install (or, without createNamespace, an upgrade) takes from -// the request. -func (req applyRequest) opts(mode exec.Mode) applyOpts { - o := applyOpts{ - ExpectRevision: req.ExpectRevision, - Action: actionName(mode), - Note: req.Note, - ForceConflicts: req.ForceConflicts, - SkipSchemaValidation: req.ApplyFlags.SkipSchemaValidation, - } - if f := req.ApplyFlags.ForceConflicts; f != nil { - o.ForceConflicts = *f - } - // An upgrade's namespace exists already. - if mode == exec.Install { - o.CreateNamespace = req.ApplyFlags.CreateNamespace - } - return o -} - func (s *Server) handlePlan(w http.ResponseWriter, r *http.Request) { ctx, cancel := contextWithTimeout(r, 30*time.Second) defer cancel() @@ -174,6 +54,10 @@ func (s *Server) handlePlan(w http.ResponseWriter, r *http.Request) { } func (s *Server) compose(ctx context.Context, req planRequest) (*plan.Plan, error) { + var err error + if req, err = normalizeUserValues(req); err != nil { + return nil, err + } if req.ReuseSource { return s.composeLive(ctx, req) } @@ -222,6 +106,9 @@ func (s *Server) compose(ctx context.Context, req planRequest) (*plan.Plan, erro if release == "" { release = resolved.Model } + if err := validateUserIdentity(req, release); err != nil { + return nil, err + } overrides := values.Tree{} if req.OverridesYAML != "" { @@ -313,6 +200,7 @@ func (s *Server) compose(ctx context.Context, req planRequest) (*plan.Plan, erro Edits: edits, CreateNamespace: req.CreateNamespace, Autoscale: req.autoscale, + CanarySystem: req.canarySystem, GPUResource: gpuRes, GPUNodeSelectorTerms: catalogTerms, }) @@ -333,130 +221,6 @@ func (s *Server) compose(ctx context.Context, req planRequest) (*plan.Plan, erro return p, nil } -// carryForward fills a request from a release's last plan, so an upgrade keeps -// the deploy inputs and moves only the model version. It needs no database: the -// whole plan sits beside the release. -func (s *Server) carryForward(ctx context.Context, req planRequest) (planRequest, error) { - prev, err := s.currentPlan(ctx, req.Namespace, req.FromRelease) - if err != nil { - return req, err - } - // Release and namespace come from the previous plan and are not overridable. - // They identify the release being upgraded; changing them does not rename - // anything, it installs a second release beside the first. - // Naming no catalog stays on the release's own; naming another moves it - // there, gated in compose by checkMove once the target entry is known. - own, ownErr := s.ReleaseCatalog(ctx, prev.Source.CatalogName, prev.Source.Catalog) - catalogName := req.Catalog - var movedFrom *plan.Plan - switch { - case catalogName == "": - if ownErr != nil { - return req, fmt.Errorf("cannot upgrade %s: %w", prev.Release.Name, ownErr) - } - catalogName = own.Name - case ownErr != nil || own.Name != catalogName: - movedFrom = prev - } - out := planRequest{ - Catalog: catalogName, - Model: prev.Source.Model, - Version: req.Version, - Variant: prev.Source.Variant, - Release: prev.Release.Name, - Namespace: prev.Release.Namespace, - Overrides: prev.Layers[values.LayerForm], - ChartVersion: req.ChartVersion, - runningChart: prev.Chart, - movedFrom: movedFrom, - GPUResource: req.GPUResource, - } - if req.Variant != "" { - out.Variant = req.Variant - } - - // A request carrying the form carries all of it. `swiss upgrade` and the - // version picker send no overrides and mean "move the catalog, keep every - // setting"; the upgrade form sends the whole form and is authoritative. - // - // The overrides merge rather than replace, so a value no form field covers - // -- set once from a flag or the advanced box -- survives an upgrade instead - // of being dropped by a form that never knew about it. Edits do replace: - // the form shows them, so an empty box means the operator emptied it. - if len(req.Overrides) == 0 { - if edits := prev.Layers[values.LayerEdit]; len(edits) > 0 { - out.EditsYAML = mustYAML(edits) - } - if out.GPUResource == "" { - res, _ := values.Get(prev.Layers[values.LayerDerived], "model.gpuResource") - out.GPUResource, _ = res.(string) - } - return s.withAutoscale(prev, out), nil - } - // A plan composed with no form values has no form layer to merge into. - if out.Overrides == nil { - out.Overrides = values.Tree{} - } - values.Merge(out.Overrides, req.Overrides, values.LayerForm, nil) - out.EditsYAML = req.EditsYAML - // serviceId is identity, not a setting: the route, the scaler and the SLO - // are all named after it. Changing it on an upgrade renames none of them -- - // helm renders a new set under the new name and orphans the old ones, on a - // release that keeps its name either way. Refused rather than ignored, so a - // caller that meant it learns that a new service is a new deploy. - if req.ServiceID != "" && req.ServiceID != serviceIDOf(prev) { - return req, fmt.Errorf( - "serviceId cannot change on an upgrade: %s is deployed as %q, not %q -- deploy a new release to run a second service", - prev.Release.Name, serviceIDOf(prev), req.ServiceID) - } - if req.LocalPath != "" { - out.LocalPath = req.LocalPath - } - if len(req.GPUProducts) > 0 { - out.GPUProducts = req.GPUProducts - out.ForceGPUProducts = req.ForceGPUProducts - } - return s.withAutoscale(prev, out), nil -} - -// withAutoscale copies the live autoscale layer onto an upgrade while the -// previous plan is autoscaling. Compose drops it again when the plan it builds -// is not autoscaling, so replicaCount falls back to form and edit. -func (s *Server) withAutoscale(prev *plan.Plan, out planRequest) planRequest { - if prev == nil || !autoscaleOn(prev) { - return out - } - layer := prev.Layers[values.LayerAutoscale] - if len(layer) == 0 { - return out - } - out.autoscale = cloneTree(layer) - return out -} - -// checkMove refuses a cross-catalog upgrade that would put another model, or the -// same one on another engine, behind the release's existing serviceId and route. -// Model ids are only unique within a catalog; HF is the identity across them. -func (s *Server) checkMove(ctx context.Context, prev *plan.Plan, to string, r catalog.Resolved) error { - refuse := func(format string, args ...any) error { - return fmt.Errorf("cannot move %s to catalog %q: %s -- deploy a new release instead", prev.Release.Name, to, fmt.Sprintf(format, args...)) - } - hf := prev.Source.HF - if hf == "" { - var err error - if hf, err = s.recordedHF(ctx, prev); err != nil { - return refuse("its plan predates recording the model's HF repo, and its own catalog could not supply it (%v)", err) - } - } - if r.Source.HF != hf { - return refuse("%s there is %s, not %s, so it is a different model", r.Model, r.Source.HF, hf) - } - if r.Variant.Engine != prev.Engine { - return refuse("variant %s runs on %s there, not %s", r.Variant.ID, r.Variant.Engine, prev.Engine) - } - return nil -} - // gpuInventoryError is a forced GPU product that could not be checked because // the node list was unreadable: the cluster's failure, not the request's. type gpuInventoryError struct{ err error } @@ -1233,11 +997,3 @@ func (s *Server) handleRun(w http.ResponseWriter, r *http.Request) { } writeJSON(w, http.StatusOK, run) } - -func mustYAML(t values.Tree) string { - b, err := yaml.Marshal(t) - if err != nil { - return "" - } - return string(b) -} diff --git a/internal/server/deploy_request.go b/internal/server/deploy_request.go new file mode 100644 index 0000000..073a161 --- /dev/null +++ b/internal/server/deploy_request.go @@ -0,0 +1,130 @@ +package server + +import ( + "github.com/modelsphere/swiss/internal/exec" + "github.com/modelsphere/swiss/internal/plan" + "github.com/modelsphere/swiss/internal/values" +) + +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. + Catalog string `json:"catalog,omitempty"` + Model string `json:"model"` + Version string `json:"version,omitempty"` + Variant string `json:"variant,omitempty"` + Release string `json:"release,omitempty"` + Namespace string `json:"namespace,omitempty"` + // ServiceID is the identity modelRoute, sloRequirement and the scaler all + // key off, and LocalPath overrides the site's path template. Both are form + // values; they are named here rather than left to Overrides because a + // deploy form should not have to know the key path. + ServiceID string `json:"serviceId,omitempty"` + LocalPath string `json:"localPath,omitempty"` + // GPUProducts narrows the deploy to these accelerator products, any one of + // which will do. Named here rather than left to Overrides because the node + // label they match on depends on the variant's vendor, which is the + // catalog's business and not the form's. + GPUProducts []string `json:"gpuProducts,omitempty"` + // ForceGPUProducts accepts gpuProducts the variant's catalog entry does not + // list, provided each one is on this cluster under the resource the variant's + // own products resolve to. + // The variant's GPU count, image and values are used unchanged. + ForceGPUProducts bool `json:"forceGpuProducts,omitempty"` + // GPUResource picks the extended resource to request among those the site + // profile's nodes.gpus lists for the variant's vendor, in place of the one + // its products' nodes report or the vendor's default. An upgrade sending + // no form keeps the resource the release runs on. + GPUResource string `json:"gpuResource,omitempty"` + Overrides values.Tree `json:"overrides,omitempty"` + // Present, including {}, replaces retained user form/edit values. + UserValues *values.Tree `json:"userValues,omitempty"` + // OverridesYAML is the advanced section: a values fragment typed by hand. + // Parsed here rather than in the browser so there is one parser, and the + // ownership check still decides what it may contain. + OverridesYAML string `json:"overridesYAML,omitempty"` + // EditsYAML is the plan editor: applied after every layer, exempt from + // ownership, and labelled "edit" wherever the plan is shown. + EditsYAML string `json:"editsYAML,omitempty"` + // CreateNamespace composes helmfile's createNamespace into the plan, on top + // of the site profile's own setting. helm is what creates the namespace; + // this is where the plan says so. + // + // Deprecated: applyFlags.createNamespace on the install. + CreateNamespace bool `json:"createNamespace,omitempty"` + // ChartVersion must fall in the variant's chart.version. Empty resolves as + // chart.Resolve does. + ChartVersion string `json:"chartVersion,omitempty"` + + // Set by carryForward, never by the caller. + runningChart plan.ChartRef + movedFrom *plan.Plan + // autoscale is the live replicaCount layer, copied while the previous plan + // is autoscaling and the new form keeps the rule. Unexported so a request + // body cannot set it. + autoscale values.Tree + canarySystem values.Tree +} + +type applyRequest struct { + PlanHash string `json:"planHash"` + // ExpectRevision is the live helm revision the diff was computed against. + // It is the whole of the optimistic lock: the revision is helm's, held in + // the cluster, so two operators who diffed the same release cannot both + // apply. A version column in swissd's own database would have been a lock + // on a copy rather than on the thing being changed. + // + // Zero -- absent -- asserts nothing, which is what an apply that skipped the + // diff sends. The diff is optional, so the lock it carries is optional with + // it; the caller is giving up the concurrency check, not evading one. + ExpectRevision int `json:"expectRevision,omitempty"` + // Note is why this is being applied, in the operator's own words. Optional, + // recorded in the audit log and beside the release, and read by nothing -- + // a diff says what changed, and only a person can say why. + Note string `json:"note,omitempty"` + // ForceConflicts lets the upgrade take fields another manager owns, which + // is what a hand `kubectl edit` leaves behind. Per apply, never stored in + // the plan -- see exec.ApplyOptions. + // + // Deprecated: applyFlags.forceConflicts, which wins when sent. + ForceConflicts bool `json:"forceConflicts,omitempty"` + ApplyFlags applyFlags `json:"applyFlags,omitempty"` + // ApplyWith is a new install's backend, helm or llmsvc; empty is the site + // profile's. An existing release keeps its own. + ApplyWith string `json:"applyWith,omitempty"` +} + +// applyFlags are per-request options for diff, install and apply, never stored +// in the plan. Each endpoint takes the ones that apply to it and ignores the rest. +type applyFlags struct { + SkipSchemaValidation bool `json:"skipSchemaValidation,omitempty"` + // A pointer so that a false sent here overrides the deprecated top-level + // forceConflicts. + ForceConflicts *bool `json:"forceConflicts,omitempty"` + CreateNamespace bool `json:"createNamespace,omitempty"` +} + +// opts is what an install (or, without createNamespace, an upgrade) takes from +// the request. +func (req applyRequest) opts(mode exec.Mode) applyOpts { + o := applyOpts{ + ExpectRevision: req.ExpectRevision, + Action: actionName(mode), + Note: req.Note, + ForceConflicts: req.ForceConflicts, + SkipSchemaValidation: req.ApplyFlags.SkipSchemaValidation, + } + if f := req.ApplyFlags.ForceConflicts; f != nil { + o.ForceConflicts = *f + } + // An upgrade's namespace exists already. + if mode == exec.Install { + o.CreateNamespace = req.ApplyFlags.CreateNamespace + } + return o +} diff --git a/internal/server/upgrade.go b/internal/server/upgrade.go new file mode 100644 index 0000000..22e0b4e --- /dev/null +++ b/internal/server/upgrade.go @@ -0,0 +1,129 @@ +package server + +import ( + "context" + "fmt" + + "github.com/modelsphere/swiss/internal/catalog" + "github.com/modelsphere/swiss/internal/plan" + "github.com/modelsphere/swiss/internal/values" + "gopkg.in/yaml.v3" +) + +func (s *Server) carryForward(ctx context.Context, req planRequest) (planRequest, error) { + prev, err := s.currentPlan(ctx, req.Namespace, req.FromRelease) + if err != nil { + return req, err + } + catalogName, movedFrom, err := s.upgradeCatalog(ctx, prev, req.Catalog) + if err != nil { + return req, err + } + out := planRequest{ + Catalog: catalogName, Model: prev.Source.Model, Version: req.Version, Variant: prev.Source.Variant, + Release: prev.Release.Name, Namespace: prev.Release.Namespace, Overrides: prev.Layers[values.LayerForm], + ChartVersion: req.ChartVersion, runningChart: prev.Chart, movedFrom: movedFrom, GPUResource: req.GPUResource, + canarySystem: cloneTree(prev.Layers[values.LayerCanarySystem]), + } + if req.Variant != "" { + out.Variant = req.Variant + } + out, bare := carryUserConfiguration(prev, req, out) + if bare { + return s.withAutoscale(prev, out), nil + } + if req.ServiceID != "" && req.ServiceID != serviceIDOf(prev) { + return req, fmt.Errorf("serviceId cannot change on an upgrade: %s is deployed as %q, not %q -- deploy a new release to run a second service", prev.Release.Name, serviceIDOf(prev), req.ServiceID) + } + if req.LocalPath != "" { + out.LocalPath = req.LocalPath + } + if len(req.GPUProducts) > 0 { + out.GPUProducts, out.ForceGPUProducts = req.GPUProducts, req.ForceGPUProducts + } + return s.withAutoscale(prev, out), nil +} + +func (s *Server) upgradeCatalog(ctx context.Context, previous *plan.Plan, requested string) (string, *plan.Plan, error) { + own, err := s.ReleaseCatalog(ctx, previous.Source.CatalogName, previous.Source.Catalog) + if requested == "" { + if err != nil { + return "", nil, fmt.Errorf("cannot upgrade %s: %w", previous.Release.Name, err) + } + return own.Name, nil, nil + } + if err != nil || own.Name != requested { + return requested, previous, nil + } + return requested, nil, nil +} + +func carryUserConfiguration(previous *plan.Plan, req, out planRequest) (planRequest, bool) { + switch { + case req.UserValues != nil: + out.Overrides, out.EditsYAML, out.UserValues = values.Tree{}, req.EditsYAML, req.UserValues + out.ServiceID = serviceIDOf(previous) + case len(req.Overrides) == 0: + if edits := previous.Layers[values.LayerEdit]; len(edits) > 0 { + out.EditsYAML = mustYAML(edits) + } + default: + if out.Overrides == nil { + out.Overrides = values.Tree{} + } + values.Merge(out.Overrides, req.Overrides, values.LayerForm, nil) + out.EditsYAML = req.EditsYAML + } + bare := req.UserValues == nil && len(req.Overrides) == 0 + if bare && out.GPUResource == "" { + resource, _ := values.Get(previous.Layers[values.LayerDerived], "model.gpuResource") + out.GPUResource, _ = resource.(string) + } + return out, bare +} + +// withAutoscale copies the live autoscale layer onto an upgrade while the +// previous plan is autoscaling. Compose drops it again when the plan it builds +// is not autoscaling, so replicaCount falls back to form and edit. +func (s *Server) withAutoscale(prev *plan.Plan, out planRequest) planRequest { + if prev == nil || !autoscaleOn(prev) { + return out + } + layer := prev.Layers[values.LayerAutoscale] + if len(layer) == 0 { + return out + } + out.autoscale = cloneTree(layer) + return out +} + +// checkMove refuses a cross-catalog upgrade that would put another model, or the +// same one on another engine, behind the release's existing serviceId and route. +// Model ids are only unique within a catalog; HF is the identity across them. +func (s *Server) checkMove(ctx context.Context, prev *plan.Plan, to string, r catalog.Resolved) error { + refuse := func(format string, args ...any) error { + return fmt.Errorf("cannot move %s to catalog %q: %s -- deploy a new release instead", prev.Release.Name, to, fmt.Sprintf(format, args...)) + } + hf := prev.Source.HF + if hf == "" { + var err error + if hf, err = s.recordedHF(ctx, prev); err != nil { + return refuse("its plan predates recording the model's HF repo, and its own catalog could not supply it (%v)", err) + } + } + if r.Source.HF != hf { + return refuse("%s there is %s, not %s, so it is a different model", r.Model, r.Source.HF, hf) + } + if r.Variant.Engine != prev.Engine { + return refuse("variant %s runs on %s there, not %s", r.Variant.ID, r.Variant.Engine, prev.Engine) + } + return nil +} + +func mustYAML(t values.Tree) string { + b, err := yaml.Marshal(t) + if err != nil { + return "" + } + return string(b) +} diff --git a/internal/server/user_values.go b/internal/server/user_values.go new file mode 100644 index 0000000..aa199c3 --- /dev/null +++ b/internal/server/user_values.go @@ -0,0 +1,43 @@ +package server + +import ( + "fmt" + "strings" + + "github.com/modelsphere/swiss/internal/values" + "gopkg.in/yaml.v3" +) + +func normalizeUserValues(req planRequest) (planRequest, error) { + if req.UserValues == nil { + return req, nil + } + if len(req.Overrides) > 0 || strings.TrimSpace(req.OverridesYAML) != "" || strings.TrimSpace(req.EditsYAML) != "" || req.ReuseSource { + return req, fmt.Errorf("userValues cannot be combined with overrides, overridesYAML, editsYAML or reuseSource") + } + if enabled, _ := values.Get(*req.UserValues, "scaler.enabled"); enabled == true { + if value, fixed := (*req.UserValues)["replicaCount"]; fixed && value != nil { + return req, fmt.Errorf("userValues.replicaCount cannot be set while scaler.enabled is true") + } + } + document, err := yaml.Marshal(*req.UserValues) + if err != nil { + return req, fmt.Errorf("userValues: %w", err) + } + req.EditsYAML = string(document) + return req, nil +} + +func validateUserIdentity(req planRequest, release string) error { + if req.UserValues == nil { + return nil + } + identity := req.ServiceID + if identity == "" { + identity = release + } + if actual, present := values.Get(*req.UserValues, "serviceId"); present && actual != identity { + return fmt.Errorf("userValues.serviceId must match the service identity %q", identity) + } + return nil +} diff --git a/internal/server/user_values_test.go b/internal/server/user_values_test.go new file mode 100644 index 0000000..32e4603 --- /dev/null +++ b/internal/server/user_values_test.go @@ -0,0 +1,170 @@ +package server + +import ( + "net/http" + "reflect" + "testing" + + "github.com/modelsphere/swiss/internal/cluster" + "github.com/modelsphere/swiss/internal/plan" + "github.com/modelsphere/swiss/internal/values" + "gopkg.in/yaml.v3" +) + +func TestUserValuesReplacesUpgradeDocument(t *testing.T) { + probe := deployed(t, planRequest{Model: "qwen3.6-35b-a3b", Release: "r", ServiceID: "r", Overrides: values.Tree{"replicaCount": 3, "priorityClassName": "old"}, EditsYAML: "oldCustom: true\n"}) + srv, _ := deployServerWith(t, probe, true) + for _, document := range []map[string]any{{"replicaCount": 2, "custom": map[string]any{"keep": true}}, {}} { + code, result := post(t, srv, "/api/plans", map[string]any{"fromRelease": "r", "userValues": document}) + if code != http.StatusOK { + t.Fatalf("status %d: %v", code, result) + } + edits := layerDoc(result, values.LayerEdit) + if !reflect.DeepEqual(jsonNumbers(edits), jsonNumbers(document)) { + t.Fatalf("user document %v, want %v", edits, document) + } + form := layerDoc(result, values.LayerForm) + if form["serviceId"] != "r" || form["replicaCount"] != nil || form["priorityClassName"] != nil { + t.Fatalf("retained form values or missing identity: %v", form) + } + if planValues(result)["oldCustom"] != nil { + t.Fatalf("retained old edit: %v", planValues(result)) + } + } +} + +func jsonNumbers(document map[string]any) map[string]any { + result := map[string]any{} + for key, value := range document { + if integer, ok := value.(int); ok { + result[key] = float64(integer) + } else { + result[key] = value + } + } + return result +} + +func TestUserValuesRejectsAmbiguousInputs(t *testing.T) { + srv, _ := deployServerWith(t, liveProbe(), true) + for _, legacy := range []map[string]any{{"overrides": map[string]any{"replicaCount": 2}}, {"overridesYAML": "x: 1"}, {"editsYAML": "x: 1"}, {"reuseSource": true}} { + body := map[string]any{"model": "qwen3.6-35b-a3b", "userValues": map[string]any{}} + for key, value := range legacy { + body[key] = value + } + code, result := post(t, srv, "/api/plans", body) + if code != http.StatusBadRequest { + t.Fatalf("ambiguous input accepted: %v, status %d: %v", body, code, result) + } + } +} + +func TestUserValuesCannotRenameService(t *testing.T) { + probe := deployed(t, planRequest{Model: "qwen3.6-35b-a3b", Release: "r", ServiceID: "r"}) + srv, _ := deployServerWith(t, probe, true) + for _, body := range []map[string]any{ + {"fromRelease": "r", "serviceId": "other", "userValues": map[string]any{}}, + {"fromRelease": "r", "userValues": map[string]any{"serviceId": "other"}}, + {"model": "qwen3.6-35b-a3b", "release": "r", "serviceId": "r", "userValues": map[string]any{"serviceId": nil}}, + } { + code, result := post(t, srv, "/api/plans", body) + if code != http.StatusBadRequest { + t.Fatalf("rename accepted, status %d: %v", code, result) + } + } +} + +func TestUserValuesCarriesAutoscaleThroughBothCompositionEndpoints(t *testing.T) { + previous, _ := livePlan(t, planRequest{Model: "qwen3.6-35b-a3b", Release: "r", ServiceID: "r"}) + setAutoscaleRule(previous) + previous.Layers[values.LayerAutoscale] = values.Tree{"replicaCount": 4} + files, err := previous.Files("") + if err != nil { + t.Fatal(err) + } + probe := liveProbe() + probe.Maps["swiss/site-profile"]["profile.yaml"] = profileYAML + "chartRepo: oci://ghcr.io/modelsphere/charts\n" + probe.Rel = append(probe.Rel, cluster.Release{Name: "r", Namespace: "models", Status: "deployed", Revision: 3, SwissFiles: files}) + srv, server := deployServerWith(t, probe, true) + server.cfg.Server.HelmBin, _ = stubShowValues(t, 0) + document := values.Tree{} + values.Merge(document, previous.Layers[values.LayerForm], values.LayerEdit, nil) + document["replicaCount"] = 2 + body := map[string]any{"fromRelease": "r", "userValues": document} + code, result := post(t, srv, "/api/plans", body) + if code != http.StatusOK { + t.Fatalf("status %d: %v", code, result) + } + if jsonInt(planValues(result)["replicaCount"]) != 4 { + t.Fatalf("autoscale lost: %v", result) + } + code, preview := post(t, srv, "/api/v2/compose?dryRun=true&dumpHelmValues=true", body) + if code != http.StatusOK { + t.Fatalf("preview status %d: %v", code, preview) + } + helmValues := preview["helmValues"].(map[string]any) + var effective values.Tree + if err := yaml.Unmarshal([]byte(helmValues["overrides"].(string)), &effective); err != nil { + t.Fatal(err) + } + if jsonInt(effective["replicaCount"]) != 4 { + t.Fatalf("preview lost autoscale: %v", effective) + } + document["sloRequirement"] = values.Tree{"enabled": false} + code, result = post(t, srv, "/api/plans", body) + if code != http.StatusOK || len(layerDoc(result, values.LayerAutoscale)) != 0 || jsonInt(planValues(result)["replicaCount"]) != 2 { + t.Fatalf("leaving autoscale did not restore the user count: %d %v", code, result) + } +} + +func TestUserValuesCapability(t *testing.T) { + srv := testServer(t, fakeProbe()) + code, result := get(t, srv, "/api/cluster") + capabilities, _ := result["capabilities"].(map[string]any) + if code != http.StatusOK || capabilities["userValues"] != true { + t.Fatalf("missing capability: %d %v", code, result) + } +} + +func TestUserValuesRejectsTwoReplicaControllers(t *testing.T) { + srv, _ := deployServerWith(t, liveProbe(), true) + code, result := post(t, srv, "/api/plans", map[string]any{"model": "qwen3.6-35b-a3b", "userValues": map[string]any{"replicaCount": 2, "scaler": map[string]any{"enabled": true}}}) + if code != http.StatusBadRequest { + t.Fatalf("competing replica settings accepted: %d %v", code, result) + } +} + +func TestUserValuesCarriesCanarySystem(t *testing.T) { + previous, _ := livePlan(t, planRequest{Model: "qwen3.6-35b-a3b", Release: "r", ServiceID: "r"}) + previous.Layers[values.LayerCanarySystem] = values.Tree{"scaler": values.Tree{"enabled": false}} + files, err := previous.Files("") + if err != nil { + t.Fatal(err) + } + probe := liveProbe() + probe.Rel = append(probe.Rel, cluster.Release{Name: "r", Namespace: "models", Status: "deployed", Revision: 3, SwissFiles: files}) + srv, _ := deployServerWith(t, probe, true) + code, result := post(t, srv, "/api/plans", map[string]any{"fromRelease": "r", "userValues": map[string]any{"scaler": map[string]any{"enabled": true}}}) + if code != http.StatusOK || layerDoc(result, values.LayerCanarySystem)["scaler"] == nil { + t.Fatalf("lost canary policy: %d %v", code, result) + } + if enabled, _ := values.Get(planValues(result), "scaler.enabled"); enabled != false { + t.Fatalf("user values bypassed canary policy: %v", result) + } +} + +func TestUserSLOOptIn(t *testing.T) { + configuration := &plan.Plan{Layers: map[string]values.Tree{values.LayerSite: {"sloRequirement": values.Tree{"enabled": true}}}} + if userSLO(configuration) { + t.Fatal("site default opted a release in") + } + configuration.Layers[values.LayerForm] = values.Tree{"sloRequirement": values.Tree{"enabled": false}} + configuration.Layers[values.LayerEdit] = values.Tree{"sloRequirement": values.Tree{"enabled": true}} + if !userSLO(configuration) { + t.Fatal("YAML opt-in was ignored") + } + configuration.Layers[values.LayerEdit] = values.Tree{"sloRequirement": values.Tree{"enabled": false}} + if userSLO(configuration) { + t.Fatal("YAML opt-out was ignored") + } +}