trident-acl-agent: add storm trident-acl-agent E2E test harness, test images, and pipeline

[bfjelds]

Summary

Adds the storm-trident E2E scenario that validates trident-acl-agent end-to-end against real tridentd (while mocking kubelet/Nebraska) on a VM, testing update, rollback, reboot, and commit.

Includes storm code, pipeline, test images, and development documentation on how to run these tests locally.

Context

This is the last step in enabling trident-acl-agent to run updates and rollbacks. Related PRs:

• trident: promote rollback to stable v1; adjust rollback return type to detect no-servicing #729
• trident-acl-agent: rewrite as annotation-based ACL update agent #730
• [this PR] trident-acl-agent: add storm trident-acl-agent E2E test harness, test images, and pipeline #731

Validation

• Validate grpc invocation with full trident-acl-agent tests: https://dev.azure.com/mariner-org/ECF/_build/results?buildId=1176259&view=results

PR details

Storm code

• proxies/ — fake apiserver (serves the Node annotation protocol the agent watches/patches, including a real K8s-compatible watch stream), fake Nebraska/image servers, a minimal kubelet API shim, and an RP client used to drive scenarios and poll status annotations.
• tests/ — update.go (stage/finalize/commit against a real reboot), rollback.go (rollback stage/finalize/commit against a real reboot, plus a regression test that a second rollback against an empty rollback chain is detected as a no-op via servicing_kind rather than reporting a false Success and rebooting again), vm.go/logs.go (VM lifecycle and log collection helpers).
• trident.go, utils/config — scenario wiring and config plumbing specific to the aclagent suite.
• README.md — usage instructions for running the suite locally.

Registered in tools/cmd/storm-trident/main.go alongside storm-trident's other scenarios.

Test images (tests/images)

• baseimg-acl-agent.yaml / updateimg-acl-agent.yaml — VM image configurations for the base and post-update ACL test images.
• testimages.py — wires the update image into the existing COSI-based image build path (the base qcow2 is built via the Makefile target below, not testimages.py, since the qcow2 tooling differs from COSI).

Build tooling

New artifacts/trident-vm-acl-agent-testimage.qcow2 Makefile target for the base image.

Pipeline

New trident-acl-agent-test.yml stage wired into e2e-template.yml, running the aclagent storm scenario in CI.

Docs

docs/Development/Testing/TridentAclAgent-Tests.md documents the suite; Testing.md links to it.

[azure-pipelines]

Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Adds a new Storm E2E scenario to validate trident-acl-agent end-to-end against a real tridentd on a VM (with fake Kubernetes apiserver + Nebraska/image server), along with the VM test images, CI pipeline stage wiring, and developer docs to run the suite locally.

Changes:

