[tmpnet] Ensure GetNodeURIs returns locally-accessible URIs to ensure kube compatibility #3973
Conversation
Previously it was possible to use TestEnvironment.URIs to access node API endpoints. If a node is running in a kube cluster, however, it won't be accessible from outside that cluster. Adding TestEnvironment.GetLocalNodeURIs and updating callers to use that ensures URIs that will work regardless of the location of nodes and test workloads.
maru-ava
added
testing
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the use of TestEnvironment.URIs with the new GetLocalNodeURIs to ensure that API endpoints are accessible in Kubernetes environments. Key changes include adding the GetLocalNodeURIs function and updating all callers (in fixtures, e2e, and antithesis tests) to use the local URIs instead of public ones.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fixture/tmpnet/utils.go | Added GetLocalNodeURIs function and adjusted GetNodeURIs for better node status checking |
| tests/fixture/tmpnet/network.go | Added wrapper method on Network to use GetLocalNodeURIs |
| tests/fixture/e2e/env.go | Removed URIs field; added GetLocalNodeURIs for TestEnvironment and updated GetRandomNodeURI |
| tests/e2e/x/transfer/virtuous.go | Updated to use local URIs for API calls |
| tests/e2e/p/validator_sets.go | Updated to use local URIs for platformvm client initialization |
| tests/antithesis/config.go | Updated configuration logic to use local URIs |
Comments suppressed due to low confidence (1)
tests/e2e/env.go:257
- Consider adding a check to ensure that 'availableNodes' is not empty before selecting a random node to avoid an index out of range panic.
randomNode := availableNodes[r.Intn(len(availableNodes))]
| @@ -46,8 +46,6 @@ type TestEnvironment struct { | |||
| RootNetworkDir string | |||
| // The directory where the test network configuration is stored | |||
| NetworkDir string | |||
| // URIs used to access the API endpoints of nodes of the network | |||
| URIs []tmpnet.NodeURI | |||
There was a problem hiding this comment.
Requiring access via a function instead of a field ensures the caller is always provided locally-accessible URIs whether nodes are running as processes or as pods in a kube cluster.
tests/fixture/tmpnet/utils.go
Outdated
| // GetNodeURIs returns the URIs of the given nodes provided they are running and not ephemeral. Prefer | ||
| // GetLocalNodeURIs if targeting nodes that could be running in a Kubernetes cluster. |
There was a problem hiding this comment.
Do you think it'd make sense to elaborate on the function doc? From my understanding, GetLocalNodeURIs() returns the URIs of available nodes running locally (e.g. inside a process runtime). However, the current doc for GetNodeURIs() doesn't make clear (IMO) what is offers when compared to GetLocalNodeURIs().
|
Fair points, updated to address review comments, PTAL. Always fine to ask questions, and there are additional strategies for discovering context independently e.g.
|
maru-ava
changed the title
PR Chain: tmpnet+kube
This PR chain enables tmpnet to deploy temporary networks to Kubernetes. Early PRs refactor tmpnet to support the addition in #3615 of a new tmpnet node runtime for kube.
Why this should be merged
Previously it was possible to use
TestEnvironment.URIsto access node API endpoints. If a node is running in a kube cluster, however, it won't be accessible via direct URI from outside that cluster. AddingTestEnvironment.GetNodeURIsand updating callers to use that ensures URIs that will work regardless of the location of nodes and test workloads.How this was tested
CI
Need to be documented in RELEASES.md?
N/A