Skip to content

feat(api): replace complete user deployment values - #52

Closed
imneov wants to merge 1 commit into
modelsphere:masterfrom
imneov:feat/complete-user-values
Closed

imneov wants to merge 1 commit into
modelsphere:masterfrom
imneov:feat/complete-user-values

Conversation

@imneov

@imneov imneov commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

What this PR does and why

A configuration editor cannot clear an upgrade setting through the legacy overrides API: omitted keys are merged back from the running form. Recomposing without fromRelease also loses the live autoscale layer. Add an explicit userValues document so clients can replace all user settings through normal upgrade composition.

  • Advertise support as GET /api/cluster → capabilities.userValues.
  • Accept userValues in plan and dry-run composition. An empty mapping clears retained form/edit settings; omitted userValues keeps legacy behavior.
  • Preserve upgrade source validation, service identity, autoscale and canary-system layers. Reject conflicting service identity, mixed legacy inputs, and fixed replicas alongside an enabled scaler.
  • Read the explicit form/edit SLO choice together for autoscale opt-in; site/catalog defaults alone do not opt a release in.

The complete document uses the existing edit layer. Authentication, authorization and diff/apply revision checks use the existing endpoints and policies.

Related work

Supports the unified deployment editor in modelsphere/console#81, related to modelsphere/console#66.

Design and API contract: docs/design/user-values.md, docs/architecture.md, docs/openapi.yaml.

How it was tested

  • go vet ./... passed.
  • go test ./... passed, including Helm chart tests with an isolated empty kubeconfig.
  • Added integration coverage for complete/empty user replacement, legacy requests, service identity, ambiguous input, conflicting replica controls, explicit SLO choice, autoscale carry/removal, canary policy preservation, and parity between plan and dry-run values.
  • Changed Go files are formatted; git diff --check passed.

These are local tests using fake cluster data and stub chart reads. Live model deployment and interruption/recovery remain release acceptance checks.

Notes for reviewers

Start with user_values.go and user_values_test.go, then upgrade.go and the layer input in internal/compose/compose.go. Request structs and upgrade helpers were extracted from deploy.go to keep that file below 1000 lines.

The frontend must use the advertised capability and be released after this API is available.

Release note

The deployment API accepts complete userValues documents for deploy and upgrade, including clearing retained user settings, while preserving upgrade validation and later autoscale/experiment policy layers. Existing clients retain legacy request behavior. Explicit edit-layer SLO choices now participate in autoscale opt-in alongside form 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.

@huskcass huskcass left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design blocker: userValues replacement clears both saved form and edit settings. A later YAML save can silently discard settings previously entered in the web form simply because they are omitted from the YAML. Users expect those form settings to remain effective, with YAML as an override layer—not replacement of all user configuration. The existing UI already supports changing values; resetting overrides to inherit defaults does not justify this broader configuration ownership/UI contract change. Swiss should simplify while preserving existing features and UI expectations, with added complexity justified by a concrete unmet need.

Other findings at 9a0cd9e:

  • Medium: autoscaleOn now reads stored edits. Existing releases can start or stop autoscaling after the Swiss upgrade without a redeploy; document the upgrade impact.
  • Low: the scaler.enabled / replicaCount guard misses scaler settings inherited from catalog/site.
  • Low: on install, userValues.serviceId must equal the release name unless top-level serviceId is also supplied.
  • Low: the retained canary-system layer has no removal path.

Verdict: Blocked on the configuration ownership/UI contract change.

— Review Golden

@imneov imneov closed this Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants