[tmpnet] Delegate writing of the flag file to the runtime #3897
Conversation
| // Ensure a min stake duration compatible with testing staking logic | ||
| config.MinStakeDurationKey: "1s", | ||
| } | ||
| defaultFlags.SetDefaults(tmpnet.DefaultE2EFlags()) |
There was a problem hiding this comment.
The e2e flags are now expected to be set on the network rather than applied automatically by tmpnet.
| @@ -762,10 +763,11 @@ func (n *Network) GetNodeURIs() []NodeURI { | |||
| return GetNodeURIs(n.Nodes) | |||
| } | |||
|
|
|||
| // Retrieves bootstrap IPs and IDs for all nodes except the skipped one (this supports | |||
There was a problem hiding this comment.
This cleanup opportunity was discovered in the process of updating how flags were written.
| @@ -323,6 +319,59 @@ func (n *Node) GetUniqueID() string { | |||
| return n.network.UUID + "-" + strings.ToLower(nodeIDString[startIndex:endIndex]) | |||
| } | |||
|
|
|||
| // composeFlags determines the set of flags that should be used to | |||
| // start the node. | |||
| func (n *Node) composeFlags() (FlagsMap, error) { | |||
There was a problem hiding this comment.
These flags are common across runtimes
| @@ -23,17 +23,6 @@ func (n *Node) GetFlagsPath() string { | |||
| return filepath.Join(n.DataDir, "flags.json") | |||
| } | |||
|
|
|||
| func (n *Node) writeFlags(flags FlagsMap) error { | |||
There was a problem hiding this comment.
Another cleanup opportunity - this method was no longer used.
| ) | ||
|
|
||
| type ProcessRuntimeConfig struct { |
There was a problem hiding this comment.
Not sure why this was previously defined in node.go, this seems like a preferable location.
| @@ -144,6 +154,51 @@ func (p *ProcessRuntime) Start(ctx context.Context) error { | |||
| return p.writeMonitoringConfig() | |||
| } | |||
|
|
|||
| func (p *ProcessRuntime) writeFlags() error { | |||
There was a problem hiding this comment.
Flags set in this method are process-specific
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the way node flags are written by delegating that responsibility to the process runtime. Key changes include the introduction of a ProcessRuntimeConfig struct and associated methods (getRuntimeConfig and writeFlags), the removal of the node-level writeFlags function, and adjustments in related tests and defaults to support the new delegation model.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/fixture/tmpnet/process_runtime.go | Introduces ProcessRuntimeConfig, getRuntimeConfig, and writeFlags to handle flag writing. |
| tests/fixture/tmpnet/node_config.go | Removes the node.writeFlags function as flag writing is now delegated. |
| tests/fixture/tmpnet/node.go | Updates the runtime configuration handling and removes the legacy node.writeFlags call in StartNode. |
| tests/fixture/tmpnet/defaults.go | Renames and refactors default flag functions (DefaultTestFlags → DefaultTmpnetFlags, adds DefaultE2EFlags). |
| tests/e2e/e2e_test.go | Updates default flags usage to align with the new DefaultE2EFlags. |
| tests/antithesis/compose.go | Adjusts to use DefaultTmpnetFlags rather than the removed DefaultTestFlags. |
maru-ava
force-pushed
the
tmpnet-move-waitforhealthy
branch
from
9c8d963 to
ce11d86
Compare
This is intended to enable the process and kube runtimes to vary how they compose the flags that configure a given node.
maru-ava
force-pushed
the
tmpnet-runtime-flag-writing
branch
from
2cf574c to
a6d0977
Compare
Why this should be merged
This is intended to enable the process and kube runtimes to vary how they compose the flags that configure a given node.
How this was tested
CI
Need to be documented in RELEASES.md?
N/A
TODO