• Introduces storm-trident run aclagent scenario with test cases for deploy, update (stage/finalize/reboot/commit), rollback, log collection, and cleanup.
• Adds fake in-process infrastructure (apiserver, Omaha/Nebraska proxy, image server) used by the scenario to drive and observe Node annotation state transitions.
• Adds new ACL-agent VM image configs + Makefile target(s), plus a new CI stage to build images and run the scenario.

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tools/storm/utils/vm/qemu/qemu.go	Adds configurable qcow2 image selection regex for QEMU VM deployment.
tools/storm/aclagent/utils/config/config.go	Defines Storm scenario configuration/flags for the ACL-agent suite.
tools/storm/aclagent/trident.go	Registers the new aclagent Storm scenario and test case sequencing.
tools/storm/aclagent/tests/vm.go	VM deploy/check/cleanup wrappers for QEMU/Azure platforms.
tools/storm/aclagent/tests/update.go	Implements the A/B update E2E flow, including fake services and post-reboot commit validation.
tools/storm/aclagent/tests/rollback.go	Implements rollback E2E flow and regression coverage for no-op rollback.
tools/storm/aclagent/tests/logs.go	Hooks into existing VM log-fetch helper for the scenario.
tools/storm/aclagent/README.md	Scenario usage and behavior documentation.
tools/storm/aclagent/proxies/scenario.go	Scenario YAML schema + validation for driving test steps.
tools/storm/aclagent/proxies/rp.go	“RP client” to patch requests / poll status via the fake apiserver.
tools/storm/aclagent/proxies/nebraska.go	Fake Nebraska/Omaha endpoint for update availability + package metadata.
tools/storm/aclagent/proxies/kubelet.go	Kubelet-ish helper for simulated ready/reboot signaling (currently unused by the main flow).
tools/storm/aclagent/proxies/imageserver.go	Simple HTTP server to serve a real update artifact (e.g. .cosi).
tools/storm/aclagent/proxies/constants.go	Shared constants for annotations/labels and marker defaults.
tools/storm/aclagent/proxies/apiserver.go	Minimal fake Kubernetes apiserver with LIST/WATCH/PATCH support for Node.
tools/go.mod	Adds Kubernetes API dependencies required by the fake apiserver/client types.
tools/go.sum	Updates module sums to reflect new Kubernetes-related dependencies.
tools/cmd/storm-trident/main.go	Registers the aclagent scenario with the storm-trident binary.
tests/images/trident-vm-testimage/README.md	Documents the new ACL-agent VM image variant.
tests/images/trident-vm-testimage/base/updateimg-acl-agent.yaml	MIC config for the ACL-agent update image (.cosi) that enables the agent.
tests/images/trident-vm-testimage/base/baseimg-acl-agent.yaml	MIC config for the ACL-agent base qcow2 image (agent installed, not enabled).
tests/images/testimages.py	Wires the ACL-agent update image into the test-image build plumbing.
Makefile	Adds a new qcow2 build target for the ACL-agent base test image.
docs/Development/Testing/TridentAclAgent-Tests.md	Adds developer documentation for building/running the ACL-agent Storm scenario.
docs/Development/Testing/Testing.md	Links to the new ACL-agent testing documentation.
.pipelines/templates/stages/testing_acl_agent/trident-acl-agent-test.yml	Adds CI stages/jobs to build images and run the ACL-agent Storm scenario.
.pipelines/templates/e2e-template.yml	Wires the new ACL-agent test stage into the E2E template.

Suppressed comments (1)

tools/storm/aclagent/proxies/scenario.go:81

