test: move Redis e2e fixture into utils

[aeron-gh]

/kind cleanup

What this PR does / why we need it:

Moves the Redis provisioning fixture out of the rate-limit test in test/e2e/router/shared.go into test/e2e/utils, with the manifest path as a parameter. ensureRedis stays as a thin wrapper, so the global rate-limit test behaves exactly as before.

Motivation: the KVCache-aware e2e work discussed in #1328 and drafted in #1222 needs Redis provisioning too; this makes the existing fixture reusable instead of tied to one test. Split out first since it is mechanical and reviewable on its own.

Special notes for your reviewer:

Verbatim move, no behavior change. go build and go vet pass on ./test/e2e/....

Does this PR introduce a user-facing change?:

NONE

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hzxuzhonghu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• test/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[HARSHRAJ2789]

Checked the move line by line against origin/main. It is verbatim apart from the manifest path becoming a parameter and the dropped utils. prefixes, the wrapper preserves the old call, and go build ./test/e2e/... and go vet ./test/e2e/... are clean here too. As a move it looks right.

Two things are worth settling before the KVCache suite consumes it, because both are assumptions that were safe while the path was hardcoded and stop being safe once manifestPath is a parameter.

The readiness wait polls the passed namespace, not the one the Deployment was created in. Inside the loop, namespaceToUse falls back to namespace only when the object carries none, so an object that pins its own namespace is created there. The wait afterwards is WaitForDeploymentReady(t, ctx, kubeClient, namespace, redisDeploymentName, ...), which polls Deployments(namespace) (utils.go:98). redis-standalone.yaml sets no namespace: on any of its four objects, so the two agree today and the rate-limit test is unaffected. A fixture manifest that pins one would create in one namespace and poll another, and the failure is a two-minute timeout with no indication that the namespaces disagreed. Capturing the Deployment's namespaceToUse alongside its name would close it.

Only the first Deployment is waited on. redisDeploymentName is set under redisDeploymentName == "", so a manifest with two Deployments returns after the first is ready while the second may still be starting. redis-standalone.yaml has exactly one, so this stays latent for now, but a KVCache fixture that brings up Redis alongside anything else would hit it. Collecting every Deployment and waiting on all of them keeps the contract the name implies.

Neither blocks this PR as a refactor. Raising them here because #1328 and #1222 are the stated consumers, and both are cheaper to fix in the fixture than to debug from a timeout inside a new suite.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[aeron-gh]

ardcoded and stop being safe once manifestPath is a parameter.

The readiness wait polls the passed namespace, not the one the Deployment was created in. Inside the loop, namespaceToUse falls back to namespace only when the object carries none, so an object that pins its own namespace is created there. The wait afterwards is WaitForDeploymentReady(t, ctx, kubeClient, namespace, redisDeploymentName, ...), which polls Deployments(namespace) (utils.go:98). redis-standalone.yaml sets no namespace: on any of its four objects, so the two agree today and the rate-limit test is unaffected. A fixture manifest that pins one would create in one namespace and poll another, and the failure is a two-minute timeout with no indication that the namespaces disagreed. Capturing the Deployment's namespaceToUse alongside its name would close it.

Only the first Deployment is waited on. redisDeploymentName is set under redisDeploymentName == "", so a manifest with two Deployments returns after the first is ready while the second may still be starting. redis-standalone.yaml has exactly one, so this stays latent for now, but a KVCache fixture that brings up Redis alongside anything else would hit it. Collecting every Deployment and waiting on all of them keeps the contract the name implies.

Neither blocks this PR as a refactor. Raising them here because #1328 and #1222 are the stated consumers, and both are cheaper to fix in the fixture than to debug from a timeout inside a new suite.

good catches, both verified, and both are exactly the assumptions that stop holding once the path is a parameter. pushed a commit: the deployment ref is captured after the namespace resolves and before create (so a preexisting deployment still gets waited on, same as today), and the wait now loops over every deployment in the manifest, each polled in the namespace it landed in. redis-standalone behaves the same, one deployment, nothing pinned.

[HARSHRAJ2789]

Checked the commit. Both are closed correctly, and capturing the ref before Create is a better call than what I described, since it keeps the preexisting-object path waiting the way it does today.

One consequence worth naming for the consumers: the wait is now sequential with its own two-minute budget per Deployment, so a fixture with two of them can spend four minutes here before anything is asserted. The router suite currently uses 9m 12s of the 18m target on my machine, so there is room, but it is worth knowing before a KVCache fixture brings up Redis alongside a second component.

[aeron-gh]

Checked the commit. Both are closed correctly, and capturing the ref before Create is a better call than what I described, since it keeps the preexisting-object path waiting the way it does today.

One consequence worth naming for the consumers: the wait is now sequential with its own two-minute budget per Deployment, so a fixture with two of them can spend four minutes here before anything is asserted. The router suite currently uses 9m 12s of the 18m target on my machine, so there is room, but it is worth knowing before a KVCache fixture brings up Redis alongside a second component.

right, worst case additive, though every create happens before the first wait starts, so the second deployment is already rolling while the first is polled and it rarely costs the full budget. if a fixture ever carries two, the waits can go concurrent then. good number to have on record.