diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 1d34431..b794545 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -748,6 +748,10 @@ paths: 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. + + Each of the variant's `inputs` is answered in the `derived` layer by the + site profile's `inputs`, else the input's `default`; `overrides` go over + it. A plan that leaves one without a valid answer is 400. operationId: composePlan requestBody: required: true @@ -1788,6 +1792,17 @@ components: extra: type: object description: Values every release on this site gets, as the site layer. + inputs: + type: object + description: | + Answers to variants' `inputs`, by each input's key. Unlike `extra`, an + answer reaches only a variant that declares the key, in the `derived` + layer. A string may use `model.pathTemplate`'s placeholders + (`{{hf}}`, `{{org}}`, `{{name}}`, `{{model}}`). A key `extra` also sets is + refused. + additionalProperties: { type: [string, number, boolean] } + examples: + - storageKVOffload.mooncake.master: mooncake-master.svc:50051 Secrets: type: object @@ -1825,6 +1840,19 @@ components: minGpuMemoryGiB: { type: number } rdma: { type: boolean } + Input: + type: object + description: A chart value every deploy must answer. Answered in the `derived` layer by the site profile's `inputs`, else `default`; a deploy's overrides go over it. + required: [key, title, example] + properties: + key: { type: string, description: "The chart value's dotted key.", examples: [storageKVOffload.mooncake.master] } + title: { type: string } + description: { type: string } + type: { type: string, enum: [string, integer, boolean], default: string } + pattern: { type: string, description: "A string answer matches it, whole." } + example: { type: [string, integer, boolean], description: "The form's placeholder; never deployed." } + default: { type: [string, integer, boolean], description: "The answer when the site profile has none." } + ModelVariant: type: object required: [id, version, digest, engine, chart, requires] @@ -1907,6 +1935,7 @@ components: tag: { type: string } digest: { type: string } requires: { $ref: "#/components/schemas/Requires" } + inputs: { type: array, items: { $ref: "#/components/schemas/Input" } } values: { type: object, description: "The catalog layer's chart values." } version: { type: string } digest: { type: string, description: "sha256 of the file this version pins." } diff --git a/internal/catalog/entry.go b/internal/catalog/entry.go index f08eabd..047c013 100644 --- a/internal/catalog/entry.go +++ b/internal/catalog/entry.go @@ -49,15 +49,17 @@ type Source struct { // together, which is why they are one object rather than independent form // fields: nothing should be able to select --tp-size=8 and a 2-GPU node. type Variant struct { - ID string `yaml:"id" json:"id"` - Description string `yaml:"description,omitempty" json:"description,omitempty"` - Link string `yaml:"link,omitempty" json:"link,omitempty"` - Default bool `yaml:"default,omitempty" json:"default,omitempty"` - Engine string `yaml:"engine" json:"engine"` - Chart Chart `yaml:"chart" json:"chart"` - Image *Image `yaml:"image,omitempty" json:"image,omitempty"` - Requires Requires `yaml:"requires" json:"requires"` - Values values.Tree `yaml:"values,omitempty" json:"values,omitempty"` + ID string `yaml:"id" json:"id"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + Link string `yaml:"link,omitempty" json:"link,omitempty"` + Default bool `yaml:"default,omitempty" json:"default,omitempty"` + Engine string `yaml:"engine" json:"engine"` + Chart Chart `yaml:"chart" json:"chart"` + Image *Image `yaml:"image,omitempty" json:"image,omitempty"` + Requires Requires `yaml:"requires" json:"requires"` + // Inputs are chart values every deploy must answer. + Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` + Values values.Tree `yaml:"values,omitempty" json:"values,omitempty"` } // Chart carries a name and a version, never a repository: a public catalog diff --git a/internal/catalog/inputs.go b/internal/catalog/inputs.go new file mode 100644 index 0000000..ea9e7ec --- /dev/null +++ b/internal/catalog/inputs.go @@ -0,0 +1,159 @@ +package catalog + +import ( + "fmt" + "math" + "regexp" + "strings" + + "github.com/modelsphere/swiss/internal/values" +) + +// Input is one inputs entry. Its rules repeat the catalog's +// hack/lib/inputs.js: the catalog is untrusted where it is used. +type Input struct { + // Key is the chart value's dotted key, as helm --set takes it: storageKVOffload.mooncake.master. + Key string `yaml:"key" json:"key"` + Title string `yaml:"title" json:"title"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + // Type is string, integer or boolean. Empty is string. + Type string `yaml:"type,omitempty" json:"type,omitempty"` + // Pattern is what a whole string answer matches. + Pattern string `yaml:"pattern,omitempty" json:"pattern,omitempty"` + // Example is the form's placeholder and what catalog CI renders with; never deployed. + Example any `yaml:"example" json:"example"` + // Default answers the input when the site profile does not. + Default any `yaml:"default,omitempty" json:"default,omitempty"` +} + +const ( + InputString = "string" + InputInteger = "integer" + InputBoolean = "boolean" +) + +var inputKey = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_-]*(\.[A-Za-z_][A-Za-z0-9_-]*)*$`) + +// ValidInputKey is shared with the site profile, which keys its answers the same way. +func ValidInputKey(k string) bool { return inputKey.MatchString(k) } + +func (i Input) TypeOrDefault() string { + if i.Type == "" { + return InputString + } + return i.Type +} + +// Check reports why v does not answer i. A whole float64 is an integer: JSON decodes numbers so. +func (i Input) Check(v any) error { + switch t := i.TypeOrDefault(); t { + case InputString: + s, ok := v.(string) + if !ok { + return fmt.Errorf("%v is not a string", v) + } + if i.Pattern == "" { + return nil + } + re, err := i.pattern() + if err != nil { + return err + } + if !re.MatchString(s) { + return fmt.Errorf("%q does not match %s", s, i.Pattern) + } + case InputInteger: + if !isInteger(v) { + return fmt.Errorf("%v is not an integer", v) + } + case InputBoolean: + if _, ok := v.(bool); !ok { + return fmt.Errorf("%v is not a boolean", v) + } + default: + return fmt.Errorf("type %q is not string, integer or boolean", t) + } + return nil +} + +func (i Input) pattern() (*regexp.Regexp, error) { + re, err := regexp.Compile(`^(?:` + i.Pattern + `)$`) + if err != nil { + return nil, fmt.Errorf("pattern %s does not compile: %w", i.Pattern, err) + } + return re, nil +} + +func isInteger(v any) bool { + switch n := v.(type) { + case int, int64, uint64: + return true + case float64: + return n == math.Trunc(n) && !math.IsInf(n, 0) + } + return false +} + +// checkInputs: values leaves every input to compose, which answers it in the derived layer. +func checkInputs(inputs []Input, vals values.Tree) error { + declared := map[string]bool{} + for _, in := range inputs { + where := "inputs " + in.Key + switch { + case !ValidInputKey(in.Key): + return fmt.Errorf("inputs: key %q is not a dotted chart key", in.Key) + case in.Key == "model.localPath": + return fmt.Errorf("%s: swiss answers it from the site's model.pathTemplate, so it is never an input", where) + case declared[in.Key]: + return fmt.Errorf("%s: declared twice", where) + case strings.TrimSpace(in.Title) == "": + return fmt.Errorf("%s: title is required", where) + case in.Pattern != "" && in.TypeOrDefault() != InputString: + return fmt.Errorf("%s: pattern is only for a string input, and this one is %s", where, in.TypeOrDefault()) + } + declared[in.Key] = true + if at := setAt(vals, strings.Split(in.Key, ".")); at != "" { + return fmt.Errorf("%s: values.%s sets it; give the value as the input's default", where, at) + } + if in.Pattern != "" { + if _, err := in.pattern(); err != nil { + return fmt.Errorf("%s: %w", where, err) + } + } + if in.Example == nil { + return fmt.Errorf("%s: example is required", where) + } + if err := in.Check(in.Example); err != nil { + return fmt.Errorf("%s: example %w", where, err) + } + if in.Default != nil { + if err := in.Check(in.Default); err != nil { + return fmt.Errorf("%s: default %w", where, err) + } + } + } + return nil +} + +// setAt is where vals already says something about keys: the value, under it, or a scalar above it. +func setAt(vals values.Tree, keys []string) string { + node := map[string]any(vals) + for i, k := range keys { + next, ok := node[k] + if !ok { + return "" + } + if i == len(keys)-1 { + return strings.Join(keys, ".") + } + switch m := next.(type) { + case map[string]any: + node = m + case values.Tree: + node = m + default: + return strings.Join(keys[:i+1], ".") + } + } + return "" +} diff --git a/internal/catalog/inputs_test.go b/internal/catalog/inputs_test.go new file mode 100644 index 0000000..018a62b --- /dev/null +++ b/internal/catalog/inputs_test.go @@ -0,0 +1,100 @@ +package catalog + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/modelsphere/swiss/internal/values" +) + +var mooncakeMaster = Input{ + Key: "storageKVOffload.mooncake.master", + Title: "Mooncake master", + Pattern: `[^\s:/]+:[0-9]+`, + Example: "mooncake-master.example:50051", +} + +func inputVariant(inputs []Input, vals values.Tree) Variant { + return Variant{ + ID: "a", + Engine: "sglang", + Chart: Chart{Name: "sglang", Version: "0.9.32"}, + Requires: Requires{GPUs: 8}, + Inputs: inputs, + Values: vals, + } +} + +// The same rules as the catalog's validator (hack/lib/inputs.js), because the +// catalog is untrusted where it is used. +func TestAnInputsDefaultAndExampleAreAnswers(t *testing.T) { + withDefault := mooncakeMaster + withDefault.Default = "mc.svc:50051" + for _, in := range []Input{mooncakeMaster, withDefault} { + ok := inputVariant([]Input{in}, values.Tree{"storageKVOffload": map[string]any{"enabled": true, "mooncake": map[string]any{"protocol": "rdma"}}}) + if err := ok.validate(); err != nil { + t.Errorf("default %v: %v", in.Default, err) + } + } + one := func(in Input) []Input { return []Input{in} } + at := func(in Input, edit func(*Input)) Input { edit(&in); return in } + for name, c := range map[string]struct { + v Variant + want string + }{ + "values sets it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": "mc.svc:50051"}}}), "values.storageKVOffload.mooncake.master sets it; give the value as the input's default"}, + "a non-map above it": {inputVariant(one(mooncakeMaster), values.Tree{"storageKVOffload": map[string]any{"mooncake": "off"}}), "values.storageKVOffload.mooncake sets it"}, + "default off pattern": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Default = "no-port" })), nil), `default "no-port" does not match`}, + "a blank default": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Default = "" })), nil), `default "" does not match`}, + "default of a type": {inputVariant(one(Input{Key: "a.b", Title: "t", Type: InputInteger, Example: 1, Default: "8"}), nil), "default 8 is not an integer"}, + "declared twice": {inputVariant([]Input{mooncakeMaster, mooncakeMaster}, nil), "declared twice"}, + "the model path": {inputVariant(one(Input{Key: "model.localPath", Title: "p", Example: "/m"}), nil), "never an input"}, + "not a key": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Key = "a..b" })), nil), "not a dotted chart key"}, + "no title": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Title = " " })), nil), "title is required"}, + "no example": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Example = nil })), nil), "example is required"}, + "example off pattern": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Example = "no-port" })), nil), `example "no-port" does not match`}, + "pattern compiles": {inputVariant(one(at(mooncakeMaster, func(i *Input) { i.Pattern = "(" })), nil), "does not compile"}, + "pattern on integer": {inputVariant(one(Input{Key: "a.b", Title: "t", Type: InputInteger, Pattern: "[0-9]+", Example: 1}), nil), "only for a string input"}, + "example of a type": {inputVariant(one(Input{Key: "a.b", Title: "t", Type: InputInteger, Example: "8"}), nil), "8 is not an integer"}, + "unknown type": {inputVariant(one(Input{Key: "a.b", Title: "t", Type: "list", Example: "x"}), nil), `type "list" is not`}, + } { + if err := c.v.validate(); err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: want an error containing %q, got %v", name, c.want, err) + } + } +} + +// A number read from JSON is a float64: a whole one answers an integer input. +func TestAnIntegerInputTakesAWholeNumberOfAnyKind(t *testing.T) { + port := Input{Key: "a.port", Title: "Port", Type: InputInteger, Example: 1} + for _, v := range []any{8080, int64(8080), float64(8080)} { + if err := port.Check(v); err != nil { + t.Errorf("%T %v: %v", v, v, err) + } + } + if err := port.Check(80.5); err == nil { + t.Error("80.5 answered an integer input") + } +} + +// inputs is a known key of a v2 variant file, read strictly. The index carries +// no inputs: nothing lists or picks a variant by them. +func TestV2InputsAreReadFromTheFile(t *testing.T) { + raw := []byte("apiVersion: catalog.swiss/v2\nkind: Variant\nmodel: m\nid: a\nversion: 1.0.0\nengine: sglang\nchart: {name: sglang, version: \"0.9.32\"}\n" + + "requires:\n gpus: 1\ninputs:\n - {key: storageKVOffload.mooncake.master, title: Mooncake master, example: \"h:1\", default: \"d:1\"}\n") + f := memFetcher{"models/m/a.yaml": raw} + f["index-v2.json"] = []byte(fmt.Sprintf(`{"apiVersion":"catalog.swiss/v2","count":1,"models":[{"name":"m","source":{"hf":"org/m"},"variants":[{"id":"a","version":"1.0.0","path":"models/m/a.yaml","digest":%q,"engine":"sglang","chart":{"name":"sglang","version":"0.9.32"},"requires":{"gpus":1}}]}]}`, contentRef(raw))) + c, err := OpenFetcher(context.Background(), f) + if err != nil { + t.Fatal(err) + } + r, err := c.Resolve(context.Background(), "m", "", "") + if err != nil { + t.Fatal(err) + } + if in := r.Variant.Inputs; len(in) != 1 || in[0].Key != "storageKVOffload.mooncake.master" || in[0].Example != "h:1" || in[0].Default != "d:1" { + t.Errorf("inputs = %+v", in) + } +} diff --git a/internal/catalog/load.go b/internal/catalog/load.go index b67fcbc..f5d02be 100644 --- a/internal/catalog/load.go +++ b/internal/catalog/load.go @@ -352,5 +352,8 @@ func (v Variant) validate() error { return fmt.Errorf("%s: values.lws.size (%v) != requires.nodes (%d)", where, size, v.Requires.NodesOrDefault()) } } + if err := checkInputs(v.Inputs, v.Values); err != nil { + return fmt.Errorf("%s: %w", where, err) + } return nil } diff --git a/internal/catalog/variant_v2.go b/internal/catalog/variant_v2.go index 22d1ba4..377b3a3 100644 --- a/internal/catalog/variant_v2.go +++ b/internal/catalog/variant_v2.go @@ -23,6 +23,7 @@ type VariantV2 struct { Chart Chart `yaml:"chart" json:"chart"` Image *Image `yaml:"image,omitempty" json:"image,omitempty"` Requires Requires `yaml:"requires" json:"requires"` + Inputs []Input `yaml:"inputs,omitempty" json:"inputs,omitempty"` Values values.Tree `yaml:"values,omitempty" json:"values,omitempty"` // Deprecated comes before deleting a file; a string is the reason. Deprecated any `yaml:"deprecated,omitempty" json:"deprecated,omitempty"` @@ -57,6 +58,6 @@ func ParseVariantV2(raw []byte) (*VariantV2, error) { func (d *VariantV2) variant(isDefault bool) Variant { return Variant{ ID: d.ID, Description: d.Description, Link: d.Link, Default: isDefault, - Engine: d.Engine, Chart: d.Chart, Image: d.Image, Requires: d.Requires, Values: d.Values, + Engine: d.Engine, Chart: d.Chart, Image: d.Image, Requires: d.Requires, Inputs: d.Inputs, Values: d.Values, } } diff --git a/internal/compose/compose.go b/internal/compose/compose.go index ee4fda8..4b855d5 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -119,6 +119,9 @@ func Compose(in Input) (*plan.Plan, error) { if err := applyDefaults(siteVals, derivedVals, base, in); err != nil { return nil, err } + if err := applyInputs(derivedVals, siteVals, in); err != nil { + return nil, err + } if res, ok := values.Get(derivedVals, "model.gpuResource"); ok { if err := checkGPUResourceChart(in.Resolved.Variant.Chart.Name, chartVersion, res.(string)); err != nil { return nil, err @@ -169,6 +172,9 @@ func Compose(in Input) (*plan.Plan, error) { CreateNamespace: in.CreateNamespace, Layers: layers, } + if err := checkInputs(in.Resolved.Variant.Inputs, p.Values()); err != nil { + return nil, err + } if err := p.ComputeHash(); err != nil { return nil, err } @@ -379,6 +385,53 @@ func applyDefaults(site, derived, base values.Tree, in Input) error { return nil } +// applyInputs puts each input's answer in the derived layer, under the form's +// overrides: the site's, else the input's default. +func applyInputs(derived, site values.Tree, in Input) error { + for _, input := range in.Resolved.Variant.Inputs { + answer, ok, err := in.Profile.InputAnswer(input.Key, in.Resolved.Source.HF, in.Resolved.Model) + if err != nil { + return err + } + if !ok { + answer, ok = input.Default, input.Default != nil + } + if !ok { + continue + } + if _, set := values.Get(site, input.Key); set { + continue + } + if err := values.Set(derived, input.Key, answer); err != nil { + return err + } + } + return nil +} + +// checkInputs reads the composed values, so an answer from any layer counts. +func checkInputs(inputs []catalog.Input, composed values.Tree) error { + var missing []string + for _, input := range inputs { + v, ok := values.Get(composed, input.Key) + if !ok || v == nil { + missing = append(missing, fmt.Sprintf("%s (%s)", input.Key, input.Title)) + continue + } + if err := input.Check(v); err != nil { + return fmt.Errorf("input %s (%s): %w", input.Key, input.Title, err) + } + } + switch len(missing) { + case 0: + return nil + case 1: + return fmt.Errorf("input %s is not set: set it in the deploy's overrides, or in the site profile's inputs", missing[0]) + default: + return fmt.Errorf("inputs %s are not set: set them in the deploy's overrides, or in the site profile's inputs", strings.Join(missing, ", ")) + } +} + func checkGPUResourceChart(name, version, res string) error { since, ok := GPUResourceSince[name] if !ok { diff --git a/internal/compose/inputs_test.go b/internal/compose/inputs_test.go new file mode 100644 index 0000000..322143c --- /dev/null +++ b/internal/compose/inputs_test.go @@ -0,0 +1,129 @@ +package compose + +import ( + "strings" + "testing" + + "github.com/modelsphere/swiss/internal/catalog" + "github.com/modelsphere/swiss/internal/values" +) + +const masterKey = "storageKVOffload.mooncake.master" + +func inputsInput() Input { + in := testInput() + in.Resolved.Variant.Inputs = []catalog.Input{{ + Key: masterKey, + Title: "Mooncake master", + Pattern: `[^\s:/]+:[0-9]+`, + Example: "mooncake-master.example:50051", + }} + in.Resolved.Variant.Values["storageKVOffload"] = map[string]any{"enabled": true, "mooncake": map[string]any{"protocol": "rdma"}} + return in +} + +// Level one: the site answers, in the derived layer, beside the variant's own +// values under the same section. +func TestTheSiteAnswersAnInputInTheDerivedLayer(t *testing.T) { + in := inputsInput() + in.Profile.Inputs = map[string]any{masterKey: "mc-{{model}}.svc:50051"} + p, err := Compose(in) + if err != nil { + t.Fatal(err) + } + if got, _ := values.Get(p.Values(), masterKey); got != "mc-glm-5.3.svc:50051" { + t.Errorf("%s = %v", masterKey, got) + } + if got := p.LayerOf(masterKey); got != values.LayerDerived { + t.Errorf("answered by %s, want derived", got) + } + if got, _ := values.Get(p.Values(), "storageKVOffload.mooncake.protocol"); got != "rdma" { + t.Errorf("the variant's own mooncake values were lost: protocol = %v", got) + } +} + +// Level two: the deployer overrides any answer, and needs no site answer to. +func TestTheDeployOverridesTheSitesAnswer(t *testing.T) { + for name, site := range map[string]map[string]any{ + "over the site's answer": {masterKey: "site.svc:50051"}, + "with no site answer": nil, + } { + in := inputsInput() + in.Profile.Inputs = site + in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": "mine.svc:50051"}}} + p, err := Compose(in) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if got, _ := values.Get(p.Values(), masterKey); got != "mine.svc:50051" || p.LayerOf(masterKey) != values.LayerForm { + t.Errorf("%s: %s = %v from %s, want the deploy's", name, masterKey, got, p.LayerOf(masterKey)) + } + } +} + +func TestAnUnansweredInputRefusesThePlan(t *testing.T) { + in := inputsInput() + _, err := Compose(in) + if err == nil || !strings.Contains(err.Error(), "input storageKVOffload.mooncake.master (Mooncake master) is not set") { + t.Fatalf("want a refusal naming the input, got %v", err) + } + // "" is checked like any answer, and fails the pattern. + in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": map[string]any{"master": ""}}} + if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), `"" does not match`) { + t.Fatalf("an empty answer was taken: %v", err) + } +} + +func withDefault(in Input, def any) Input { + in.Resolved.Variant.Inputs[0].Default = def + return in +} + +// The input's default answers it in the derived layer when the site does not; +// the deploy's answer beats both. +func TestTheVariantsDefaultIsTheLowestAnswer(t *testing.T) { + for name, c := range map[string]struct { + site, form map[string]any + want string + layer string + }{ + "the default alone": {want: "tuned.svc:50051", layer: values.LayerDerived}, + "the site over it": {site: map[string]any{masterKey: "site.svc:50051"}, want: "site.svc:50051", layer: values.LayerDerived}, + "the deploy over the two": {site: map[string]any{masterKey: "site.svc:50051"}, form: map[string]any{"master": "mine.svc:50051"}, want: "mine.svc:50051", layer: values.LayerForm}, + } { + in := withDefault(inputsInput(), "tuned.svc:50051") + in.Profile.Inputs = c.site + if c.form != nil { + in.Overrides = values.Tree{"storageKVOffload": map[string]any{"mooncake": c.form}} + } + p, err := Compose(in) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if got, _ := values.Get(p.Values(), masterKey); got != c.want || p.LayerOf(masterKey) != c.layer { + t.Errorf("%s: %s = %v from %s, want %s from %s", name, masterKey, got, p.LayerOf(masterKey), c.want, c.layer) + } + } +} + +func TestAnAnswerTheInputDoesNotTakeRefusesThePlan(t *testing.T) { + in := inputsInput() + in.Profile.Inputs = map[string]any{masterKey: "no-port"} + if _, err := Compose(in); err == nil || !strings.Contains(err.Error(), `"no-port" does not match`) { + t.Fatalf("want a refusal quoting the answer, got %v", err) + } +} + +// An answer reaches only a variant that declares its key: unlike extra, a +// site answer never lands in a deploy that did not ask. +func TestASiteAnswerReachesOnlyAVariantThatDeclaresIt(t *testing.T) { + in := testInput() + in.Profile.Inputs = map[string]any{masterKey: "site.svc:50051"} + p, err := Compose(in) + if err != nil { + t.Fatal(err) + } + if _, ok := values.Get(p.Values(), masterKey); ok { + t.Error("a site answer landed in a variant that declares no input") + } +} diff --git a/internal/site/inputs_test.go b/internal/site/inputs_test.go new file mode 100644 index 0000000..5f5b502 --- /dev/null +++ b/internal/site/inputs_test.go @@ -0,0 +1,59 @@ +package site + +import ( + "strings" + "testing" +) + +const inputsHead = "name: prod\nmodel:\n pathTemplate: /models/{{name}}\nnodes:\n gpus: []\n" + +func TestInputAnswersFillTheModelPlaceholders(t *testing.T) { + p, err := Parse([]byte(inputsHead+`inputs: + storageKVOffload.mooncake.master: mooncake-master.svc:50051 + engineCache.hostPath: /data/cache/{{model}}/{{org}} + service.port: 8050 + shm.enabled: true +`), "test") + if err != nil { + t.Fatal(err) + } + for key, want := range map[string]any{ + "storageKVOffload.mooncake.master": "mooncake-master.svc:50051", + "engineCache.hostPath": "/data/cache/glm-5.3/zai-org", + "service.port": 8050, + "shm.enabled": true, + } { + got, ok, err := p.InputAnswer(key, "zai-org/GLM-5.3", "glm-5.3") + if err != nil || !ok || got != want { + t.Errorf("%s = %v (ok %v, err %v), want %v", key, got, ok, err, want) + } + } + if _, ok, _ := p.InputAnswer("not.answered", "zai-org/GLM-5.3", "glm-5.3"); ok { + t.Error("an unanswered key has an answer") + } +} + +// A model with no weights has no repo id for a template to name. +func TestATemplateNamingTheRepoNeedsASource(t *testing.T) { + p := Profile{Inputs: map[string]any{"a.b": "/data/{{name}}", "c.d": "/data/{{model}}"}} + if _, _, err := p.InputAnswer("a.b", "", "mock"); err == nil || !strings.Contains(err.Error(), "no source.hf") { + t.Errorf("want a refusal naming source.hf, got %v", err) + } + if got, _, err := p.InputAnswer("c.d", "", "mock"); err != nil || got != "/data/mock" { + t.Errorf("{{model}} needs no source: %v, %v", got, err) + } +} + +func TestAnAnswerThatCouldNeverBeUsedIsRefused(t *testing.T) { + for name, c := range map[string]struct{ yaml, want string }{ + "unknown placeholder": {"inputs:\n a.b: /x/{{release}}\n", "unknown placeholder"}, + "not a scalar": {"inputs:\n a.b: {host: h}\n", "string, integer or boolean"}, + "no value": {"inputs:\n a.b:\n", "string, integer or boolean"}, + "not a key": {"inputs:\n a..b: x\n", "not a dotted chart key"}, + "extra answers too": {"extra:\n a:\n b: y\ninputs:\n a.b: x\n", "extra sets the same key"}, + } { + if _, err := Parse([]byte(inputsHead+c.yaml), "test"); err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: want an error containing %q, got %v", name, c.want, err) + } + } +} diff --git a/internal/site/profile.go b/internal/site/profile.go index df56d7d..9abece5 100644 --- a/internal/site/profile.go +++ b/internal/site/profile.go @@ -7,7 +7,9 @@ package site import ( "fmt" + "maps" "os" + "slices" "strings" "time" @@ -76,6 +78,10 @@ type Profile struct { // and composes above the catalog. Defaults Defaults `yaml:"defaults,omitempty" json:"defaults"` + // Inputs answer a variant's inputs by key, only for a variant that declares + // the key -- unlike extra. Strings take pathTemplate's placeholders. + Inputs map[string]any `yaml:"inputs,omitempty" json:"inputs,omitempty"` + // Sites are the other clusters' swissd instances, for the nav switcher. // // In the profile rather than swissd's config because it is a fact about the @@ -202,6 +208,46 @@ func (f FeatureDefaults) Resolve() map[string]bool { } } +// checkInputs refuses an answer that could never be used, or a second answer from extra. +func checkInputs(inputs map[string]any, extra values.Tree) error { + for _, key := range slices.Sorted(maps.Keys(inputs)) { + where := fmt.Sprintf("inputs[%q]", key) + if !catalog.ValidInputKey(key) { + return fmt.Errorf("%s: not a dotted chart key", where) + } + switch v := inputs[key].(type) { + case string: + if _, err := fill(v, "org/name", "model"); err != nil { + return fmt.Errorf("%s: %w", where, err) + } + case bool, int, int64, uint64, float64: + default: + return fmt.Errorf("%s: an answer is a string, integer or boolean, not %T", where, v) + } + if _, set := values.Get(extra, key); set { + return fmt.Errorf("%s: extra sets the same key; give the one answer here", where) + } + } + return nil +} + +// InputAnswer is the site's answer to an input, placeholders filled for this model. +func (p Profile) InputAnswer(key, hf, model string) (answer any, ok bool, err error) { + v, ok := p.Inputs[key] + if !ok { + return nil, false, nil + } + s, isString := v.(string) + if !isString { + return v, true, nil + } + out, err := fill(s, hf, model) + if err != nil { + return nil, true, fmt.Errorf("inputs[%q]: %w", key, err) + } + return out, true, nil +} + // DefaultSLOAddress is the in-cluster SLO API when the profile names none. const DefaultSLOAddress = "http://slo-api.llm-scaler.svc:80" @@ -507,6 +553,9 @@ func Parse(raw []byte, origin string) (*Profile, error) { if err := p.Nodes.checkGPUs(); err != nil { return nil, fmt.Errorf("%s: %w", origin, err) } + if err := checkInputs(p.Inputs, p.Extra); err != nil { + return nil, fmt.Errorf("%s: %w", origin, err) + } return &p, nil } @@ -587,14 +636,29 @@ func (p Profile) LocalPath(hf, model string) (string, error) { if v, ok := p.Model.Overrides[model]; ok { return v, nil } - org, name, ok := strings.Cut(hf, "/") - if !ok { + if _, _, ok := strings.Cut(hf, "/"); !ok { return "", fmt.Errorf("source.hf %q is not org/name", hf) } - r := strings.NewReplacer("{{hf}}", hf, "{{org}}", org, "{{name}}", name, "{{model}}", model) - out := r.Replace(p.Model.PathTemplate) + out, err := fill(p.Model.PathTemplate, hf, model) + if err != nil { + return "", fmt.Errorf("model.pathTemplate %w", err) + } + return out, nil +} + +// fill replaces {{hf}} (the repo id), {{org}} and {{name}} (its halves) and {{model}} (the entry name). +func fill(tmpl, hf, model string) (string, error) { + if hf == "" { + for _, ph := range []string{"{{hf}}", "{{org}}", "{{name}}"} { + if strings.Contains(tmpl, ph) { + return "", fmt.Errorf("%q uses %s, and the model has no source.hf", tmpl, ph) + } + } + } + org, name, _ := strings.Cut(hf, "/") + out := strings.NewReplacer("{{hf}}", hf, "{{org}}", org, "{{name}}", name, "{{model}}", model).Replace(tmpl) if strings.Contains(out, "{{") { - return "", fmt.Errorf("model.pathTemplate %q has an unknown placeholder", p.Model.PathTemplate) + return "", fmt.Errorf("%q has an unknown placeholder", tmpl) } return out, nil }