[tmpnet] Avoid serializing the node data directory #3881

maru-ava · 2025-04-11T21:46:20Z

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

This is intended to simplify usage by CLI. It also moves responsibility for setting the data dir to the runtime since the dir will differ between the node (local fs) and kube runtimes (pvc).

How this was tested

CI, local testing of tmpnetctl

Need to be documented in RELEASES.md?

N/A

TODO

Merge [tmpnet] Unify start network flag usage between e2e and tmpnetctl #3871

maru-ava · 2025-04-11T21:48:30Z

tests/fixture/tmpnet/network.go

@@ -768,16 +757,13 @@ func (n *Network) GetNodeURIs() []NodeURI {
 // collecting the bootstrap details for restarting a node).
 // For consumption outside of avalanchego. Needs to be kept exported.
 func (n *Network) GetBootstrapIPsAndIDs(skippedNode *Node) ([]string, []string, error) {
-	// Collect staking addresses of non-ephemeral nodes for use in bootstrapping a node
-	nodes, err := ReadNodes(n, false /* includeEphemeral */)


I don't think there's much value in forcing a read here. The network can always be reloaded to achieve a similar result.

maru-ava · 2025-04-11T21:52:00Z

tests/fixture/tmpnet/node_config.go

-func (n *Node) readConfig() error {
-	bytes, err := os.ReadFile(n.getConfigPath())
+func (n *Node) readConfig(dataDir string) error {
+	configPath := getNodeConfigPath(dataDir)


When read, the node is provided the path where it is stored

maru-ava · 2025-04-11T22:59:25Z

tests/e2e/faultinjection/duplicate_node_id.go

+		// Construct a unique data dir to ensure the two nodes' data will be stored
+		// separately. Usually the dir name is the node ID but in this one case the nodes have
+		// the same node ID.
+		node2.DataDir = filepath.Join(network.Dir, node1.DataDir+"second")


If not for this test, the node data dir could always be computed as [network dir]/[node id]

maru-ava · 2025-04-11T22:59:58Z

tests/fixture/e2e/helpers.go

@@ -133,10 +133,9 @@ func NewEthClient(tc tests.TestContext, nodeURI tmpnet.NodeURI) ethclient.Client
 }

 // Adds an ephemeral node intended to be used by a single test.
-func AddEphemeralNode(tc tests.TestContext, network *tmpnet.Network, flags tmpnet.FlagsMap) *tmpnet.Node {
+func AddEphemeralNode(tc tests.TestContext, network *tmpnet.Network, node *tmpnet.Node) *tmpnet.Node {


The need to be able to set the DataDir suggested accepting a node instead of a FlagsMap here.

maru-ava · 2025-04-11T23:00:44Z

tests/fixture/tmpnet/network.go

@@ -240,13 +240,6 @@ func (n *Network) EnsureDefaultConfig(log logging.Logger) error {
 		n.PrimaryChainConfigs[alias].SetDefaults(chainConfig)
 	}

-	// Ensure nodes are configured


Discovered during testing that this configuration is duplicative - it's already being done on network bootstrap, node start, and node read

maru-ava · 2025-04-11T23:02:15Z

tests/fixture/tmpnet/network.go

 		}
-		flags.SetDefault(config.PluginDirKey, processConfig.PluginDir)
+
+		flags.SetDefault(config.DataDirKey, node.DataDir)


The kube runtime will set its own data dir since it uses a different filesystem

maru-ava · 2025-04-11T23:56:38Z

tests/fixture/tmpnet/network_config.go

-	DefaultFlags         FlagsMap                `json:",omitempty"`
-	DefaultRuntimeConfig NodeRuntimeConfig       `json:",omitempty"`
-	PreFundedKeys        []*secp256k1.PrivateKey `json:",omitempty"`
+	UUID                 string                  `json:"uuid,omitempty"`


Reviewing the serialized json, realized that fields should all be in lowerCamelCase

Copilot

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

tests/e2e/helpers.go:136

[nitpick] The AddEphemeralNode function signature now takes a node pointer rather than flags. Consider updating its documentation comment to reflect this change for clarity.

func AddEphemeralNode(tc tests.TestContext, network *tmpnet.Network, node *tmpnet.Node) *tmpnet.Node {

tests/fixture/tmpnet/network.go:550

Ensure that node.DataDir is always properly initialized before this check, as the removal of GetDataDir may lead to unintended empty values in some cases.

if len(node.DataDir) == 0 {

tests/e2e/faultinjection/duplicate_node_id.go:43

[nitpick] Since duplicate node IDs require an explicitly modified DataDir, consider adding an inline comment to explain the rationale behind manually overriding the DataDir for clarity.

node2.DataDir = node1.DataDir + "-second"

michaelkaplan13

The code all LGTM.

Just one high level understanding question on the intended usage: Previously, a Node's data directory was included in its Flags, which would get serialized when Write was called for a given Network, and now that's not the case. How does that simplify the CLI's usage of tmpnet? Is it that networks/nodes can be restarted using different data directories than before?

maru-ava · 2025-04-16T16:04:41Z

The code all LGTM.

Just one high level understanding question on the intended usage: Previously, a Node's data directory was included in its Flags, which would get serialized when Write was called for a given Network, and now that's not the case. How does that simplify the CLI's usage of tmpnet? Is it that networks/nodes can be restarted using different data directories than before?

Yes, CLI supports moving the directory of an existing network to a different location, so including the data dir meant that CLI needed to support rewriting those paths. With this change, that rewriting is no longer necessary.

Edit: A side-effect of the change is better separation between node flags for the process runtime and the kube runtime since they will use different data dirs. Updated the description to reflect this.

This is intended to simplify usage by CLI.

maru-ava added the tooling Build, test and development tooling label Apr 11, 2025

maru-ava self-assigned this Apr 11, 2025

maru-ava added this to avalanchego Apr 11, 2025

github-project-automation bot moved this to Backlog 🗄️ in avalanchego Apr 11, 2025

maru-ava commented Apr 11, 2025

View reviewed changes

maru-ava force-pushed the tmpnet-implicit-data-dir branch from d07f5ad to ddeec15 Compare April 11, 2025 21:49

maru-ava commented Apr 11, 2025

View reviewed changes

maru-ava moved this from Backlog 🗄️ to In Review 👀 in avalanchego Apr 11, 2025

maru-ava force-pushed the tmpnet-implicit-data-dir branch from ddeec15 to 4795146 Compare April 11, 2025 22:57

maru-ava commented Apr 11, 2025

View reviewed changes

maru-ava force-pushed the tmpnet-implicit-data-dir branch 2 times, most recently from b5d8d9d to d320e14 Compare April 11, 2025 23:55

maru-ava commented Apr 11, 2025

View reviewed changes

maru-ava mentioned this pull request Apr 12, 2025

[tmpnet] Enable runtime-specific restart behavior #3882

Open

1 task

maru-ava requested review from felipemadero and Copilot April 12, 2025 20:18

Copilot AI reviewed Apr 12, 2025

View reviewed changes

maru-ava force-pushed the tmpnet-start-network-vars branch from 0ccb028 to d736aec Compare April 12, 2025 21:22

maru-ava force-pushed the tmpnet-implicit-data-dir branch 3 times, most recently from ca83ff8 to 465cd68 Compare April 12, 2025 22:22

maru-ava mentioned this pull request Apr 13, 2025

[tmpnet] Switch FlagsMap from map[string]any to map[string]string #3884

Merged

1 task

maru-ava force-pushed the tmpnet-start-network-vars branch from d736aec to af712ec Compare April 13, 2025 22:28

maru-ava force-pushed the tmpnet-implicit-data-dir branch from 465cd68 to f0440ce Compare April 13, 2025 22:29

felipemadero approved these changes Apr 14, 2025

View reviewed changes

maru-ava force-pushed the tmpnet-start-network-vars branch from af712ec to 196fd09 Compare April 14, 2025 18:01

maru-ava force-pushed the tmpnet-implicit-data-dir branch from f0440ce to 1c1ab28 Compare April 14, 2025 18:10

michaelkaplan13 approved these changes Apr 16, 2025

View reviewed changes

Base automatically changed from tmpnet-start-network-vars to master April 17, 2025 00:21

[tmpnet] Avoid serializing the node data directory

3b43707

This is intended to simplify usage by CLI.

maru-ava force-pushed the tmpnet-implicit-data-dir branch from 1c1ab28 to 3b43707 Compare April 17, 2025 00:32

maru-ava enabled auto-merge April 17, 2025 00:33

maru-ava added this pull request to the merge queue Apr 17, 2025

maru-ava mentioned this pull request Apr 17, 2025

[tmpnet] Rename NodeProcess to ProcessRuntime #3890

Merged

1 task

Merged via the queue into master with commit 273c73d Apr 17, 2025
24 checks passed

github-project-automation bot moved this from In Review 👀 to Done ✅ in avalanchego Apr 17, 2025

maru-ava deleted the tmpnet-implicit-data-dir branch April 17, 2025 01:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tmpnet] Avoid serializing the node data directory #3881

[tmpnet] Avoid serializing the node data directory #3881

maru-ava commented Apr 11, 2025 •

edited

Loading

maru-ava Apr 11, 2025

maru-ava Apr 11, 2025

maru-ava Apr 11, 2025

maru-ava Apr 11, 2025

maru-ava Apr 11, 2025

maru-ava Apr 11, 2025 •

edited

Loading

maru-ava Apr 11, 2025

Copilot AI left a comment

michaelkaplan13 left a comment

maru-ava commented Apr 16, 2025 •

edited

Loading

[tmpnet] Avoid serializing the node data directory #3881

[tmpnet] Avoid serializing the node data directory #3881

Conversation

maru-ava commented Apr 11, 2025 • edited Loading

PR Chain: tmpnet+kube

Why this should be merged

How this was tested

Need to be documented in RELEASES.md?

TODO

maru-ava Apr 11, 2025

Choose a reason for hiding this comment

maru-ava Apr 11, 2025

Choose a reason for hiding this comment

maru-ava Apr 11, 2025

Choose a reason for hiding this comment

maru-ava Apr 11, 2025

Choose a reason for hiding this comment

maru-ava Apr 11, 2025

Choose a reason for hiding this comment

maru-ava Apr 11, 2025 • edited Loading

Choose a reason for hiding this comment

maru-ava Apr 11, 2025

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

michaelkaplan13 left a comment

Choose a reason for hiding this comment

maru-ava commented Apr 16, 2025 • edited Loading

maru-ava commented Apr 11, 2025 •

edited

Loading

maru-ava Apr 11, 2025 •

edited

Loading

maru-ava commented Apr 16, 2025 •

edited

Loading