From 04de194edc491bcbb50b855fb8cae337fd2fbfb8 Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Fri, 12 Jun 2026 14:21:10 -0500 Subject: [PATCH] docs: update param docs for schema-driven GetParam pattern Signed-off-by: Melvin Hillsman --- docs/architecture.md | 4 +- docs/development.md | 45 ++++++++++--------- docs/guide/01-overview.md | 13 +++--- docs/guide/03-adding-a-workload.md | 4 +- docs/mermaid/01-workload-development.mermaid | 4 +- docs/mermaid/02-contributor-deep-dive.mermaid | 2 +- .../03-orchestrator-run-phases.mermaid | 2 +- 7 files changed, 39 insertions(+), 35 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 7b0cdbd..fe5e391 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -258,9 +258,11 @@ classDiagram class BaseWorkload { +Config WorkloadConfig + +ParamSchema ParamSchema +SSHUser string +SSHPassword string +SSHAuthorizedKeys []string + +GetParam(key) string +VMResources() VMResourceSpec +ExtraVolumes() []Volume +ExtraDisks() []Disk @@ -352,7 +354,7 @@ classDiagram `BaseWorkload` is an embedded struct that provides default implementations for optional interface methods. Concrete workloads embed `BaseWorkload` and override only the methods they need — idiomatic Go composition over inheritance. -`BaseWorkload` also stores SSH credential fields and exposes a `BuildCloudConfig(opts)` helper method that injects SSH user/password/keys into the cloud-init output. Workload subclasses call `w.BuildCloudConfig(opts)` instead of `cloudinit.BuildCloudConfig(opts)` directly, keeping SSH injection as a single cross-cutting concern on the base struct. +`BaseWorkload` also stores SSH credential fields and exposes a `BuildCloudConfig(opts)` helper method that injects SSH user/password/keys into the cloud-init output. Workload subclasses call `w.BuildCloudConfig(opts)` instead of `cloudinit.BuildCloudConfig(opts)` directly, keeping SSH injection as a single cross-cutting concern on the base struct. `BaseWorkload` also stores a `ParamSchema` and provides `GetParam(key)` for schema-driven param lookup — it returns the user's override from `Config.Params` if set, otherwise the schema default. ### Workload Comparison diff --git a/docs/development.md b/docs/development.md index b0470f8..27483f2 100644 --- a/docs/development.md +++ b/docs/development.md @@ -591,7 +591,7 @@ Pattern: 5. Set `RequiresService()` to `true` and provide a `ServiceSpec()` selecting server VMs by the `virtwork/role: server` label that the orchestrator applies automatically. 6. Clients reach servers via the in-cluster DNS name `..svc.cluster.local` — never poll for pod IPs. -The canonical references are `internal/workloads/network.go` (simplest — one port, iperf3) and `internal/workloads/tps.go` (multi-port Service). All workloads support configurable `Params` via the getter-with-default pattern (see [Configurable Params](#going-further-configurable-params) below). +The canonical references are `internal/workloads/network.go` (simplest — one port, iperf3) and `internal/workloads/tps.go` (multi-port Service). All workloads support configurable `Params` via a typed param schema (see [Configurable Params](#going-further-configurable-params) below). ### Going Further: Storage-Backed Workloads @@ -606,38 +606,35 @@ Reference workloads: `disk.go` (single fio mount), `database.go` (PostgreSQL dat ### Going Further: Configurable Params -All workloads expose tunable knobs through `WorkloadConfig.Params map[string]string`. Users set these in YAML config under `workloads..params`. The standard pattern is a nil-safe getter method with a hardcoded default: +All workloads expose tunable knobs through `WorkloadConfig.Params map[string]string`. Users set these in YAML config under `workloads..params`. Each workload declares a typed **param schema** — a slice of `ParamDef` entries that define the key, type, default, and description for every supported param: ```go -func (w *MyWorkload) concurrency() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["concurrency"]; ok && val != "" { - return val - } - } - return "10" +var CPUParamSchema = ParamSchema{ + {Key: "cpu-load-percent", Type: ParamInt, Default: "100", Desc: "Target CPU load percentage for stress-ng (--cpu-load)"}, + {Key: "cpu-method", Type: ParamString, Default: "all", Desc: "CPU stressor method for stress-ng (--cpu-method)"}, } ``` -Then use the getter when building cloud-init content. Convert your systemd unit constant to a template with `%s` placeholders and interpolate with `fmt.Sprintf`: +Set the schema in your constructor on the embedded `BaseWorkload`: ```go -const mySystemdUnitTemplate = `[Unit] -Description=My workload - -[Service] -Type=simple -ExecStart=/usr/bin/my-tool --concurrency %s --duration %s -Restart=always +func NewMyWorkload(cfg config.WorkloadConfig, sshUser, sshPassword string, sshKeys []string) *MyWorkload { + return &MyWorkload{ + BaseWorkload: BaseWorkload{ + Config: cfg, + ParamSchema: MyParamSchema, + SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, + }, + } +} +``` -[Install] -WantedBy=multi-user.target -` +Then call `w.GetParam("key")` to retrieve the value — it returns the user's override if set, otherwise the schema default: +```go func (w *MyWorkload) CloudInitUserdata() (string, error) { - unit := fmt.Sprintf(mySystemdUnitTemplate, w.concurrency(), w.duration()) + unit := fmt.Sprintf(mySystemdUnitTemplate, w.GetParam("concurrency"), w.GetParam("duration")) return w.BuildCloudConfig(CloudConfigOpts{ - // ... WriteFiles: []WriteFile{{ Path: "/etc/systemd/system/virtwork-my-workload.service", Content: unit, @@ -646,6 +643,10 @@ func (w *MyWorkload) CloudInitUserdata() (string, error) { } ``` +`GetParam` panics on unknown keys — this is intentional; it catches typos in workload code at test time. The orchestrator calls `registry.ValidateParams()` before constructing workloads, rejecting unknown keys (with "did you mean?" suggestions) and type-mismatched values at deploy time. + +Five param types are available: `ParamString`, `ParamInt`, `ParamBool`, `ParamList` (semicolon-separated), and `ParamDict` (semicolon-separated `key=value` pairs). Register your workload in `DefaultRegistry()` as a `RegistryEntry` pairing the factory with the schema so validation applies automatically. + Every workload should have a `Context("param wiring")` test block with three cases: 1. **Nil params** — `Params` field omitted, output contains default values diff --git a/docs/guide/01-overview.md b/docs/guide/01-overview.md index aa6e2df..e57f2c4 100644 --- a/docs/guide/01-overview.md +++ b/docs/guide/01-overview.md @@ -25,19 +25,20 @@ The result is a single `Config` struct that every downstream component reads. Se ### 2. The Workload Registry Resolves Your Request -The string `"cpu"` needs to become executable code. Virtwork maintains a **registry** — a map from workload names to factory functions: +The string `"cpu"` needs to become executable code. Virtwork maintains a **registry** — a map from workload names to `RegistryEntry` structs, each pairing a factory function with a typed param schema: ```go registry := workloads.DefaultRegistry() -// registry["cpu"] → func(cfg, opts) → NewCPUWorkload(...) -// registry["memory"] → func(cfg, opts) → NewMemoryWorkload(...) +// registry["cpu"] → RegistryEntry{Factory: ..., ParamSchema: CPUParamSchema} +// registry["memory"] → RegistryEntry{Factory: ..., ParamSchema: MemoryParamSchema} // ... ``` When the orchestrator calls `registry.Get("cpu", cfg, opts...)`, it: -1. Looks up the factory function for `"cpu"` -2. Applies functional options (namespace, SSH credentials, disk size) -3. Returns a `Workload` instance — a **pure data producer** with no I/O +1. Looks up the `RegistryEntry` for `"cpu"` +2. Validates user-supplied params against the entry's schema (rejecting unknown keys and type mismatches) +3. Applies functional options (namespace, SSH credentials, disk size) +4. Returns a `Workload` instance — a **pure data producer** with no I/O ### 3. Cloud-Init Is Generated diff --git a/docs/guide/03-adding-a-workload.md b/docs/guide/03-adding-a-workload.md index d79be33..4620d46 100644 --- a/docs/guide/03-adding-a-workload.md +++ b/docs/guide/03-adding-a-workload.md @@ -552,10 +552,10 @@ Before submitting a new workload, verify: - [ ] Packages used are available in Fedora's default repos (or pre-installed in the golden image, if applicable) - [ ] Systemd unit has `Restart=always` and `WantedBy=multi-user.target` - [ ] Tests cover: Name, packages, systemd unit content, valid YAML, VMResources, defaults for optional methods -- [ ] Tunable values exposed as `params` with getter-with-default methods (see [development.md](../development.md#going-further-configurable-params)) +- [ ] Tunable values declared as a `ParamSchema` with `GetParam()` lookup (see [development.md](../development.md#going-further-configurable-params)) - [ ] Param wiring tests: nil params → defaults, full override, partial override - [ ] New param keys documented in `docs/configuration.md` (YAML example + params table) -- [ ] Registered in `DefaultRegistry()` (`internal/workloads/registry.go`) with a factory function +- [ ] Registered in `DefaultRegistry()` (`internal/workloads/registry.go`) with a `RegistryEntry` pairing factory and `ParamSchema` - [ ] Existing registry/orchestration test counts updated - [ ] `go test ./...` passes - [ ] `go test -race ./...` passes diff --git a/docs/mermaid/01-workload-development.mermaid b/docs/mermaid/01-workload-development.mermaid index 31691a7..99c4dc2 100644 --- a/docs/mermaid/01-workload-development.mermaid +++ b/docs/mermaid/01-workload-development.mermaid @@ -29,7 +29,7 @@ sequenceDiagram Note over Registry, Cluster: Single-VM workload flow (e.g. cpu, memory, disk, chaos-*) CLI ->> Orch: Run(workloadNames, runID) Orch ->> Registry: DefaultRegistry() - Registry -->> Orch: Registry map[name] → factory + Registry -->> Orch: Registry map[name] → RegistryEntry{factory, ParamSchema} Orch ->> Registry: Get("cpu", workloadConfig, opts...) Registry -->> Orch: *CPUWorkload (implements Workload) @@ -123,6 +123,6 @@ sequenceDiagram Note over Workload: Embed BaseWorkload for defaults:
─ VMResources, VMCount, ExtraVolumes,
ExtraDisks, DataVolumeTemplates,
RequiresService, ServiceSpec all have
sensible default implementations - Note over Registry: Register in DefaultRegistry():
"my-workload": func(cfg, opts) Workload {
return NewMyWorkload(cfg, opts.SSHUser, ...)
} + Note over Registry: Register in DefaultRegistry():
"my-workload": RegistryEntry{
Factory: func(cfg, opts) Workload { ... },
ParamSchema: MyParamSchema,
} Note over CloudInit: Build cloud-init via BaseWorkload.BuildCloudConfig:
─ Specify packages to install
─ Write systemd unit files
─ Run commands to enable services
─ SSH credentials are injected automatically diff --git a/docs/mermaid/02-contributor-deep-dive.mermaid b/docs/mermaid/02-contributor-deep-dive.mermaid index 098d116..45f4f47 100644 --- a/docs/mermaid/02-contributor-deep-dive.mermaid +++ b/docs/mermaid/02-contributor-deep-dive.mermaid @@ -53,7 +53,7 @@ sequenceDiagram RO ->> Aud: RecordEvent(execID, "workload_skipped") else Workload enabled RO ->> Reg: Get(name, workloadConfig, opts...) - Reg ->> Reg: Resolve WorkloadFactory + Reg ->> Reg: Resolve RegistryEntry Reg ->> Reg: Build RegistryOpts from functional options
(WithNamespace, WithSSHCredentials, WithDataDiskSize) Reg ->> WL: factory(cfg, registryOpts) WL -->> Reg: Workload instance diff --git a/docs/mermaid/03-orchestrator-run-phases.mermaid b/docs/mermaid/03-orchestrator-run-phases.mermaid index 95ccfff..00883ca 100644 --- a/docs/mermaid/03-orchestrator-run-phases.mermaid +++ b/docs/mermaid/03-orchestrator-run-phases.mermaid @@ -24,7 +24,7 @@ sequenceDiagram RO ->> Plan: planVMs(ctx, execID, runID, names, vmCount, registry, opts) Plan ->> Reg: DefaultRegistry() - Note right of Reg: map[string]WorkloadFactory with 9 workload types + Note right of Reg: map[string]RegistryEntry with 9 workload types loop For each workload name Plan ->> Plan: Check config: workload disabled?