diff --git a/build/golden-image/Containerfile b/build/golden-image/Containerfile index f0b7376..f112004 100644 --- a/build/golden-image/Containerfile +++ b/build/golden-image/Containerfile @@ -21,6 +21,8 @@ RUN dnf install -y \ postgresql-server \ iproute-tc \ iptables-nft \ + netperf \ + python3 \ && dnf clean all \ && rm -rf /var/cache/dnf @@ -30,7 +32,9 @@ RUN which stress-ng && \ which iperf3 && \ which pgbench && \ which tc && \ - which iptables-nft + which iptables-nft && \ + which netserver && \ + which python3 # Expose metadata for KubeVirt compatibility # The containerdisk format expects the disk image to be at /disk/ diff --git a/docs/configuration.md b/docs/configuration.md index d3c1f3e..33b3524 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -198,7 +198,7 @@ Each workload's `params` block accepts string-valued keys. The current parameter | Workload | Key | Default | Effect | |---|---|---|---| -| **tps** | `file-size` | `10M` | Size of the HTTP test file (suffix `K`/`M`/`G`) | +| **tps** | `file-size` | `10M` | Size of the HTTP test file — a positive integer followed by `K`, `M`, or `G` (e.g. `512K`, `10M`, `1G`). Decimal values and binary suffixes like `MiB` are not supported. | | **tps** | `iterations` | `30` | Number of test iterations | | **tps** | `duration` | `60` | Seconds per iteration | | **chaos-disk** | `mount` | `/mnt/data` | Mountpoint of the data disk to fill | diff --git a/internal/workloads/tps.go b/internal/workloads/tps.go index 4485916..de552a8 100644 --- a/internal/workloads/tps.go +++ b/internal/workloads/tps.go @@ -46,6 +46,10 @@ WantedBy=multi-user.target var ErrUnknownTPSRole = errors.New("unexpected tps role; expected 'server' or 'client'") +var ErrInvalidFileSize = errors.New( + "invalid file-size format: must be a positive integer followed by K, M, or G (e.g. '10M')", +) + // 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, @@ -187,49 +191,71 @@ func (w *TPSWorkload) duration() string { return "60" } -func (w *TPSWorkload) fileSizeBytes() string { - raw := w.fileSize() +func parseFileSize(raw string) (num int, suffix string, err error) { raw = strings.TrimSpace(raw) - suffix := raw[len(raw)-1:] + if len(raw) < 2 { + return 0, "", fmt.Errorf("%w: %q", ErrInvalidFileSize, raw) + } + suffix = strings.ToUpper(raw[len(raw)-1:]) numStr := raw[:len(raw)-1] - num, err := strconv.Atoi(numStr) + + switch suffix { + case "K", "M", "G": + default: + return 0, "", fmt.Errorf("%w: %q (unrecognized suffix %q)", ErrInvalidFileSize, raw, raw[len(raw)-1:]) + } + + num, err = strconv.Atoi(numStr) if err != nil { - return "10485760" // 10M fallback + return 0, "", fmt.Errorf("%w: %q (%q is not a valid integer)", ErrInvalidFileSize, raw, numStr) + } + if num <= 0 { + return 0, "", fmt.Errorf("%w: %q (value must be positive)", ErrInvalidFileSize, raw) } - switch strings.ToUpper(suffix) { + + return num, suffix, nil +} + +func (w *TPSWorkload) fileSizeBytes() (string, error) { + num, suffix, err := parseFileSize(w.fileSize()) + if err != nil { + return "", err + } + switch suffix { case "G": - return strconv.Itoa(num * 1024 * 1024 * 1024) + return strconv.Itoa(num * 1024 * 1024 * 1024), nil case "M": - return strconv.Itoa(num * 1024 * 1024) + return strconv.Itoa(num * 1024 * 1024), nil case "K": - return strconv.Itoa(num * 1024) + return strconv.Itoa(num * 1024), nil default: - return raw + return "", fmt.Errorf("%w: %q", ErrInvalidFileSize, w.fileSize()) } } -func (w *TPSWorkload) fileSizeMBCount() string { - raw := w.fileSize() - raw = strings.TrimSpace(raw) - suffix := raw[len(raw)-1:] - numStr := raw[:len(raw)-1] - num, err := strconv.Atoi(numStr) +func (w *TPSWorkload) fileSizeMBCount() (string, error) { + num, suffix, err := parseFileSize(w.fileSize()) if err != nil { - return "10" + return "", err } - switch strings.ToUpper(suffix) { + switch suffix { case "G": - return strconv.Itoa(num * 1024) + return strconv.Itoa(num * 1024), nil case "M": - return numStr + return strconv.Itoa(num), nil case "K": - return "1" // minimum 1MB + return "1", nil default: - return raw + return "", fmt.Errorf("%w: %q", ErrInvalidFileSize, w.fileSize()) } } func (w *TPSWorkload) buildServerUserdata() (string, error) { + mbCount, err := w.fileSizeMBCount() + if err != nil { + return "", fmt.Errorf("tps server: %w", err) + } + serverScript := fmt.Sprintf(`#!/usr/bin/env bash set -e @@ -252,7 +278,7 @@ echo " HTTP: port 8080 (pid $HTTP_PID)" echo " testfile: /srv/virtwork/testfile (%s)" wait -`, w.fileSize(), w.fileSizeMBCount(), w.fileSize()) +`, w.fileSize(), mbCount, w.fileSize()) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"netperf", "python3"}, @@ -276,6 +302,11 @@ wait } func (w *TPSWorkload) buildClientUserdata(namespace string) (string, error) { + fileSizeBytes, err := w.fileSizeBytes() + if err != nil { + return "", fmt.Errorf("tps client: %w", err) + } + dnsName := w.serverDNSName(namespace) clientScript := fmt.Sprintf(`#!/usr/bin/env bash @@ -364,7 +395,7 @@ done echo "==========================================" echo " TPS testing complete" echo "==========================================" -`, dnsName, w.iterations(), w.duration(), w.fileSizeBytes(), w.fileSize()) +`, dnsName, w.iterations(), w.duration(), fileSizeBytes, w.fileSize()) return w.BuildCloudConfig(CloudConfigOpts{ Packages: []string{"netperf", "curl"}, diff --git a/internal/workloads/tps_test.go b/internal/workloads/tps_test.go index 8c62ac1..acc2d18 100644 --- a/internal/workloads/tps_test.go +++ b/internal/workloads/tps_test.go @@ -368,6 +368,50 @@ var _ = Describe("TPSWorkload", func() { }) }) + Context("invalid file-size params", func() { + DescribeTable("should return error for malformed file-size", + func(fileSize string) { + w2 := workloads.NewTPSWorkload(config.WorkloadConfig{ + Enabled: config.BoolPtr(true), + VMCount: 1, + CPUCores: 2, + Memory: "2Gi", + Params: map[string]string{"file-size": fileSize}, + }, "virtwork", "virtwork", "", nil) + + _, err := w2.UserdataForRole("server", "virtwork") + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(workloads.ErrInvalidFileSize)) + + _, err = w2.UserdataForRole("client", "virtwork") + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(workloads.ErrInvalidFileSize)) + }, + Entry("decimal number", "1.5G"), + Entry("multi-char suffix GB", "2GB"), + Entry("binary suffix MiB", "10MiB"), + Entry("suffix only, no number", "M"), + Entry("unknown suffix", "10X"), + Entry("no valid structure", "garbage"), + Entry("zero value", "0M"), + Entry("negative value", "-5M"), + ) + + It("should propagate file-size error through CloudInitUserdata", func() { + w2 := workloads.NewTPSWorkload(config.WorkloadConfig{ + Enabled: config.BoolPtr(true), + VMCount: 1, + CPUCores: 2, + Memory: "2Gi", + Params: map[string]string{"file-size": "garbage"}, + }, "virtwork", "virtwork", "", nil) + + _, err := w2.CloudInitUserdata() + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(workloads.ErrInvalidFileSize)) + }) + }) + Context("configurable params", func() { It("should default file size to 10M", func() { result, err := w.UserdataForRole("server", "virtwork") diff --git a/tests/e2e/tps_test.go b/tests/e2e/tps_test.go new file mode 100644 index 0000000..75e89d4 --- /dev/null +++ b/tests/e2e/tps_test.go @@ -0,0 +1,78 @@ +//go:build e2e + +// Copyright 2026 Red Hat +// SPDX-License-Identifier: Apache-2.0 + +package e2e_test + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/opdev/virtwork/internal/testutil" + "github.com/opdev/virtwork/internal/vm" +) + +var _ = Describe("TPS workload", Label("slow"), func() { + var namespace string + + BeforeEach(func() { + namespace = testutil.UniqueNamespace("e2e-tps") + }) + + AfterEach(func() { + _, _, _, _ = testutil.RunVirtwork("cleanup", + "--namespace", namespace, "--delete-namespace") + }) + + It("should deploy server and client VMs with a service", func() { + stdout, _, exitCode, err := testutil.RunVirtwork( + "run", "--workloads", "tps", "--vm-count", "1", + "--namespace", namespace, "--no-wait") + Expect(err).NotTo(HaveOccurred()) + Expect(exitCode).To(Equal(0)) + Expect(stdout).To(ContainSubstring("virtwork-tps-server-0")) + Expect(stdout).To(ContainSubstring("virtwork-tps-client-0")) + + c := testutil.MustConnect("") + ctx := context.Background() + + svc := &corev1.Service{} + err = c.Get(ctx, client.ObjectKey{ + Name: "virtwork-tps-server", Namespace: namespace, + }, svc) + Expect(err).NotTo(HaveOccurred()) + Expect(svc.Spec.Ports).To(HaveLen(3)) + + vms, err := vm.ListVMs(ctx, c, namespace, testutil.ManagedLabels()) + Expect(err).NotTo(HaveOccurred()) + Expect(vms).To(HaveLen(2)) + }) + + It("should clean up all TPS resources", func() { + _, _, exitCode, err := testutil.RunVirtwork( + "run", "--workloads", "tps", "--vm-count", "1", + "--namespace", namespace, "--no-wait") + Expect(err).NotTo(HaveOccurred()) + Expect(exitCode).To(Equal(0)) + + stdout, _, exitCode, err := testutil.RunVirtwork( + "cleanup", "--namespace", namespace) + Expect(err).NotTo(HaveOccurred()) + Expect(exitCode).To(Equal(0)) + Expect(stdout).To(ContainSubstring(`"vms_deleted":2`)) + + c := testutil.MustConnect("") + ctx := context.Background() + Eventually(func() int { + vms, err := vm.ListVMs(ctx, c, namespace, testutil.ManagedLabels()) + Expect(err).NotTo(HaveOccurred()) + return len(vms) + }, 60*time.Second, 2*time.Second).Should(Equal(0)) + }) +})