[tmpnet] Unify start network flag usage between e2e and tmpnetctl #3871
Conversation
maru-ava
force-pushed
the
tmpnet-start-network-vars
branch
4 times, most recently
from
38fbfe6 to
9ca9fdc
Compare
maru-ava
force-pushed
the
tmpnet-start-network-vars
branch
from
9ca9fdc to
d6e4603
Compare
tests/fixture/e2e/flags.go
Outdated
| &vars.startNetwork, | ||
| "start-network", | ||
| false, | ||
| "[optional] start a new network and exit without executing any tests. The new network cannot be reused with --reuse-network. Ignored if either --reuse-network or --stop-network is provided.", |
There was a problem hiding this comment.
Should it just error out if conflicting flags are used instead of ignoring one flag? That would avoid any surprises
There was a problem hiding this comment.
Refactored to create a 'command' out of the 4 flags and validate provision of more than one.
| "network-dir", | ||
| "", | ||
| fmt.Sprintf("[optional] the dir containing the configuration of an existing network to target for testing. Will only be used if --reuse-network is specified. Also possible to configure via the %s env variable.", tmpnet.NetworkDirEnvName), | ||
| fmt.Sprintf("[optional] the dir containing the configuration of an existing network. Will only be used if --reuse-network, --restart-network or --stop-network are specified. Also possible to configure via the %s env variable.", tmpnet.NetworkDirEnvName), |
There was a problem hiding this comment.
Similar here, maybe we could error out if that flag is used without one of the 3 required flags mentioned?
There was a problem hiding this comment.
Updated to actually source a value from the env var mentioned in the doc string, which means someone having a value set in their env could run afoul of non-empty validation.
maru-ava
force-pushed
the
tmpnet-refactor-runtime-config
branch
from
7db5b52 to
d2c2a0e
Compare
maru-ava
force-pushed
the
tmpnet-start-network-vars
branch
2 times, most recently
from
13c3451 to
0ccb028
Compare
maru-ava
force-pushed
the
tmpnet-start-network-vars
branch
2 times, most recently
from
d736aec to
af712ec
Compare
maru-ava
force-pushed
the
tmpnet-start-network-vars
branch
from
af712ec to
196fd09
Compare
|
Rebased |
| ) | ||
| } | ||
|
|
||
| func (v *StartNetworkVars) GetNodeCount() (int, error) { |
There was a problem hiding this comment.
(No action needed) Could/would it make more sense to validate the node count earlier when it's initialized so that we don't need to check it's value and possibly return an error here?
There was a problem hiding this comment.
There is no earlier place to validate it.
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
Cleans up flag usage and configuration network start to ensure consistency and simplify maintenance across e2e and tmpnetctl.
How this was tested
CI
Need to be documented in RELEASES.md?
N/A
TODO