[tmpnet] Ensure tmpnet methods always have a logger (#3893)

Author: maru-ava

tests/e2e/e2e_test.go

@@ -79,5 +79,5 @@ var _ = ginkgo.SynchronizedBeforeSuite(func() []byte {
 	// Run in every ginkgo process
 
 	// Initialize the local test environment from the global state
-	e2e.InitSharedTestEnvironment(ginkgo.GinkgoT(), envBytes)
+	e2e.InitSharedTestEnvironment(e2e.NewTestContext(), envBytes)
 })

tests/fixture/e2e/env.go

@@ -26,11 +26,12 @@ import (
 // access to the shared env to GetEnv which adds a test context.
 var env *TestEnvironment
 
-func InitSharedTestEnvironment(t require.TestingT, envBytes []byte) {
-	require := require.New(t)
+func InitSharedTestEnvironment(tc tests.TestContext, envBytes []byte) {
+	require := require.New(tc)
 	require.Nil(env, "env already initialized")
 	env = &TestEnvironment{}
 	require.NoError(json.Unmarshal(envBytes, env))
+	env.testContext = tc
 
 	// Ginkgo parallelization is at the process level, so a given key
 	// can safely be used by all tests in a given process without fear
@@ -126,7 +127,7 @@ func NewTestEnvironment(tc tests.TestContext, flagVars *FlagVars, desiredNetwork
 
 		if len(networkDir) > 0 {
 			var err error
-			network, err = tmpnet.ReadNetwork(networkDir)
+			network, err = tmpnet.ReadNetwork(tc.Log(), networkDir)
 			require.NoError(err)
 			tc.Log().Info("loaded a network",
 				zap.String("networkDir", networkDir),
@@ -153,7 +154,7 @@ func NewTestEnvironment(tc tests.TestContext, flagVars *FlagVars, desiredNetwork
 		}
 
 		if network != nil && networkCmd == RestartNetworkCmd {
-			require.NoError(network.Restart(tc.DefaultContext(), tc.Log()))
+			require.NoError(network.Restart(tc.DefaultContext()))
 		}
 	}
 
@@ -221,8 +222,9 @@ func (te *TestEnvironment) GetRandomNodeURI() tmpnet.NodeURI {
 
 // Retrieve the network to target for testing.
 func (te *TestEnvironment) GetNetwork() *tmpnet.Network {
-	network, err := tmpnet.ReadNetwork(te.NetworkDir)
-	require.NoError(te.testContext, err)
+	tc := te.testContext
+	network, err := tmpnet.ReadNetwork(tc.Log(), te.NetworkDir)
+	require.NoError(tc, err)
 	return network
 }
 
@@ -233,14 +235,15 @@ func (te *TestEnvironment) NewKeychain() *secp256k1fx.Keychain {
 
 // Create a new private network that is not shared with other tests.
 func (te *TestEnvironment) StartPrivateNetwork(network *tmpnet.Network) {
-	require := require.New(te.testContext)
+	tc := te.testContext
+	require := require.New(tc)
 	// Use the same configuration as the shared network
-	sharedNetwork, err := tmpnet.ReadNetwork(te.NetworkDir)
+	sharedNetwork, err := tmpnet.ReadNetwork(tc.Log(), te.NetworkDir)
 	require.NoError(err)
 	network.DefaultRuntimeConfig = sharedNetwork.DefaultRuntimeConfig
 
 	StartNetwork(
-		te.testContext,
+		tc,
 		network,
 		te.RootNetworkDir,
 		te.PrivateNetworkShutdownDelay,

tests/fixture/e2e/helpers.go

@@ -136,7 +136,7 @@ func NewEthClient(tc tests.TestContext, nodeURI tmpnet.NodeURI) ethclient.Client
 func AddEphemeralNode(tc tests.TestContext, network *tmpnet.Network, node *tmpnet.Node) *tmpnet.Node {
 	require := require.New(tc)
 
-	require.NoError(network.StartNode(tc.DefaultContext(), tc.Log(), node))
+	require.NoError(network.StartNode(tc.DefaultContext(), node))
 
 	tc.DeferCleanup(func() {
 		tc.Log().Info("shutting down ephemeral node",
@@ -236,7 +236,7 @@ func CheckBootstrapIsPossible(tc tests.TestContext, network *tmpnet.Network) *tm
 	}
 
 	node := tmpnet.NewEphemeralNode(flags)
-	require.NoError(network.StartNode(tc.DefaultContext(), tc.Log(), node))
+	require.NoError(network.StartNode(tc.DefaultContext(), node))
 	// StartNode will initiate node stop if an error is encountered during start,
 	// so no further cleanup effort is required if an error is seen here.
 

tests/fixture/tmpnet/network.go

@@ -128,6 +128,8 @@ type Network struct {
 
 	// Subnets that have been enabled on the network
 	Subnets []*Subnet
+
+	log logging.Logger
 }
 
 func NewDefaultNetwork(owner string) *Network {
@@ -172,8 +174,8 @@ func BootstrapNewNetwork(
 }
 
 // Stops the nodes of the network configured in the provided directory.
-func StopNetwork(ctx context.Context, dir string) error {
-	network, err := ReadNetwork(dir)
+func StopNetwork(ctx context.Context, log logging.Logger, dir string) error {
+	network, err := ReadNetwork(log, dir)
 	if err != nil {
 		return err
 	}
@@ -182,21 +184,22 @@ func StopNetwork(ctx context.Context, dir string) error {
 
 // Restarts the nodes of the network configured in the provided directory.
 func RestartNetwork(ctx context.Context, log logging.Logger, dir string) error {
-	network, err := ReadNetwork(dir)
+	network, err := ReadNetwork(log, dir)
 	if err != nil {
 		return err
 	}
-	return network.Restart(ctx, log)
+	return network.Restart(ctx)
 }
 
 // Reads a network from the provided directory.
-func ReadNetwork(dir string) (*Network, error) {
+func ReadNetwork(log logging.Logger, dir string) (*Network, error) {
 	canonicalDir, err := toCanonicalDir(dir)
 	if err != nil {
 		return nil, err
 	}
 	network := &Network{
 		Dir: canonicalDir,
+		log: log,
 	}
 	if err := network.Read(); err != nil {
 		return nil, fmt.Errorf("failed to read network: %w", err)
@@ -213,6 +216,8 @@ func (n *Network) EnsureDefaultConfig(log logging.Logger) error {
 		zap.Any("runtimeConfig", n.DefaultRuntimeConfig),
 	)
 
+	n.log = log
+
 	// A UUID supports centralized metrics collection
 	if len(n.UUID) == 0 {
 		n.UUID = uuid.NewString()
@@ -342,9 +347,8 @@ func (n *Network) StartNodes(ctx context.Context, log logging.Logger, nodesToSta
 	// Record the time before nodes are started to ensure visibility of subsequently collected metrics via the emitted link
 	startTime := time.Now()
 
-	// Configure the networking for each node and start
 	for _, node := range nodesToStart {
-		if err := n.StartNode(ctx, log, node); err != nil {
+		if err := n.StartNode(ctx, node); err != nil {
 			return err
 		}
 	}
@@ -438,7 +442,7 @@ func (n *Network) Bootstrap(ctx context.Context, log logging.Logger) error {
 
 	if len(n.Nodes) == 1 {
 		// Ensure the node is restarted to pick up subnet and chain configuration
-		return n.RestartNode(ctx, log, bootstrapNode)
+		return n.RestartNode(ctx, bootstrapNode)
 	}
 
 	// TODO(marun) This last restart of the bootstrap node might be unnecessary if:
@@ -450,7 +454,7 @@ func (n *Network) Bootstrap(ctx context.Context, log logging.Logger) error {
 	if err := bootstrapNode.Stop(ctx); err != nil {
 		return fmt.Errorf("failed to stop node %s: %w", bootstrapNode.NodeID, err)
 	}
-	if err := n.StartNode(ctx, log, bootstrapNode); err != nil {
+	if err := n.StartNode(ctx, bootstrapNode); err != nil {
 		return fmt.Errorf("failed to start node %s: %w", bootstrapNode.NodeID, err)
 	}
 
@@ -459,19 +463,19 @@ func (n *Network) Bootstrap(ctx context.Context, log logging.Logger) error {
 }
 
 // Starts the provided node after configuring it for the network.
-func (n *Network) StartNode(ctx context.Context, log logging.Logger, node *Node) error {
+func (n *Network) StartNode(ctx context.Context, node *Node) error {
 	if err := n.EnsureNodeConfig(node); err != nil {
 		return err
 	}
 	if err := node.Write(); err != nil {
 		return err
 	}
 
-	if err := n.writeNodeFlags(log, node); err != nil {
+	if err := n.writeNodeFlags(node); err != nil {
 		return fmt.Errorf("writing node flags: %w", err)
 	}
 
-	if err := node.Start(log); err != nil {
+	if err := node.Start(); err != nil {
 		// Attempt to stop an unhealthy node to provide some assurance to the caller
 		// that an error condition will not result in a lingering process.
 		err = errors.Join(err, node.Stop(ctx))
@@ -482,7 +486,7 @@ func (n *Network) StartNode(ctx context.Context, log logging.Logger, node *Node)
 }
 
 // Restart a single node.
-func (n *Network) RestartNode(ctx context.Context, log logging.Logger, node *Node) error {
+func (n *Network) RestartNode(ctx context.Context, node *Node) error {
 	runtimeConfig := node.getRuntimeConfig()
 	if runtimeConfig.Process != nil && runtimeConfig.Process.ReuseDynamicPorts {
 		// Attempt to save the API port currently being used so the
@@ -497,10 +501,10 @@ func (n *Network) RestartNode(ctx context.Context, log logging.Logger, node *Nod
 	if err := node.Stop(ctx); err != nil {
 		return fmt.Errorf("failed to stop node %s: %w", node.NodeID, err)
 	}
-	if err := n.StartNode(ctx, log, node); err != nil {
+	if err := n.StartNode(ctx, node); err != nil {
 		return fmt.Errorf("failed to start node %s: %w", node.NodeID, err)
 	}
-	log.Info("waiting for node to report healthy",
+	n.log.Info("waiting for node to report healthy",
 		zap.Stringer("nodeID", node.NodeID),
 	)
 	return WaitForHealthy(ctx, node)
@@ -535,11 +539,11 @@ func (n *Network) Stop(ctx context.Context) error {
 	return nil
 }
 
-// Restarts all non-ephemeral nodes in the network.
-func (n *Network) Restart(ctx context.Context, log logging.Logger) error {
-	log.Info("restarting network")
+// Restarts all nodes in the network.
+func (n *Network) Restart(ctx context.Context) error {
+	n.log.Info("restarting network")
 	for _, node := range n.Nodes {
-		if err := n.RestartNode(ctx, log, node); err != nil {
+		if err := n.RestartNode(ctx, node); err != nil {
 			return err
 		}
 	}
@@ -669,7 +673,7 @@ func (n *Network) CreateSubnets(ctx context.Context, log logging.Logger, apiURI
 				// Only running nodes should be restarted
 				continue
 			}
-			if err := n.RestartNode(ctx, log, node); err != nil {
+			if err := n.RestartNode(ctx, node); err != nil {
 				return err
 			}
 		}
@@ -737,7 +741,7 @@ func (n *Network) CreateSubnets(ctx context.Context, log logging.Logger, apiURI
 		if !validatorsToRestart.Contains(node.NodeID) {
 			continue
 		}
-		if err := n.RestartNode(ctx, log, node); err != nil {
+		if err := n.RestartNode(ctx, node); err != nil {
 			return err
 		}
 	}
@@ -877,7 +881,7 @@ func (n *Network) GetChainConfigContent() (string, error) {
 
 // writeNodeFlags determines the set of flags that should be used to
 // start the given node and writes them to a file in the node path.
-func (n *Network) writeNodeFlags(log logging.Logger, node *Node) error {
+func (n *Network) writeNodeFlags(node *Node) error {
 	flags := maps.Clone(node.Flags)
 
 	// Convert the network id to a string to ensure consistency in JSON round-tripping.
@@ -902,7 +906,7 @@ func (n *Network) writeNodeFlags(log logging.Logger, node *Node) error {
 
 		isSingleNodeNetwork := (len(n.Nodes) == 1 && len(n.Genesis.InitialStakers) == 1)
 		if isSingleNodeNetwork {
-			log.Info("defaulting to sybil protection disabled to enable a single-node network to start")
+			n.log.Info("defaulting to sybil protection disabled to enable a single-node network to start")
 			flags.SetDefault(config.SybilProtectionEnabledKey, "false")
 		}
 	}

tests/fixture/tmpnet/network_test.go

@@ -26,7 +26,7 @@ func TestNetworkSerialization(t *testing.T) {
 	// Ensure node runtime is initialized
 	require.NoError(network.readNodes())
 
-	loadedNetwork, err := ReadNetwork(network.Dir)
+	loadedNetwork, err := ReadNetwork(logging.NoLog{}, network.Dir)
 	require.NoError(err)
 	for _, key := range loadedNetwork.PreFundedKeys {
 		// Address() enables comparison with the original network by

tests/fixture/tmpnet/node.go

@@ -18,7 +18,6 @@ import (
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/staking"
 	"github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner"
-	"github.com/ava-labs/avalanchego/utils/logging"
 	"github.com/ava-labs/avalanchego/vms/platformvm/signer"
 )
 
@@ -40,8 +39,8 @@ type NodeRuntime interface {
 	readState() error
 	GetLocalURI(ctx context.Context) (string, func(), error)
 	GetLocalStakingAddress(ctx context.Context) (netip.AddrPort, func(), error)
-	Start(log logging.Logger) error
 	InitiateStop() error
+	Start() error
 	WaitForStopped(ctx context.Context) error
 	IsHealthy(ctx context.Context) (bool, error)
 }
@@ -141,8 +140,8 @@ func (n *Node) IsHealthy(ctx context.Context) (bool, error) {
 	return n.getRuntime().IsHealthy(ctx)
 }
 
-func (n *Node) Start(log logging.Logger) error {
-	return n.getRuntime().Start(log)
+func (n *Node) Start() error {
+	return n.getRuntime().Start()
 }
 
 func (n *Node) InitiateStop(ctx context.Context) error {

tests/fixture/tmpnet/process_runtime.go

@@ -81,7 +81,9 @@ func (p *ProcessRuntime) readState() error {
 // its staking port. The network will start faster with this
 // synchronization due to the avoidance of exponential backoff
 // if a node tries to connect to a beacon that is not ready.
-func (p *ProcessRuntime) Start(log logging.Logger) error {
+func (p *ProcessRuntime) Start() error {
+	log := p.node.network.log
+
 	// Avoid attempting to start an already running node.
 	proc, err := p.getProcess()
 	if err != nil {

tests/fixture/tmpnet/tmpnetctl/main.go

@@ -121,7 +121,11 @@ func main() {
 			}
 			ctx, cancel := context.WithTimeout(context.Background(), tmpnet.DefaultNetworkTimeout)
 			defer cancel()
-			if err := tmpnet.StopNetwork(ctx, networkDir); err != nil {
+			log, err := tests.LoggerForFormat("", rawLogFormat)
+			if err != nil {
+				return err
+			}
+			if err := tmpnet.StopNetwork(ctx, log, networkDir); err != nil {
 				return err
 			}
 			fmt.Fprintf(os.Stdout, "Stopped network configured at: %s\n", networkDir)

tests/upgrade/upgrade_test.go

@@ -99,7 +99,7 @@ var _ = ginkgo.Describe("[Upgrade]", func() {
 				},
 			}
 
-			require.NoError(network.StartNode(tc.DefaultContext(), tc.Log(), node))
+			require.NoError(network.StartNode(tc.DefaultContext(), node))
 
 			tc.By(fmt.Sprintf("waiting for node %q to report healthy after restart", node.NodeID))
 			e2e.WaitForHealthy(tc, node)