• Validate() currently counts assert-failure-reason as a valid mutually-exclusive step kind, but that step kind is not runnable. If removing assert-failure-reason, update the validation error accordingly so invalid scenarios fail fast with a correct message.

		if step.AssertFailureReason != "" {
			kinds++
		}
		if kinds != 1 {
			return fmt.Errorf("scenario step %d must set exactly one of patch/expect/assert-failure-reason", index)

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (8)

docs/Development/Testing/TridentAclAgent-Tests.md:176

• The "Reboot Choice" section describes shim-based reboot interception, but the harness code treats finalize as a real reboot and waits for SSH to go down/up. The docs should describe the real reboot flow, or the implementation should be updated to actually use a reboot shim.

## Reboot Choice

This scenario uses shim-based reboot interception rather than a full VM
reboot: a `reboot`/`systemctl reboot` shim on `PATH` inside the VM signals the
scenario's controller and exits the agent process instead of actually
rebooting. The scenario then restarts `trident-acl-agent` fresh, exercising
its post-reboot resume logic without tearing down the SSH session or the
in-process fake services. This is less realistic than a full reboot, but it
keeps the test deterministic and fast.

tests/images/trident-vm-testimage/README.md:21

• This section says the ACL-agent image "enables trident-acl-agent.service" and that the scenario writes "localhost" tunnel endpoints, but the base qcow2 config (baseimg-acl-agent.yaml) leaves the service disabled, and the storm harness configures endpoints via --host-endpoint-ip (not localhost reverse tunnels). Updating the README will keep the image docs consistent with the scenario and image configs.

For both, a set of corresponding update images is available. The ACL-agent
variant reuses the servicing-style VM image layout but additionally installs
`trident-acl-agent` and enables `trident-acl-agent.service` so storm ACL-agent
scenarios can drive a real in-guest agent talking to `tridentd`. The image does
not preseed /etc/trident/trident-acl-agent.conf with runner-specific tunnel
ports; the test scenario should SSH in after boot and write the real localhost
proxy endpoints for Nebraska and the Kubernetes API server.

tools/storm/aclagent/README.md:89

• The "Reboot choice" section says the scenario uses a reboot shim to avoid a full reboot, but run-ab-update/run-rollback explicitly wait for SSH to drop and come back, and describe a real systemctl reboot triggered by the agent. The docs should match the harness's real-reboot behavior.

## Reboot choice

This scenario keeps the shim-based reboot interception from the old tester.
That is less realistic than a full VM reboot, but it keeps the test deterministic
and lets the storm runner hold the reverse SSH tunnels and in-process fake services
steady while the agent drives the finalize path.

tools/storm/aclagent/proxies/apiserver.go:285

• handlePatch calls Decoder.DisallowUnknownFields(), but the body is decoded into map[string]any, so unknown fields are never rejected. This can hide mismatches between what the harness expects and what the agent actually patches.

Decode directly into metadataPatch (with DisallowUnknownFields) before applying the patch.

	defer r.Body.Close()
	body := json.NewDecoder(r.Body)
	body.DisallowUnknownFields()
	var raw map[string]any
	if err := body.Decode(&raw); err != nil {
		http.Error(w, fmt.Sprintf("invalid patch body: %v", err), http.StatusBadRequest)
		return
	}

tools/storm/aclagent/README.md:46

• This README describes prepareVmForAclAgent as using localhost:<port> via reverse-SSH-forwarded tunnels and running enable --now, but the harness configures the agent to talk to http://<host-endpoint-ip>:<port> and does an explicit restart (to reconnect between test cases). Keeping the README aligned with the actual harness behavior will reduce confusion when running locally.

`prepareVmForAclAgent` writes `/etc/trident/trident-acl-agent.conf` pointing
at the `localhost:<port>` endpoints storm reverse-SSH-forwards into the VM,
then runs `systemctl enable --now trident-acl-agent.service`. Before that
runs, the service simply isn't started -- no crash-looping, no log noise.

docs/Development/Testing/TridentAclAgent-Tests.md:153

• The "Test Cases" list is out of sync with the current harness: config/service setup happens in run-ab-update/run-rollback (not check-deployment), the scenario includes run-rollback, and the harness performs a real reboot (SSH goes down/up) rather than a shim-based simulated reboot.

1. **deploy-vm** — Copies the base qcow2 image and creates a QEMU VM
2. **check-deployment** — Verifies the VM booted and is accessible via SSH;
   writes `/etc/trident/trident-acl-agent.conf` pointing at the
   `localhost:<port>` endpoints storm reverse-SSH-forwards into the VM, then
   runs `systemctl enable --now trident-acl-agent.service`
3. **run-ab-update** — Starts the fake apiserver and fake Nebraska/Omaha
   endpoints in-process, seeds bootstrap node labels, patches the desired
   update-image label, and waits for `trident-acl-agent` to drive a real
   Trident A/B update to completion (including the shim-based simulated
   reboot)

docs/Development/Testing/TridentAclAgent-Tests.md:159

• The flags table lists --artifacts-dir default as /tmp, but the scenario's TestConfig sets ArtifactsDir default to .. This mismatch can cause local runs to look in the wrong directory by default.

| Flag | Description | Default |
|------|-------------|---------|
| `--artifacts-dir` | Directory containing VM images | `/tmp` |
| `--output-path` | Output directory for logs | `./output` |
| `--platform` | `qemu` or `azure` | `qemu` |
| `--ssh-private-key-path` | Path to SSH private key | `~/.ssh/id_rsa` |
| `--api-server-port` | Port for the fake Kubernetes API server | `18080` |

tools/storm/aclagent/proxies/rp.go:63

• ScenarioStep supports assert-failure-reason (and Scenario.Validate allows it), but RPClient.runStep never handles AssertFailureReason. Any scenario YAML that uses assert-failure-reason will currently fail with "no recognized action" instead of performing the assertion.

Either implement assert-failure-reason handling in the runner, or remove the field (and its validation path) until it’s supported.

func (c *RPClient) runStep(ctx context.Context, index int, step ScenarioStep) (*StepReport, error) {
	switch {
	case step.Patch != nil:
		if err := c.patchNodeRequest(ctx, step.Patch); err != nil {
			return nil, err
		}
		return &StepReport{Index: index, Kind: "patch", Passed: true, Message: "patched fake Node request annotation"}, nil
	case step.Expect != nil:
		return c.expectStatus(ctx, index, step.Expect)
	default:
		return nil, fmt.Errorf("step %d had no recognized action", index)
	}

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (5)

tools/storm/aclagent/utils/config/config.go:4

• ArtifactsDir default is "." here, but other storm scenarios default it to "/tmp" (e.g. tools/storm/servicing/utils/config/config.go:4 and tools/storm/rollback/utils/config/config.go:4). Keeping this consistent avoids surprising behavior when running different scenarios locally and matches the docs’ stated default.

	ArtifactsDir          string `help:"Directory containing artifacts for the VM" default:"."`

tools/storm/utils/vm/qemu/qemu.go:24

• ImagePattern is compiled as a regex (tools/storm/utils/file/file.go:13) and the default pattern contains an unescaped . before qcow2, so it will also match names like ...testimageXqcow2 and can accidentally match unexpected files (or trigger the "multiple files found" error). Escaping the dot makes the default behave as intended.

	SecureBoot   bool   `help:"Enable secure boot for the VM" default:"false"`
	SerialLog    string `help:"Path to the serial log file" default:"/tmp/trident-vm-verity-test.log"`
	ImagePattern string `help:"Regex pattern used to find the base VM image (.qcow2) in the artifacts directory" default:"^trident-vm-.*-testimage.qcow2$"`

tools/storm/aclagent/README.md:46

• This describes prepareVmForAclAgent as using localhost:<port> via reverse-SSH tunnels and running enable --now, but the implementation writes host-IP endpoints (see tools/storm/aclagent/tests/update.go:262-277) and does systemctl enable + systemctl restart (update.go:360-366). Updating this keeps the doc aligned with the actual harness behavior (and the rationale about tunnels not surviving reboot).

`prepareVmForAclAgent` writes `/etc/trident/trident-acl-agent.conf` pointing
at the `localhost:<port>` endpoints storm reverse-SSH-forwards into the VM,
then runs `systemctl enable --now trident-acl-agent.service`. Before that
runs, the service simply isn't started -- no crash-looping, no log noise.

tools/storm/aclagent/README.md:89

• This section still says the scenario uses shim-based reboot interception and depends on reverse SSH tunnels, but the tests wait for a real reboot by polling SSH reachability (tools/storm/aclagent/tests/update.go:403-429) and explicitly avoid tunnels for the fake endpoints (update.go:124-127). This mismatch will confuse anyone debugging reboot-related failures.

This scenario keeps the shim-based reboot interception from the old tester.
That is less realistic than a full VM reboot, but it keeps the test deterministic
and lets the storm runner hold the reverse SSH tunnels and in-process fake services
steady while the agent drives the finalize path.

docs/Development/Testing/TridentAclAgent-Tests.md:142

• The check-deployment description says the config points at localhost:<port> via reverse-SSH forwarding and that it runs enable --now, but the harness uses a host-reachable IP/port (tests/update.go:262-277) and performs systemctl enable + systemctl restart (update.go:360-366). Aligning this documentation matters because it affects how people reason about connectivity across the real reboot.

1. **deploy-vm** — Copies the base qcow2 image and creates a QEMU VM
2. **check-deployment** — Verifies the VM booted and is accessible via SSH;
   writes `/etc/trident/trident-acl-agent.conf` pointing at the
   `localhost:<port>` endpoints storm reverse-SSH-forwards into the VM, then
   runs `systemctl enable --now trident-acl-agent.service`

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (10)

tools/storm/aclagent/proxies/rp.go:83

• Issue: decodeStatus() errors are ignored, so a malformed acl.azure.com/update-status annotation will be silently treated as “no status yet” and the step will just time out, hiding the real failure.

Evidence: status, _ := decodeStatus(node) drops the error.

Suggestion: handle the error explicitly and fail the step with the decode error + raw annotation value so the scenario report is self-diagnosing.

		node, err := c.getNode(ctx)
		if err != nil {
			return nil, err
		}
		status, _ := decodeStatus(node)

tools/storm/aclagent/tests/update.go:128

• Issue: The fake apiserver binds to 0.0.0.0, which exposes the unauthenticated test endpoint on all host interfaces. This is avoidable (and can be surprising on developer machines) since the VM only needs it reachable via HostEndpointIP.

Evidence: ListenAndServe(..., "0.0.0.0:<port>").

Suggestion: bind to testConfig.HostEndpointIP (or another explicit listen address) so the server is only exposed where needed.

	// Bind on all interfaces (not 127.0.0.1) so the VM can reach the fake
	// apiserver directly over the libvirt NAT network at testConfig.HostEndpointIP,
	// instead of relying on reverse SSH tunnels. Tunnels don't survive a real
	// VM reboot; a real host IP does.
	if _, err := apiServer.ListenAndServe(ctx, fmt.Sprintf("0.0.0.0:%d", testConfig.APIServerPort)); err != nil {

tools/storm/aclagent/tests/update.go:158

• Issue: The fake image server binds to 0.0.0.0, exposing a local artifact over unauthenticated HTTP on all host interfaces.

Evidence: imageServer.ListenAndServe(..., "0.0.0.0:<port>").

Suggestion: bind to testConfig.HostEndpointIP so it’s only reachable on the interface the VM uses.

			return fmt.Errorf("failed to hash image %s: %w", imagePath, err)
		}
		imageServer := &stormproxies.ImageServer{ImagePath: imagePath}
		if _, err := imageServer.ListenAndServe(ctx, fmt.Sprintf("0.0.0.0:%d", testConfig.ImageServerPort)); err != nil {
			return fmt.Errorf("failed to start fake image server: %w", err)

tools/storm/aclagent/tests/update.go:173

• Issue: The fake Nebraska endpoint binds to 0.0.0.0, exposing an unauthenticated update server mock on all host interfaces.

Evidence: nebraska.ListenAndServe(..., "0.0.0.0:<port>").

Suggestion: bind to testConfig.HostEndpointIP so it’s only reachable on the interface the VM uses.

		SHA384:      nebraskaSHA384,
		PackageName: nebraskaPackageName,
	}}
	if _, err := nebraska.ListenAndServe(ctx, fmt.Sprintf("0.0.0.0:%d", testConfig.NebraskaPort)); err != nil {
		return fmt.Errorf("failed to start fake Nebraska endpoint: %w", err)

tools/storm/aclagent/tests/rollback.go:42

• Issue: The fake apiserver in rollback binds to 0.0.0.0, exposing an unauthenticated test endpoint on all host interfaces.

Evidence: ListenAndServe(..., "0.0.0.0:<port>").

Suggestion: bind to testConfig.HostEndpointIP (or another explicit listen address) to limit exposure to the interface the VM uses.

	nodeStore := stormproxies.NewNodeStore(stormproxies.NewSeedNode(testConfig.NodeName, map[string]string{}))
	apiServer := stormproxies.NewAPIServer(testConfig.NodeName, nodeStore)
	if _, err := apiServer.ListenAndServe(ctx, fmt.Sprintf("0.0.0.0:%d", testConfig.APIServerPort)); err != nil {
		return fmt.Errorf("failed to start fake apiserver: %w", err)
	}

tools/storm/aclagent/trident.go:72

• Issue: On error paths, runTestCase calls tc.FailFromError(err) but then continues and unconditionally logs the test as “passed”. That makes logs misleading and can mask follow-on errors after a known failure.

Evidence: after tc.FailFromError(err) the function falls through to logrus.Infof("test case '%s' passed"...).

Suggestion: return immediately after FailFromError, and only log “passed” when there was no error.

	if testCaseSpecificConfig.OutputPath != "" {
		testCaseSpecificConfig.OutputPath = filepath.Join(testCaseSpecificConfig.OutputPath, tc.Name())
		if err := os.MkdirAll(testCaseSpecificConfig.OutputPath, 0o755); err != nil {
			tc.FailFromError(err)
		}

docs/Development/Testing/TridentAclAgent-Tests.md:142

• Issue: The doc says check-deployment writes config pointing at localhost:<port> via reverse SSH tunnels, but the harness now configures the VM to talk directly to the test runner at HostEndpointIP:<port> (and avoids tunnels specifically because they don’t survive a real reboot).

Evidence: RunABUpdate binds the fake apiserver on 0.0.0.0 and advertises http://<HostEndpointIP>:<port> to the VM.

Suggestion: update this step description to match the actual networking model (host-reachable IP/port, no reverse tunnels).

2. **check-deployment** — Verifies the VM booted and is accessible via SSH;
   writes `/etc/trident/trident-acl-agent.conf` pointing at the
   `localhost:<port>` endpoints storm reverse-SSH-forwards into the VM, then
   runs `systemctl enable --now trident-acl-agent.service`

docs/Development/Testing/TridentAclAgent-Tests.md:157

• Issue: Documented default for --artifacts-dir doesn’t match the scenario’s flag default.

Evidence: docs list /tmp, but TestConfig.ArtifactsDir defaults to ..

Suggestion: update the doc table default (or the flag default) so they stay consistent.

| `--artifacts-dir` | Directory containing VM images | `/tmp` |

tools/storm/aclagent/README.md:46

• Issue: This README says the scenario uses localhost:<port> endpoints via reverse SSH forwarding, but the harness configures the VM to reach the fake services directly on HostEndpointIP:<port> (and intentionally avoids tunnels for reboot survival).

Evidence: prepareVmForAclAgent writes endpoints using HostEndpointIP.

Suggestion: update the README wording to match the actual networking model.

`prepareVmForAclAgent` writes `/etc/trident/trident-acl-agent.conf` pointing
at the `localhost:<port>` endpoints storm reverse-SSH-forwards into the VM,
then runs `systemctl enable --now trident-acl-agent.service`. Before that
runs, the service simply isn't started -- no crash-looping, no log noise.

tests/images/trident-vm-testimage/README.md:25

• Issue: This README says the scenario should write “localhost proxy endpoints”, but the harness configures the VM to talk to the test runner over a host-reachable IP (HostEndpointIP) rather than SSH-tunneled localhost.

Evidence: prepareVmForAclAgent uses HostEndpointIP for the apiserver/Nebraska endpoints.

Suggestion: update this text to describe writing <host-endpoint-ip>:<port> endpoints (reachable from the VM) instead of localhost.

into it there is no test harness left to `systemctl enable --now` it. Neither
image preseeds /etc/trident/trident-acl-agent.conf with runner-specific
tunnel ports; the test scenario should SSH in after boot and write the real
localhost proxy endpoints for Nebraska and the Kubernetes API server.