From a8eec9e63e4ccb95410ab2d447aced464b9b62bd Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Fri, 12 Jun 2026 11:20:53 -0500 Subject: [PATCH 1/7] feat: add ParamDef types and GetParam on BaseWorkload Introduce ParamType (String, Int, Bool, List, Dict), ParamDef, and ParamSchema types with per-type validation. Add ParamSchema field and GetParam method to BaseWorkload so workloads can look up param values against a declared schema with defaults. Signed-off-by: Melvin Hillsman --- internal/workloads/params.go | 79 ++++++++++ internal/workloads/params_test.go | 247 ++++++++++++++++++++++++++++++ internal/workloads/workload.go | 17 ++ 3 files changed, 343 insertions(+) create mode 100644 internal/workloads/params.go create mode 100644 internal/workloads/params_test.go diff --git a/internal/workloads/params.go b/internal/workloads/params.go new file mode 100644 index 0000000..018492f --- /dev/null +++ b/internal/workloads/params.go @@ -0,0 +1,79 @@ +// Copyright 2026 Red Hat +// SPDX-License-Identifier: Apache-2.0 + +package workloads + +import ( + "fmt" + "strconv" + "strings" +) + +// ParamType identifies the expected value category for a workload param. +type ParamType int + +const ( + ParamString ParamType = iota + ParamInt + ParamBool + ParamList + ParamDict +) + +// ParamDef declares a single configurable param for a workload. +type ParamDef struct { + Key string + Type ParamType + Default string + Desc string +} + +// Validate checks that value conforms to the param's declared type. +func (d *ParamDef) Validate(value string) error { + switch d.Type { + case ParamString: + if value == "" { + return fmt.Errorf("param %q must not be empty", d.Key) + } + case ParamInt: + if _, err := strconv.Atoi(value); err != nil { + return fmt.Errorf("param %q must be an integer, got %q", d.Key, value) + } + case ParamBool: + lower := strings.ToLower(value) + if lower != "true" && lower != "false" { + return fmt.Errorf("param %q must be \"true\" or \"false\", got %q", d.Key, value) + } + case ParamList: + parts := strings.Split(value, ";") + for i, p := range parts { + if p == "" { + return fmt.Errorf("param %q: empty element at position %d", d.Key, i) + } + } + case ParamDict: + parts := strings.Split(value, ";") + for i, p := range parts { + if p == "" { + return fmt.Errorf("param %q: empty element at position %d", d.Key, i) + } + if !strings.Contains(p, "=") { + return fmt.Errorf("param %q: entry %q must contain \"=\"", d.Key, p) + } + } + } + return nil +} + +// ParamSchema is the ordered list of param declarations for a workload. +type ParamSchema []ParamDef + +// Find returns the ParamDef for the given key, or nil if not declared. +func (s ParamSchema) Find(key string) *ParamDef { + for i := range s { + if s[i].Key == key { + return &s[i] + } + } + return nil +} diff --git a/internal/workloads/params_test.go b/internal/workloads/params_test.go new file mode 100644 index 0000000..8d237ba --- /dev/null +++ b/internal/workloads/params_test.go @@ -0,0 +1,247 @@ +// Copyright 2026 Red Hat +// SPDX-License-Identifier: Apache-2.0 + +package workloads_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/opdev/virtwork/internal/config" + "github.com/opdev/virtwork/internal/workloads" +) + +var _ = Describe("ParamSchema", func() { + var schema workloads.ParamSchema + + BeforeEach(func() { + schema = workloads.ParamSchema{ + {Key: "load", Type: workloads.ParamInt, Default: "100", Desc: "load percent"}, + {Key: "method", Type: workloads.ParamString, Default: "all", Desc: "stressor method"}, + {Key: "verbose", Type: workloads.ParamBool, Default: "false", Desc: "enable verbose"}, + {Key: "targets", Type: workloads.ParamList, Default: "a;b;c", Desc: "target list"}, + {Key: "labels", Type: workloads.ParamDict, Default: "k1=v1;k2=v2", Desc: "label map"}, + } + }) + + Describe("Find", func() { + It("returns the ParamDef for a known key", func() { + def := schema.Find("load") + Expect(def).NotTo(BeNil()) + Expect(def.Key).To(Equal("load")) + Expect(def.Type).To(Equal(workloads.ParamInt)) + Expect(def.Default).To(Equal("100")) + }) + + It("returns nil for an unknown key", func() { + Expect(schema.Find("nonexistent")).To(BeNil()) + }) + + It("finds each type correctly", func() { + Expect(schema.Find("method").Type).To(Equal(workloads.ParamString)) + Expect(schema.Find("verbose").Type).To(Equal(workloads.ParamBool)) + Expect(schema.Find("targets").Type).To(Equal(workloads.ParamList)) + Expect(schema.Find("labels").Type).To(Equal(workloads.ParamDict)) + }) + }) + + Describe("Validate", func() { + Context("ParamString", func() { + It("accepts a non-empty string", func() { + def := schema.Find("method") + Expect(def.Validate("matrixprod")).To(Succeed()) + }) + + It("rejects an empty string", func() { + def := schema.Find("method") + Expect(def.Validate("")).To(MatchError(ContainSubstring("must not be empty"))) + }) + }) + + Context("ParamInt", func() { + It("accepts a valid integer", func() { + def := schema.Find("load") + Expect(def.Validate("42")).To(Succeed()) + }) + + It("accepts zero", func() { + def := schema.Find("load") + Expect(def.Validate("0")).To(Succeed()) + }) + + It("accepts negative integers", func() { + def := schema.Find("load") + Expect(def.Validate("-1")).To(Succeed()) + }) + + It("rejects non-numeric strings", func() { + def := schema.Find("load") + Expect(def.Validate("banana")).To(MatchError(ContainSubstring("must be an integer"))) + }) + + It("rejects floats", func() { + def := schema.Find("load") + Expect(def.Validate("3.14")).To(MatchError(ContainSubstring("must be an integer"))) + }) + }) + + Context("ParamBool", func() { + It("accepts true (lowercase)", func() { + def := schema.Find("verbose") + Expect(def.Validate("true")).To(Succeed()) + }) + + It("accepts false (lowercase)", func() { + def := schema.Find("verbose") + Expect(def.Validate("false")).To(Succeed()) + }) + + It("accepts TRUE (uppercase)", func() { + def := schema.Find("verbose") + Expect(def.Validate("TRUE")).To(Succeed()) + }) + + It("accepts False (mixed case)", func() { + def := schema.Find("verbose") + Expect(def.Validate("False")).To(Succeed()) + }) + + It("rejects non-boolean strings", func() { + def := schema.Find("verbose") + Expect(def.Validate("yes")).To(MatchError(ContainSubstring("must be \"true\" or \"false\""))) + }) + + It("rejects numeric boolean equivalents", func() { + def := schema.Find("verbose") + Expect(def.Validate("1")).To(MatchError(ContainSubstring("must be \"true\" or \"false\""))) + }) + }) + + Context("ParamList", func() { + It("accepts a single element", func() { + def := schema.Find("targets") + Expect(def.Validate("alpha")).To(Succeed()) + }) + + It("accepts semicolon-separated elements", func() { + def := schema.Find("targets") + Expect(def.Validate("a;b;c")).To(Succeed()) + }) + + It("rejects empty elements", func() { + def := schema.Find("targets") + Expect(def.Validate("a;;c")).To(MatchError(ContainSubstring("empty element"))) + }) + + It("rejects a trailing semicolon", func() { + def := schema.Find("targets") + Expect(def.Validate("a;b;")).To(MatchError(ContainSubstring("empty element"))) + }) + + It("rejects an empty string", func() { + def := schema.Find("targets") + Expect(def.Validate("")).To(MatchError(ContainSubstring("empty element"))) + }) + }) + + Context("ParamDict", func() { + It("accepts a single key=value pair", func() { + def := schema.Find("labels") + Expect(def.Validate("env=prod")).To(Succeed()) + }) + + It("accepts multiple semicolon-separated pairs", func() { + def := schema.Find("labels") + Expect(def.Validate("k1=v1;k2=v2")).To(Succeed()) + }) + + It("accepts values containing equals signs", func() { + def := schema.Find("labels") + Expect(def.Validate("expr=a=b")).To(Succeed()) + }) + + It("rejects entries without an equals sign", func() { + def := schema.Find("labels") + Expect(def.Validate("noequals")).To(MatchError(ContainSubstring("must contain \"=\""))) + }) + + It("rejects mixed valid and invalid entries", func() { + def := schema.Find("labels") + Expect(def.Validate("k1=v1;bad")).To(MatchError(ContainSubstring("must contain \"=\""))) + }) + + It("rejects empty elements", func() { + def := schema.Find("labels") + Expect(def.Validate("k1=v1;;k2=v2")).To(MatchError(ContainSubstring("empty element"))) + }) + }) + }) +}) + +var _ = Describe("GetParam", func() { + var schema workloads.ParamSchema + + BeforeEach(func() { + schema = workloads.ParamSchema{ + {Key: "load", Type: workloads.ParamInt, Default: "100", Desc: "load percent"}, + {Key: "method", Type: workloads.ParamString, Default: "all", Desc: "stressor method"}, + } + }) + + It("returns the default when Params is nil", func() { + w := workloads.BaseWorkload{ + Config: config.WorkloadConfig{}, + ParamSchema: schema, + } + Expect(w.GetParam("load")).To(Equal("100")) + Expect(w.GetParam("method")).To(Equal("all")) + }) + + It("returns the default when Params is empty", func() { + w := workloads.BaseWorkload{ + Config: config.WorkloadConfig{Params: map[string]string{}}, + ParamSchema: schema, + } + Expect(w.GetParam("load")).To(Equal("100")) + }) + + It("returns the user value when set", func() { + w := workloads.BaseWorkload{ + Config: config.WorkloadConfig{ + Params: map[string]string{"load": "50", "method": "matrixprod"}, + }, + ParamSchema: schema, + } + Expect(w.GetParam("load")).To(Equal("50")) + Expect(w.GetParam("method")).To(Equal("matrixprod")) + }) + + It("returns the default for keys not present in Params", func() { + w := workloads.BaseWorkload{ + Config: config.WorkloadConfig{ + Params: map[string]string{"load": "75"}, + }, + ParamSchema: schema, + } + Expect(w.GetParam("load")).To(Equal("75")) + Expect(w.GetParam("method")).To(Equal("all")) + }) + + It("returns the default when a param value is empty string", func() { + w := workloads.BaseWorkload{ + Config: config.WorkloadConfig{ + Params: map[string]string{"load": ""}, + }, + ParamSchema: schema, + } + Expect(w.GetParam("load")).To(Equal("100")) + }) + + It("panics on an unknown key", func() { + w := workloads.BaseWorkload{ + Config: config.WorkloadConfig{}, + ParamSchema: schema, + } + Expect(func() { w.GetParam("nonexistent") }).To(PanicWith(ContainSubstring("unknown param key"))) + }) +}) diff --git a/internal/workloads/workload.go b/internal/workloads/workload.go index 4918297..e372daf 100644 --- a/internal/workloads/workload.go +++ b/internal/workloads/workload.go @@ -78,11 +78,28 @@ type VMResourceSpec struct { // Embed this struct in concrete workloads to inherit sensible defaults. type BaseWorkload struct { Config config.WorkloadConfig + ParamSchema ParamSchema SSHUser string SSHPassword string SSHAuthorizedKeys []string } +// GetParam returns the user-supplied value for key, or its schema default. +// Panics if key is not declared in ParamSchema (programming error). +func (b *BaseWorkload) GetParam(key string) string { + for i := range b.ParamSchema { + if b.ParamSchema[i].Key == key { + if b.Config.Params != nil { + if val, ok := b.Config.Params[key]; ok && val != "" { + return val + } + } + return b.ParamSchema[i].Default + } + } + panic(fmt.Sprintf("BUG: unknown param key %q — not in schema", key)) +} + // VMResources returns the CPU and memory spec from the workload config. func (b *BaseWorkload) VMResources() VMResourceSpec { return VMResourceSpec{ From 4ff71da0228898d6aca8f56b52e854b2cb1e07b1 Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Fri, 12 Jun 2026 11:25:36 -0500 Subject: [PATCH 2/7] refactor: replace param getters with schema-driven GetParam Replace 27 private getter methods across 9 workloads with GetParam calls backed by per-workload ParamSchema declarations. Each workload now declares an exported XxxParamSchema var and sets it on the embedded BaseWorkload in its constructor. Signed-off-by: Melvin Hillsman --- internal/workloads/chaos_disk.go | 53 +++++++------------------- internal/workloads/chaos_network.go | 29 +++++--------- internal/workloads/chaos_process.go | 41 ++++++-------------- internal/workloads/cpu.go | 27 ++++--------- internal/workloads/database.go | 39 +++++-------------- internal/workloads/disk.go | 59 ++++++----------------------- internal/workloads/memory.go | 37 +++++------------- internal/workloads/network.go | 27 ++++--------- internal/workloads/tps.go | 47 +++++++---------------- 9 files changed, 94 insertions(+), 265 deletions(-) diff --git a/internal/workloads/chaos_disk.go b/internal/workloads/chaos_disk.go index dcce4c3..2ac689f 100644 --- a/internal/workloads/chaos_disk.go +++ b/internal/workloads/chaos_disk.go @@ -54,6 +54,14 @@ RestartSec=10 WantedBy=multi-user.target ` +// ChaosDiskParamSchema declares the configurable params for the chaos-disk workload. +var ChaosDiskParamSchema = ParamSchema{ + {Key: "mount", Type: ParamString, Default: "/mnt/data", Desc: "Mount point for the data disk"}, + {Key: "fill-percent", Type: ParamInt, Default: "90", Desc: "Target fill percentage of the data disk"}, + {Key: "fill-sleep", Type: ParamInt, Default: "60", Desc: "Seconds to hold disk at fill level before releasing"}, + {Key: "release-sleep", Type: ParamInt, Default: "30", Desc: "Seconds to wait after releasing before re-filling"}, +} + // ChaosDiskWorkload generates cloud-init userdata for a disk fill/release chaos // workload using fallocate and dd. type ChaosDiskWorkload struct { @@ -71,6 +79,7 @@ func NewChaosDiskWorkload( return &ChaosDiskWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: ChaosDiskParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -79,42 +88,6 @@ func NewChaosDiskWorkload( } } -func (w *ChaosDiskWorkload) mount() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["mount"]; ok && val != "" { - return val - } - } - return "/mnt/data" -} - -func (w *ChaosDiskWorkload) fillPercent() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["fill-percent"]; ok && val != "" { - return val - } - } - return "90" -} - -func (w *ChaosDiskWorkload) fillSleep() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["fill-sleep"]; ok && val != "" { - return val - } - } - return "60" -} - -func (w *ChaosDiskWorkload) releaseSleep() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["release-sleep"]; ok && val != "" { - return val - } - } - return "30" -} - // Name returns "chaos-disk". func (w *ChaosDiskWorkload) Name() string { return "chaos-disk" @@ -123,12 +96,12 @@ func (w *ChaosDiskWorkload) Name() string { // CloudInitUserdata returns cloud-init YAML that writes a fill/release script // and a systemd service that runs it in a loop. func (w *ChaosDiskWorkload) CloudInitUserdata() (string, error) { - mountPoint := w.mount() + mountPoint := w.GetParam("mount") script := fmt.Sprintf(chaosDiskScriptTemplate, mountPoint, - w.fillPercent(), - w.releaseSleep(), - w.fillSleep()) + w.GetParam("fill-percent"), + w.GetParam("release-sleep"), + w.GetParam("fill-sleep")) unit := chaosDiskSystemdUnit return w.BuildCloudConfig(CloudConfigOpts{ diff --git a/internal/workloads/chaos_network.go b/internal/workloads/chaos_network.go index 19a273e..69b1e4d 100644 --- a/internal/workloads/chaos_network.go +++ b/internal/workloads/chaos_network.go @@ -45,6 +45,12 @@ RestartSec=10 WantedBy=multi-user.target ` +// ChaosNetworkParamSchema declares the configurable params for the chaos-network workload. +var ChaosNetworkParamSchema = ParamSchema{ + {Key: "latency-ms", Type: ParamInt, Default: "100", Desc: "Added latency in milliseconds via tc/netem"}, + {Key: "packet-loss-percent", Type: ParamString, Default: "5.0", Desc: "Packet loss percentage via tc/netem"}, +} + // ChaosNetworkWorkload generates cloud-init userdata for network chaos injection // using tc (traffic control) and netem (network emulation). type ChaosNetworkWorkload struct { @@ -61,6 +67,7 @@ func NewChaosNetworkWorkload( return &ChaosNetworkWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: ChaosNetworkParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -68,24 +75,6 @@ func NewChaosNetworkWorkload( } } -func (w *ChaosNetworkWorkload) latencyMs() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["latency-ms"]; ok && val != "" { - return val - } - } - return "100" -} - -func (w *ChaosNetworkWorkload) packetLossPercent() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["packet-loss-percent"]; ok && val != "" { - return val - } - } - return "5.0" -} - // Name returns "chaos-network". func (w *ChaosNetworkWorkload) Name() string { return "chaos-network" @@ -96,8 +85,8 @@ func (w *ChaosNetworkWorkload) Name() string { // (sch_netem); the start script runs modprobe as a fallback for non-golden images. func (w *ChaosNetworkWorkload) CloudInitUserdata() (string, error) { startScript := fmt.Sprintf(chaosNetworkStartScript, - w.latencyMs(), - w.packetLossPercent()) + w.GetParam("latency-ms"), + w.GetParam("packet-loss-percent")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"iproute-tc"}, diff --git a/internal/workloads/chaos_process.go b/internal/workloads/chaos_process.go index 8d5e298..69f48d2 100644 --- a/internal/workloads/chaos_process.go +++ b/internal/workloads/chaos_process.go @@ -124,6 +124,13 @@ StandardError=journal WantedBy=multi-user.target ` +// ChaosProcessParamSchema declares the configurable params for the chaos-process workload. +var ChaosProcessParamSchema = ParamSchema{ + {Key: "signal", Type: ParamString, Default: "SIGTERM", Desc: "Signal to send to target processes"}, + {Key: "interval", Type: ParamInt, Default: "30", Desc: "Seconds between chaos actions"}, + {Key: "min-pid", Type: ParamInt, Default: "1000", Desc: "Minimum PID to target (skips kernel threads)"}, +} + // ChaosProcessWorkload generates cloud-init userdata for a process chaos workload // that randomly kills non-essential processes to test resilience. type ChaosProcessWorkload struct { @@ -139,6 +146,7 @@ func NewChaosProcessWorkload( return &ChaosProcessWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: ChaosProcessParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -146,33 +154,6 @@ func NewChaosProcessWorkload( } } -func (w *ChaosProcessWorkload) signal() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["signal"]; ok && val != "" { - return val - } - } - return "SIGTERM" -} - -func (w *ChaosProcessWorkload) interval() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["interval"]; ok && val != "" { - return val - } - } - return "30" -} - -func (w *ChaosProcessWorkload) minPid() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["min-pid"]; ok && val != "" { - return val - } - } - return "1000" -} - // Name returns "chaos-process". func (w *ChaosProcessWorkload) Name() string { return "chaos-process" @@ -182,9 +163,9 @@ func (w *ChaosProcessWorkload) Name() string { // and runs it as a systemd service. func (w *ChaosProcessWorkload) CloudInitUserdata() (string, error) { unit := fmt.Sprintf(chaosProcessSystemdUnitTemplate, - w.signal(), - w.interval(), - w.minPid()) + w.GetParam("signal"), + w.GetParam("interval"), + w.GetParam("min-pid")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"procps-ng"}, diff --git a/internal/workloads/cpu.go b/internal/workloads/cpu.go index d54e644..fc279af 100644 --- a/internal/workloads/cpu.go +++ b/internal/workloads/cpu.go @@ -23,6 +23,12 @@ RestartSec=10 WantedBy=multi-user.target ` +// CPUParamSchema declares the configurable params for the CPU workload. +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)"}, +} + // CPUWorkload generates cloud-init userdata for a continuous CPU stress workload // using stress-ng. type CPUWorkload struct { @@ -38,6 +44,7 @@ func NewCPUWorkload( return &CPUWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: CPUParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -45,24 +52,6 @@ func NewCPUWorkload( } } -func (w *CPUWorkload) cpuLoadPercent() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["cpu-load-percent"]; ok && val != "" { - return val - } - } - return "100" -} - -func (w *CPUWorkload) cpuMethod() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["cpu-method"]; ok && val != "" { - return val - } - } - return "all" -} - // Name returns "cpu". func (w *CPUWorkload) Name() string { return "cpu" @@ -71,7 +60,7 @@ func (w *CPUWorkload) Name() string { // CloudInitUserdata returns cloud-init YAML that installs stress-ng and runs a // continuous CPU stress workload via systemd. func (w *CPUWorkload) CloudInitUserdata() (string, error) { - unit := fmt.Sprintf(cpuSystemdUnitTemplate, w.cpuLoadPercent(), w.cpuMethod()) + unit := fmt.Sprintf(cpuSystemdUnitTemplate, w.GetParam("cpu-load-percent"), w.GetParam("cpu-method")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"stress-ng"}, WriteFiles: []WriteFile{ diff --git a/internal/workloads/database.go b/internal/workloads/database.go index 60a8fbb..7ba9c10 100644 --- a/internal/workloads/database.go +++ b/internal/workloads/database.go @@ -86,6 +86,13 @@ RestartSec=10 WantedBy=multi-user.target ` +// DatabaseParamSchema declares the configurable params for the database workload. +var DatabaseParamSchema = ParamSchema{ + {Key: "scale-factor", Type: ParamInt, Default: "50", Desc: "pgbench scale factor (-s)"}, + {Key: "clients", Type: ParamInt, Default: "10", Desc: "Number of concurrent pgbench clients (-c)"}, + {Key: "duration", Type: ParamInt, Default: "300", Desc: "pgbench run duration in seconds (-T)"}, +} + // DatabaseWorkload generates cloud-init userdata for a PostgreSQL database // benchmark workload using pgbench. It formats a data disk, initializes // PostgreSQL, creates a pgbench database at scale 50, and runs continuous @@ -105,6 +112,7 @@ func NewDatabaseWorkload( return &DatabaseWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: DatabaseParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -113,33 +121,6 @@ func NewDatabaseWorkload( } } -func (w *DatabaseWorkload) scaleFactor() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["scale-factor"]; ok && val != "" { - return val - } - } - return "50" -} - -func (w *DatabaseWorkload) clients() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["clients"]; ok && val != "" { - return val - } - } - return "10" -} - -func (w *DatabaseWorkload) duration() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["duration"]; ok && val != "" { - return val - } - } - return "300" -} - // Name returns "database". func (w *DatabaseWorkload) Name() string { return "database" @@ -149,8 +130,8 @@ func (w *DatabaseWorkload) Name() string { // a setup script for one-time database initialization, and creates a systemd // service that runs continuous pgbench benchmarks. func (w *DatabaseWorkload) CloudInitUserdata() (string, error) { - setupScript := fmt.Sprintf(dbSetupScriptTemplate, w.scaleFactor()) - serviceUnit := fmt.Sprintf(dbSystemdUnitTemplate, w.clients(), w.duration()) + setupScript := fmt.Sprintf(dbSetupScriptTemplate, w.GetParam("scale-factor")) + serviceUnit := fmt.Sprintf(dbSystemdUnitTemplate, w.GetParam("clients"), w.GetParam("duration")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"postgresql-server"}, WriteFiles: []WriteFile{ diff --git a/internal/workloads/disk.go b/internal/workloads/disk.go index a5e18ec..f8a34f1 100644 --- a/internal/workloads/disk.go +++ b/internal/workloads/disk.go @@ -58,6 +58,15 @@ RestartSec=10 WantedBy=multi-user.target ` +// DiskParamSchema declares the configurable params for the disk workload. +var DiskParamSchema = ParamSchema{ + {Key: "block-size-rw", Type: ParamString, Default: "4k", Desc: "Block size for random read/write fio profile"}, + {Key: "block-size-seq", Type: ParamString, Default: "128k", Desc: "Block size for sequential write fio profile"}, + {Key: "rwmixread", Type: ParamInt, Default: "70", Desc: "Read percentage in mixed read/write fio profile"}, + {Key: "numjobs", Type: ParamInt, Default: "4", Desc: "Number of parallel fio jobs"}, + {Key: "runtime", Type: ParamInt, Default: "300", Desc: "Runtime in seconds per fio profile run"}, +} + // DiskWorkload generates cloud-init userdata for a disk I/O workload using fio. // It alternates between a 4K random read/write mix and 128K sequential writes. type DiskWorkload struct { @@ -75,6 +84,7 @@ func NewDiskWorkload( return &DiskWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: DiskParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -83,51 +93,6 @@ func NewDiskWorkload( } } -func (w *DiskWorkload) blockSizeRW() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["block-size-rw"]; ok && val != "" { - return val - } - } - return "4k" -} - -func (w *DiskWorkload) blockSizeSeq() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["block-size-seq"]; ok && val != "" { - return val - } - } - return "128k" -} - -func (w *DiskWorkload) rwMixRead() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["rwmixread"]; ok && val != "" { - return val - } - } - return "70" -} - -func (w *DiskWorkload) numJobs() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["numjobs"]; ok && val != "" { - return val - } - } - return "4" -} - -func (w *DiskWorkload) fioRuntime() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["runtime"]; ok && val != "" { - return val - } - } - return "300" -} - // Name returns "disk". func (w *DiskWorkload) Name() string { return "disk" @@ -137,9 +102,9 @@ func (w *DiskWorkload) Name() string { // profiles, and creates a systemd service that alternates between them. func (w *DiskWorkload) CloudInitUserdata() (string, error) { mixedRW := fmt.Sprintf(fioMixedRWProfileTemplate, - w.rwMixRead(), w.blockSizeRW(), w.numJobs(), w.fioRuntime()) + w.GetParam("rwmixread"), w.GetParam("block-size-rw"), w.GetParam("numjobs"), w.GetParam("runtime")) seqWrite := fmt.Sprintf(fioSeqWriteProfileTemplate, - w.blockSizeSeq(), w.fioRuntime()) + w.GetParam("block-size-seq"), w.GetParam("runtime")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"fio"}, WriteFiles: []WriteFile{ diff --git a/internal/workloads/memory.go b/internal/workloads/memory.go index ca175e2..dc05d28 100644 --- a/internal/workloads/memory.go +++ b/internal/workloads/memory.go @@ -23,6 +23,13 @@ RestartSec=10 WantedBy=multi-user.target ` +// MemoryParamSchema declares the configurable params for the memory workload. +var MemoryParamSchema = ParamSchema{ + {Key: "memory-percent", Type: ParamInt, Default: "80", Desc: "Target memory usage percentage for stress-ng (--vm-bytes)"}, + {Key: "vm-stressors", Type: ParamInt, Default: "1", Desc: "Number of VM stressor workers for stress-ng (--vm)"}, + {Key: "vm-method", Type: ParamString, Default: "all", Desc: "Memory stressor method for stress-ng (--vm-method)"}, +} + // MemoryWorkload generates cloud-init userdata for a continuous memory pressure // workload using stress-ng. It uses a single VM worker (--vm 1) targeting 80% // of available memory to produce sustained pressure without triggering OOM kills. @@ -39,6 +46,7 @@ func NewMemoryWorkload( return &MemoryWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: MemoryParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -46,33 +54,6 @@ func NewMemoryWorkload( } } -func (w *MemoryWorkload) memoryPercent() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["memory-percent"]; ok && val != "" { - return val - } - } - return "80" -} - -func (w *MemoryWorkload) vmStressors() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["vm-stressors"]; ok && val != "" { - return val - } - } - return "1" -} - -func (w *MemoryWorkload) vmMethod() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["vm-method"]; ok && val != "" { - return val - } - } - return "all" -} - // Name returns "memory". func (w *MemoryWorkload) Name() string { return "memory" @@ -81,7 +62,7 @@ func (w *MemoryWorkload) Name() string { // CloudInitUserdata returns cloud-init YAML that installs stress-ng and runs a // continuous memory pressure workload via systemd. func (w *MemoryWorkload) CloudInitUserdata() (string, error) { - unit := fmt.Sprintf(memorySystemdUnitTemplate, w.vmStressors(), w.memoryPercent(), w.vmMethod()) + unit := fmt.Sprintf(memorySystemdUnitTemplate, w.GetParam("vm-stressors"), w.GetParam("memory-percent"), w.GetParam("vm-method")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"stress-ng"}, WriteFiles: []WriteFile{ diff --git a/internal/workloads/network.go b/internal/workloads/network.go index a61653a..f8981b8 100644 --- a/internal/workloads/network.go +++ b/internal/workloads/network.go @@ -31,6 +31,12 @@ WantedBy=multi-user.target var ErrUnknownNetworkRole = errors.New("unexpected network role; expected 'server' or 'client'") +// NetworkParamSchema declares the configurable params for the network workload. +var NetworkParamSchema = ParamSchema{ + {Key: "parallel-streams", Type: ParamInt, Default: "4", Desc: "Number of parallel iperf3 streams (-P)"}, + {Key: "test-duration", Type: ParamInt, Default: "60", Desc: "Duration in seconds per iperf3 test (-t)"}, +} + // NetworkWorkload generates cloud-init userdata for an iperf3 network benchmark. // It creates two VMs: a server running iperf3 in listen mode, and a client that // runs bidirectional tests against the server via DNS. A K8s Service routes @@ -50,6 +56,7 @@ func NewNetworkWorkload( return &NetworkWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: NetworkParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -58,24 +65,6 @@ func NewNetworkWorkload( } } -func (w *NetworkWorkload) parallelStreams() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["parallel-streams"]; ok && val != "" { - return val - } - } - return "4" -} - -func (w *NetworkWorkload) testDuration() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["test-duration"]; ok && val != "" { - return val - } - } - return "60" -} - // Name returns "network". func (w *NetworkWorkload) Name() string { return "network" @@ -185,7 +174,7 @@ RestartSec=10 [Install] WantedBy=multi-user.target -`, dnsName, w.testDuration(), w.parallelStreams()) +`, dnsName, w.GetParam("test-duration"), w.GetParam("parallel-streams")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"iperf3"}, diff --git a/internal/workloads/tps.go b/internal/workloads/tps.go index 01a81c5..a25436e 100644 --- a/internal/workloads/tps.go +++ b/internal/workloads/tps.go @@ -50,6 +50,13 @@ var ErrInvalidFileSize = errors.New( "invalid file-size format: must be a positive integer followed by K, M, or G (e.g. '10M')", ) +// TPSParamSchema declares the configurable params for the TPS workload. +var TPSParamSchema = ParamSchema{ + {Key: "file-size", Type: ParamString, Default: "10M", Desc: "Size of the test file for HTTP transfer (e.g. 10M, 1G)"}, + {Key: "iterations", Type: ParamInt, Default: "30", Desc: "Number of test iterations per cycle"}, + {Key: "duration", Type: ParamInt, Default: "60", Desc: "Duration in seconds per netperf test (-l)"}, +} + // TPSWorkload generates cloud-init userdata for a combined TPS benchmark. // It creates server/client VM pairs. The server runs netserver and python3 // http.server. The client runs TCP_RR tests and HTTP GET download loops, @@ -69,6 +76,7 @@ func NewTPSWorkload( return &TPSWorkload{ BaseWorkload: BaseWorkload{ Config: cfg, + ParamSchema: TPSParamSchema, SSHUser: sshUser, SSHPassword: sshPassword, SSHAuthorizedKeys: sshKeys, @@ -170,33 +178,6 @@ func (w *TPSWorkload) serverDNSName(namespace string) string { return fmt.Sprintf("virtwork-tps-server.%s.svc.cluster.local", namespace) } -func (w *TPSWorkload) fileSize() string { - if w.Config.Params != nil { - if size, ok := w.Config.Params["file-size"]; ok && size != "" { - return size - } - } - return "10M" -} - -func (w *TPSWorkload) iterations() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["iterations"]; ok && val != "" { - return val - } - } - return "30" -} - -func (w *TPSWorkload) duration() string { - if w.Config.Params != nil { - if val, ok := w.Config.Params["duration"]; ok && val != "" { - return val - } - } - return "60" -} - func parseFileSize(raw string) (num int, suffix string, err error) { raw = strings.TrimSpace(raw) if len(raw) < 2 { @@ -223,7 +204,7 @@ func parseFileSize(raw string) (num int, suffix string, err error) { } func (w *TPSWorkload) fileSizeBytes() (string, error) { - num, suffix, err := parseFileSize(w.fileSize()) + num, suffix, err := parseFileSize(w.GetParam("file-size")) if err != nil { return "", err } @@ -235,12 +216,12 @@ func (w *TPSWorkload) fileSizeBytes() (string, error) { case "K": return strconv.Itoa(num * 1024), nil default: - return "", fmt.Errorf("%w: %q", ErrInvalidFileSize, w.fileSize()) + return "", fmt.Errorf("%w: %q", ErrInvalidFileSize, w.GetParam("file-size")) } } func (w *TPSWorkload) fileSizeMBCount() (string, error) { - num, suffix, err := parseFileSize(w.fileSize()) + num, suffix, err := parseFileSize(w.GetParam("file-size")) if err != nil { return "", err } @@ -252,7 +233,7 @@ func (w *TPSWorkload) fileSizeMBCount() (string, error) { case "K": return "1", nil default: - return "", fmt.Errorf("%w: %q", ErrInvalidFileSize, w.fileSize()) + return "", fmt.Errorf("%w: %q", ErrInvalidFileSize, w.GetParam("file-size")) } } @@ -284,7 +265,7 @@ echo " HTTP: port 8080 (pid $HTTP_PID)" echo " testfile: /srv/virtwork/testfile (%s)" wait -`, w.fileSize(), mbCount, w.fileSize()) +`, w.GetParam("file-size"), mbCount, w.GetParam("file-size")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"netperf", "python3"}, @@ -401,7 +382,7 @@ done echo "==========================================" echo " TPS testing complete" echo "==========================================" -`, dnsName, w.iterations(), w.duration(), fileSizeBytes, w.fileSize()) +`, dnsName, w.GetParam("iterations"), w.GetParam("duration"), fileSizeBytes, w.GetParam("file-size")) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"netperf", "curl"}, From 36d4e9ee08f674d643f75f263205d2ca7105a97e Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Fri, 12 Jun 2026 11:27:22 -0500 Subject: [PATCH 3/7] feat: extend registry with param schemas and validation Change Registry from map[string]WorkloadFactory to map[string]RegistryEntry, pairing each factory with its ParamSchema. Add ValidateParams (rejects unknown keys with "did you mean?" suggestions, validates values against declared types) and AllParamSchemas (enables future CLI help and doc generation). Signed-off-by: Melvin Hillsman --- internal/workloads/registry.go | 160 +++++++++++++++++----------- internal/workloads/registry_test.go | 96 +++++++++++++++++ 2 files changed, 194 insertions(+), 62 deletions(-) diff --git a/internal/workloads/registry.go b/internal/workloads/registry.go index 1635056..4504a03 100644 --- a/internal/workloads/registry.go +++ b/internal/workloads/registry.go @@ -50,8 +50,14 @@ func WithDataDiskSize(size string) Option { // WorkloadFactory creates a Workload from a WorkloadConfig and resolved options. type WorkloadFactory func(config.WorkloadConfig, *RegistryOpts) Workload -// Registry maps workload names to their factory functions. -type Registry map[string]WorkloadFactory +// RegistryEntry pairs a workload factory with its param schema. +type RegistryEntry struct { + Factory WorkloadFactory + ParamSchema ParamSchema +} + +// Registry maps workload names to their registry entries. +type Registry map[string]RegistryEntry // AllWorkloadNames returns a sorted list of all built-in workload names, // derived from the default registry. @@ -62,72 +68,59 @@ func AllWorkloadNames() []string { // DefaultRegistry returns a Registry pre-populated with all built-in workloads. func DefaultRegistry() Registry { return Registry{ - "chaos-process": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewChaosProcessWorkload( - cfg, - opts.SSHUser, - opts.SSHPassword, - opts.SSHAuthorizedKeys, - ) + "chaos-process": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewChaosProcessWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: ChaosProcessParamSchema, }, - "chaos-network": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewChaosNetworkWorkload( - cfg, - opts.SSHUser, - opts.SSHPassword, - opts.SSHAuthorizedKeys, - ) + "chaos-network": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewChaosNetworkWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: ChaosNetworkParamSchema, }, - "chaos-disk": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewChaosDiskWorkload( - cfg, - opts.DataDiskSize, - opts.SSHUser, - opts.SSHPassword, - opts.SSHAuthorizedKeys, - ) + "chaos-disk": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewChaosDiskWorkload(cfg, opts.DataDiskSize, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: ChaosDiskParamSchema, }, - "cpu": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewCPUWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + "cpu": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewCPUWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: CPUParamSchema, }, - "memory": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewMemoryWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + "memory": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewMemoryWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: MemoryParamSchema, }, - "disk": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewDiskWorkload( - cfg, - opts.DataDiskSize, - opts.SSHUser, - opts.SSHPassword, - opts.SSHAuthorizedKeys, - ) + "disk": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewDiskWorkload(cfg, opts.DataDiskSize, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: DiskParamSchema, }, - "database": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewDatabaseWorkload( - cfg, - opts.DataDiskSize, - opts.SSHUser, - opts.SSHPassword, - opts.SSHAuthorizedKeys, - ) + "database": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewDatabaseWorkload(cfg, opts.DataDiskSize, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: DatabaseParamSchema, }, - "network": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewNetworkWorkload( - cfg, - opts.Namespace, - opts.SSHUser, - opts.SSHPassword, - opts.SSHAuthorizedKeys, - ) + "network": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewNetworkWorkload(cfg, opts.Namespace, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: NetworkParamSchema, }, - "tps": func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewTPSWorkload( - cfg, - opts.Namespace, - opts.SSHUser, - opts.SSHPassword, - opts.SSHAuthorizedKeys, - ) + "tps": { + Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { + return NewTPSWorkload(cfg, opts.Namespace, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + }, + ParamSchema: TPSParamSchema, }, } } @@ -209,7 +202,7 @@ func levenshtein(a, b string) int { // Get retrieves a workload by name, constructing it with the given config and options. // Returns an error listing available names if the workload is not found. func (r Registry) Get(name string, cfg config.WorkloadConfig, opts ...Option) (Workload, error) { - factory, ok := r[name] + entry, ok := r[name] if !ok { return nil, fmt.Errorf( "workload %q not found; available: %s; %w", @@ -226,7 +219,50 @@ func (r Registry) Get(name string, cfg config.WorkloadConfig, opts ...Option) (W opt(resolved) } - return factory(cfg, resolved), nil + return entry.Factory(cfg, resolved), nil +} + +// ValidateParams checks that all param keys are declared in the workload's +// schema and that values conform to their declared types. Returns an error +// with "did you mean?" suggestions for unknown keys. +func (r Registry) ValidateParams(workload string, params map[string]string) error { + entry, ok := r[workload] + if !ok { + return fmt.Errorf("unknown workload %q; %w", workload, ErrWorkloadUnknown) + } + schema := entry.ParamSchema + for key, val := range params { + def := schema.Find(key) + if def == nil { + keys := make([]string, len(schema)) + for i, d := range schema { + keys[i] = d.Key + } + suggestion := closestMatch(key, keys) + if suggestion != "" { + return fmt.Errorf( + "unknown param %q for workload %q (did you mean %q?)", + key, workload, suggestion, + ) + } + return fmt.Errorf("unknown param %q for workload %q", key, workload) + } + if err := def.Validate(val); err != nil { + return fmt.Errorf("param %q for workload %q: %w", key, workload, err) + } + } + return nil +} + +// AllParamSchemas returns the param schema for every registered workload. +func (r Registry) AllParamSchemas() map[string]ParamSchema { + schemas := make(map[string]ParamSchema, len(r)) + for name, entry := range r { + if len(entry.ParamSchema) > 0 { + schemas[name] = entry.ParamSchema + } + } + return schemas } // List returns all registered workload names in sorted order. diff --git a/internal/workloads/registry_test.go b/internal/workloads/registry_test.go index d6bfd63..a3e671c 100644 --- a/internal/workloads/registry_test.go +++ b/internal/workloads/registry_test.go @@ -224,6 +224,102 @@ var _ = Describe("AllWorkloadNames", func() { }) }) +var _ = Describe("ValidateParams", func() { + var reg workloads.Registry + + BeforeEach(func() { + reg = workloads.DefaultRegistry() + }) + + It("should accept valid params for a workload", func() { + err := reg.ValidateParams("cpu", map[string]string{ + "cpu-load-percent": "50", + "cpu-method": "matrixprod", + }) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should accept empty params", func() { + err := reg.ValidateParams("cpu", map[string]string{}) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should reject unknown param keys", func() { + err := reg.ValidateParams("cpu", map[string]string{ + "bogus-key": "42", + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unknown param")) + Expect(err.Error()).To(ContainSubstring("bogus-key")) + }) + + It("should suggest close matches for typos", func() { + err := reg.ValidateParams("cpu", map[string]string{ + "cpu-load-percnt": "50", + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("did you mean")) + Expect(err.Error()).To(ContainSubstring("cpu-load-percent")) + }) + + It("should reject non-integer values for ParamInt keys", func() { + err := reg.ValidateParams("cpu", map[string]string{ + "cpu-load-percent": "banana", + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must be an integer")) + }) + + It("should accept valid integer values", func() { + err := reg.ValidateParams("database", map[string]string{ + "scale-factor": "100", + "clients": "20", + "duration": "600", + }) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should reject empty string values for ParamString keys", func() { + err := reg.ValidateParams("cpu", map[string]string{ + "cpu-method": "", + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must not be empty")) + }) + + It("should return error for unknown workload", func() { + err := reg.ValidateParams("nonexistent", map[string]string{"key": "val"}) + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, workloads.ErrWorkloadUnknown)).To(BeTrue()) + }) +}) + +var _ = Describe("AllParamSchemas", func() { + It("should return schemas for all 9 workloads", func() { + schemas := workloads.DefaultRegistry().AllParamSchemas() + Expect(schemas).To(HaveLen(9)) + Expect(schemas).To(HaveKey("cpu")) + Expect(schemas).To(HaveKey("memory")) + Expect(schemas).To(HaveKey("disk")) + Expect(schemas).To(HaveKey("database")) + Expect(schemas).To(HaveKey("network")) + Expect(schemas).To(HaveKey("tps")) + Expect(schemas).To(HaveKey("chaos-disk")) + Expect(schemas).To(HaveKey("chaos-network")) + Expect(schemas).To(HaveKey("chaos-process")) + }) + + It("should contain correct param count for cpu", func() { + schemas := workloads.DefaultRegistry().AllParamSchemas() + Expect(schemas["cpu"]).To(HaveLen(2)) + }) + + It("should contain correct param count for disk", func() { + schemas := workloads.DefaultRegistry().AllParamSchemas() + Expect(schemas["disk"]).To(HaveLen(5)) + }) +}) + var _ = Describe("ValidateWorkloadNames", func() { It("should accept all valid workload names", func() { err := workloads.ValidateWorkloadNames(workloads.AllWorkloadNames()) From b54c903718e5e6d1a86c6e1ba45b50b63bb13417 Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Fri, 12 Jun 2026 11:28:25 -0500 Subject: [PATCH 4/7] feat: validate workload params against schema at deploy time Wire ValidateParams in the orchestrator before workload construction so invalid param keys and type-mismatched values produce clear errors instead of silently falling through to defaults. Signed-off-by: Melvin Hillsman --- internal/orchestrator/orchestrator.go | 6 ++++ internal/orchestrator/orchestrator_test.go | 38 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index db11b7d..3971a46 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -217,6 +217,12 @@ func (ro *RunOrchestrator) planVMs( } } + if len(wlCfg.Params) > 0 { + if err := registry.ValidateParams(name, wlCfg.Params); err != nil { + return nil, nil, nil, nil, fmt.Errorf("invalid params for workload %q: %w", name, err) + } + } + w, err := registry.Get(name, wlCfg, registryOpts...) if err != nil { return nil, nil, nil, nil, fmt.Errorf("creating workload %q: %w", name, err) diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index e791114..0688551 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -175,6 +175,44 @@ var _ = Describe("RunOrchestrator", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("nonexistent")) }) + + It("should return error for invalid params", func() { + cfg.DryRun = true + cfg.Workloads = map[string]config.WorkloadConfig{ + "cpu": { + Params: map[string]string{ + "cpu-load-percent": "banana", + }, + }, + } + + logger := logging.NewLogger(buf, false) + ro := orchestrator.NewRunOrchestrator(logger, nil, cfg, auditor, buf) + + _, err := ro.Run(ctx, 0, "test-run-id", []string{"cpu"}, 1) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid params")) + Expect(err.Error()).To(ContainSubstring("must be an integer")) + }) + + It("should return error for unknown param keys", func() { + cfg.DryRun = true + cfg.Workloads = map[string]config.WorkloadConfig{ + "cpu": { + Params: map[string]string{ + "bogus-key": "42", + }, + }, + } + + logger := logging.NewLogger(buf, false) + ro := orchestrator.NewRunOrchestrator(logger, nil, cfg, auditor, buf) + + _, err := ro.Run(ctx, 0, "test-run-id", []string{"cpu"}, 1) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unknown param")) + Expect(err.Error()).To(ContainSubstring("bogus-key")) + }) }) Context("normal mode with fake client", func() { From d6a3926ac08d9c30af14c13e268fcae7050c7c29 Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Fri, 12 Jun 2026 11:28:59 -0500 Subject: [PATCH 5/7] style: apply gofumpt and golines formatting Signed-off-by: Melvin Hillsman --- internal/workloads/cpu.go | 7 ++++++- internal/workloads/memory.go | 14 ++++++++++++-- internal/workloads/registry.go | 16 ++++++++++++++-- internal/workloads/tps.go | 7 ++++++- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/internal/workloads/cpu.go b/internal/workloads/cpu.go index fc279af..de732aa 100644 --- a/internal/workloads/cpu.go +++ b/internal/workloads/cpu.go @@ -25,7 +25,12 @@ WantedBy=multi-user.target // CPUParamSchema declares the configurable params for the CPU workload. var CPUParamSchema = ParamSchema{ - {Key: "cpu-load-percent", Type: ParamInt, Default: "100", Desc: "Target CPU load percentage for stress-ng (--cpu-load)"}, + { + 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)"}, } diff --git a/internal/workloads/memory.go b/internal/workloads/memory.go index dc05d28..50954d7 100644 --- a/internal/workloads/memory.go +++ b/internal/workloads/memory.go @@ -25,7 +25,12 @@ WantedBy=multi-user.target // MemoryParamSchema declares the configurable params for the memory workload. var MemoryParamSchema = ParamSchema{ - {Key: "memory-percent", Type: ParamInt, Default: "80", Desc: "Target memory usage percentage for stress-ng (--vm-bytes)"}, + { + Key: "memory-percent", + Type: ParamInt, + Default: "80", + Desc: "Target memory usage percentage for stress-ng (--vm-bytes)", + }, {Key: "vm-stressors", Type: ParamInt, Default: "1", Desc: "Number of VM stressor workers for stress-ng (--vm)"}, {Key: "vm-method", Type: ParamString, Default: "all", Desc: "Memory stressor method for stress-ng (--vm-method)"}, } @@ -62,7 +67,12 @@ func (w *MemoryWorkload) Name() string { // CloudInitUserdata returns cloud-init YAML that installs stress-ng and runs a // continuous memory pressure workload via systemd. func (w *MemoryWorkload) CloudInitUserdata() (string, error) { - unit := fmt.Sprintf(memorySystemdUnitTemplate, w.GetParam("vm-stressors"), w.GetParam("memory-percent"), w.GetParam("vm-method")) + unit := fmt.Sprintf( + memorySystemdUnitTemplate, + w.GetParam("vm-stressors"), + w.GetParam("memory-percent"), + w.GetParam("vm-method"), + ) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"stress-ng"}, WriteFiles: []WriteFile{ diff --git a/internal/workloads/registry.go b/internal/workloads/registry.go index 4504a03..19857ab 100644 --- a/internal/workloads/registry.go +++ b/internal/workloads/registry.go @@ -82,7 +82,13 @@ func DefaultRegistry() Registry { }, "chaos-disk": { Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewChaosDiskWorkload(cfg, opts.DataDiskSize, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + return NewChaosDiskWorkload( + cfg, + opts.DataDiskSize, + opts.SSHUser, + opts.SSHPassword, + opts.SSHAuthorizedKeys, + ) }, ParamSchema: ChaosDiskParamSchema, }, @@ -106,7 +112,13 @@ func DefaultRegistry() Registry { }, "database": { Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewDatabaseWorkload(cfg, opts.DataDiskSize, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + return NewDatabaseWorkload( + cfg, + opts.DataDiskSize, + opts.SSHUser, + opts.SSHPassword, + opts.SSHAuthorizedKeys, + ) }, ParamSchema: DatabaseParamSchema, }, diff --git a/internal/workloads/tps.go b/internal/workloads/tps.go index a25436e..f16a945 100644 --- a/internal/workloads/tps.go +++ b/internal/workloads/tps.go @@ -52,7 +52,12 @@ var ErrInvalidFileSize = errors.New( // TPSParamSchema declares the configurable params for the TPS workload. var TPSParamSchema = ParamSchema{ - {Key: "file-size", Type: ParamString, Default: "10M", Desc: "Size of the test file for HTTP transfer (e.g. 10M, 1G)"}, + { + Key: "file-size", + Type: ParamString, + Default: "10M", + Desc: "Size of the test file for HTTP transfer (e.g. 10M, 1G)", + }, {Key: "iterations", Type: ParamInt, Default: "30", Desc: "Number of test iterations per cycle"}, {Key: "duration", Type: ParamInt, Default: "60", Desc: "Duration in seconds per netperf test (-l)"}, } From 06ea24edbcb51bb01d42b02d84a356e299f8f652 Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Fri, 12 Jun 2026 11:47:35 -0500 Subject: [PATCH 6/7] fix: use wrapped static errors in param validation (err113) Signed-off-by: Melvin Hillsman --- internal/workloads/params.go | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/internal/workloads/params.go b/internal/workloads/params.go index 018492f..7585733 100644 --- a/internal/workloads/params.go +++ b/internal/workloads/params.go @@ -4,11 +4,19 @@ package workloads import ( + "errors" "fmt" "strconv" "strings" ) +var ( + ErrParamEmpty = errors.New("param must not be empty") + ErrParamNotInt = errors.New("param must be an integer") + ErrParamNotBool = errors.New("param must be \"true\" or \"false\"") + ErrParamInvalidElement = errors.New("param has invalid element") +) + // ParamType identifies the expected value category for a workload param. type ParamType int @@ -33,32 +41,47 @@ func (d *ParamDef) Validate(value string) error { switch d.Type { case ParamString: if value == "" { - return fmt.Errorf("param %q must not be empty", d.Key) + return fmt.Errorf("param %q: %w", d.Key, ErrParamEmpty) } case ParamInt: if _, err := strconv.Atoi(value); err != nil { - return fmt.Errorf("param %q must be an integer, got %q", d.Key, value) + return fmt.Errorf("param %q value %q: %w", d.Key, value, ErrParamNotInt) } case ParamBool: lower := strings.ToLower(value) if lower != "true" && lower != "false" { - return fmt.Errorf("param %q must be \"true\" or \"false\", got %q", d.Key, value) + return fmt.Errorf("param %q value %q: %w", d.Key, value, ErrParamNotBool) } case ParamList: parts := strings.Split(value, ";") for i, p := range parts { if p == "" { - return fmt.Errorf("param %q: empty element at position %d", d.Key, i) + return fmt.Errorf( + "param %q position %d empty element: %w", + d.Key, + i, + ErrParamInvalidElement, + ) } } case ParamDict: parts := strings.Split(value, ";") for i, p := range parts { if p == "" { - return fmt.Errorf("param %q: empty element at position %d", d.Key, i) + return fmt.Errorf( + "param %q position %d empty element: %w", + d.Key, + i, + ErrParamInvalidElement, + ) } if !strings.Contains(p, "=") { - return fmt.Errorf("param %q: entry %q must contain \"=\"", d.Key, p) + return fmt.Errorf( + "param %q entry %q must contain \"=\": %w", + d.Key, + p, + ErrParamInvalidElement, + ) } } } From b17ed898d5d1e5b15da15b9abe135dd9a8c459f1 Mon Sep 17 00:00:00 2001 From: Melvin Hillsman Date: Fri, 12 Jun 2026 11:51:30 -0500 Subject: [PATCH 7/7] fix: use wrapped static errors in registry ValidateParams (err113) Signed-off-by: Melvin Hillsman --- internal/workloads/registry.go | 56 ++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 10 deletions(-) diff --git a/internal/workloads/registry.go b/internal/workloads/registry.go index 19857ab..9dfea7f 100644 --- a/internal/workloads/registry.go +++ b/internal/workloads/registry.go @@ -13,7 +13,10 @@ import ( "github.com/opdev/virtwork/internal/constants" ) -var ErrWorkloadUnknown = errors.New("workload not found") +var ( + ErrWorkloadUnknown = errors.New("workload not found") + ErrParamUnknown = errors.New("unknown param") +) // RegistryOpts holds optional parameters for workload construction. // Fields are populated via functional Option values. @@ -70,13 +73,23 @@ func DefaultRegistry() Registry { return Registry{ "chaos-process": { Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewChaosProcessWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + return NewChaosProcessWorkload( + cfg, + opts.SSHUser, + opts.SSHPassword, + opts.SSHAuthorizedKeys, + ) }, ParamSchema: ChaosProcessParamSchema, }, "chaos-network": { Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewChaosNetworkWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + return NewChaosNetworkWorkload( + cfg, + opts.SSHUser, + opts.SSHPassword, + opts.SSHAuthorizedKeys, + ) }, ParamSchema: ChaosNetworkParamSchema, }, @@ -100,13 +113,24 @@ func DefaultRegistry() Registry { }, "memory": { Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewMemoryWorkload(cfg, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + return NewMemoryWorkload( + cfg, + opts.SSHUser, + opts.SSHPassword, + opts.SSHAuthorizedKeys, + ) }, ParamSchema: MemoryParamSchema, }, "disk": { Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewDiskWorkload(cfg, opts.DataDiskSize, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + return NewDiskWorkload( + cfg, + opts.DataDiskSize, + opts.SSHUser, + opts.SSHPassword, + opts.SSHAuthorizedKeys, + ) }, ParamSchema: DiskParamSchema, }, @@ -124,13 +148,25 @@ func DefaultRegistry() Registry { }, "network": { Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewNetworkWorkload(cfg, opts.Namespace, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + return NewNetworkWorkload( + cfg, + opts.Namespace, + opts.SSHUser, + opts.SSHPassword, + opts.SSHAuthorizedKeys, + ) }, ParamSchema: NetworkParamSchema, }, "tps": { Factory: func(cfg config.WorkloadConfig, opts *RegistryOpts) Workload { - return NewTPSWorkload(cfg, opts.Namespace, opts.SSHUser, opts.SSHPassword, opts.SSHAuthorizedKeys) + return NewTPSWorkload( + cfg, + opts.Namespace, + opts.SSHUser, + opts.SSHPassword, + opts.SSHAuthorizedKeys, + ) }, ParamSchema: TPSParamSchema, }, @@ -253,11 +289,11 @@ func (r Registry) ValidateParams(workload string, params map[string]string) erro suggestion := closestMatch(key, keys) if suggestion != "" { return fmt.Errorf( - "unknown param %q for workload %q (did you mean %q?)", - key, workload, suggestion, + "%w %q for workload %q (did you mean %q?)", + ErrParamUnknown, key, workload, suggestion, ) } - return fmt.Errorf("unknown param %q for workload %q", key, workload) + return fmt.Errorf("%w %q for workload %q", ErrParamUnknown, key, workload) } if err := def.Validate(val); err != nil { return fmt.Errorf("param %q for workload %q: %w", key, workload, err)