From f9453cbe970b25bd117431b2e7f1dfe2148c911f Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 28 Jul 2022 17:28:09 +0800 Subject: [PATCH] lntemp: fix context inheritance This commit changes how the context is created and inherited to better reflex the relationship of the components. We now have the following context structure, ``` For ephemeral nodes, the context chain is, main HarnessTest -> sub HarnessTest -> HarnessNode -> HarnessRPC For standby nodes, the context chain is, main HarnessTest -> standby HarnessNode -> HarnessRPC ``` --- lntemp/harness.go | 8 +++-- lntemp/harness_node_manager.go | 40 ++++++++---------------- lntemp/node/harness_node.go | 57 +++++++++++++++++++--------------- 3 files changed, 51 insertions(+), 54 deletions(-) diff --git a/lntemp/harness.go b/lntemp/harness.go index 07e0a4966..097185d79 100644 --- a/lntemp/harness.go +++ b/lntemp/harness.go @@ -408,9 +408,13 @@ func (h *HarnessTest) SetTestName(name string) { func (h *HarnessTest) NewNode(name string, extraArgs []string) *node.HarnessNode { - node, err := h.manager.newNode(h.T, name, extraArgs, false, nil, false) + node, err := h.manager.newNode(h.T, name, extraArgs, nil, false) require.NoErrorf(h, err, "unable to create new node for %s", name) + // Start the node. + err = node.Start(h.runCtx) + require.NoError(h, err, "failed to start node %s", node.Name()) + return node } @@ -428,7 +432,7 @@ func (h *HarnessTest) Shutdown(node *node.HarnessNode) { func (h *HarnessTest) RestartNode(hn *node.HarnessNode, chanBackups ...*lnrpc.ChanBackupSnapshot) { - err := h.manager.restartNode(hn, nil, chanBackups...) + err := h.manager.restartNode(h.runCtx, hn, nil, chanBackups...) require.NoErrorf(h, err, "failed to restart node %s", hn.Name()) // Give the node some time to catch up with the chain before we diff --git a/lntemp/harness_node_manager.go b/lntemp/harness_node_manager.go index 3cf8469a9..c4aba1c87 100644 --- a/lntemp/harness_node_manager.go +++ b/lntemp/harness_node_manager.go @@ -1,6 +1,7 @@ package lntemp import ( + "context" "fmt" "sync" "sync/atomic" @@ -70,7 +71,7 @@ func (nm *nodeManager) nextNodeID() uint32 { // node can be used immediately. Otherwise, the node will require an additional // initialization phase where the wallet is either created or restored. func (nm *nodeManager) newNode(t *testing.T, name string, extraArgs []string, - useSeed bool, password []byte, cmdOnly bool, + password []byte, useSeed bool, opts ...node.Option) (*node.HarnessNode, error) { cfg := &node.BaseNodeConfig{ @@ -84,6 +85,7 @@ func (nm *nodeManager) newNode(t *testing.T, name string, extraArgs []string, NodeID: nm.nextNodeID(), LndBinary: nm.lndBinary, NetParams: harnessNetParams, + HasSeed: useSeed, } for _, opt := range opts { opt(cfg) @@ -96,27 +98,7 @@ func (nm *nodeManager) newNode(t *testing.T, name string, extraArgs []string, // Put node in activeNodes to ensure Shutdown is called even if start // returns an error. - defer nm.registerNode(node) - - switch { - // If the node uses seed to start, we'll need to create the wallet and - // unlock the wallet later. - case useSeed: - err = node.StartWithSeed() - - // Start the node only with the lnd process without creating the grpc - // connection, which is used in testing etcd leader selection. - case cmdOnly: - err = node.StartLndCmd() - - // By default, we'll create a node with wallet being unlocked. - default: - err = node.Start() - } - - if err != nil { - return nil, fmt.Errorf("failed to start: %w", err) - } + nm.registerNode(node) return node, nil } @@ -153,10 +135,10 @@ func (nm *nodeManager) shutdownNode(node *node.HarnessNode) error { // crashes, etc. Additionally, each time the node is restarted, the caller can // pass a set of SCBs to pass in via the Unlock method allowing them to restore // channels during restart. -func (nm *nodeManager) restartNode(node *node.HarnessNode, +func (nm *nodeManager) restartNode(ctxt context.Context, node *node.HarnessNode, callback func() error, chanBackups ...*lnrpc.ChanBackupSnapshot) error { - err := nm.restartNodeNoUnlock(node, callback) + err := nm.restartNodeNoUnlock(ctxt, node, callback) if err != nil { return err } @@ -193,8 +175,8 @@ func (nm *nodeManager) restartNode(node *node.HarnessNode, // blocking. If the callback parameter is non-nil, then the function will be // executed after the node shuts down, but *before* the process has been // started up again. -func (nm *nodeManager) restartNodeNoUnlock(node *node.HarnessNode, - callback func() error) error { +func (nm *nodeManager) restartNodeNoUnlock(ctxt context.Context, + node *node.HarnessNode, callback func() error) error { if err := node.Stop(); err != nil { return fmt.Errorf("restart node got error: %w", err) @@ -206,5 +188,9 @@ func (nm *nodeManager) restartNodeNoUnlock(node *node.HarnessNode, } } - return node.Start() + if node.Cfg.HasSeed { + return node.StartWithSeed(ctxt) + } + + return node.Start(ctxt) } diff --git a/lntemp/node/harness_node.go b/lntemp/node/harness_node.go index 1629afb82..8f7297f7b 100644 --- a/lntemp/node/harness_node.go +++ b/lntemp/node/harness_node.go @@ -332,9 +332,9 @@ func (hn *HarnessNode) SetExtraArgs(extraArgs []string) { // StartLndCmd handles the startup of lnd, creating log files, and possibly // kills the process when needed. -func (hn *HarnessNode) StartLndCmd() error { - // Init the runCtx. - hn.runCtx, hn.cancel = context.WithCancel(context.Background()) +func (hn *HarnessNode) StartLndCmd(ctxb context.Context) error { + // Init the run context. + hn.runCtx, hn.cancel = context.WithCancel(ctxb) args := hn.Cfg.GenArgs() hn.cmd = exec.Command(hn.Cfg.LndBinary, args...) //nolint:gosec @@ -365,9 +365,9 @@ func (hn *HarnessNode) StartLndCmd() error { // start. // // NOTE: caller needs to take extra step to create and unlock the wallet. -func (hn *HarnessNode) StartWithSeed() error { +func (hn *HarnessNode) StartWithSeed(ctxt context.Context) error { // Start lnd process and prepare logs. - if err := hn.StartLndCmd(); err != nil { + if err := hn.StartLndCmd(ctxt); err != nil { return fmt.Errorf("start lnd error: %w", err) } @@ -388,9 +388,9 @@ func (hn *HarnessNode) StartWithSeed() error { // Start will start the lnd process, creates the grpc connection, and waits // until the server is fully started. -func (hn *HarnessNode) Start() error { +func (hn *HarnessNode) Start(ctxt context.Context) error { // Start lnd process and prepare logs. - if err := hn.StartLndCmd(); err != nil { + if err := hn.StartLndCmd(ctxt); err != nil { return fmt.Errorf("start lnd error: %w", err) } @@ -500,7 +500,7 @@ func (hn *HarnessNode) waitTillServerState( func (hn *HarnessNode) initLightningClient() error { // Wait until the server is fully started. if err := hn.WaitUntilServerActive(); err != nil { - return err + return fmt.Errorf("waiting for server active: %v", err) } // Set the harness node's pubkey to what the node claims in GetInfo. @@ -677,29 +677,36 @@ func (hn *HarnessNode) Stop() error { // Close any attempts at further grpc connections. if hn.conn != nil { - err := status.Code(hn.conn.Close()) - switch err { - case codes.OK: - return nil - - // When the context is canceled above, we might get the - // following error as the context is no longer active. - case codes.Canceled: - return nil - - case codes.Unknown: - return fmt.Errorf("unknown error attempting to stop "+ - "grpc client: %v", err) - - default: - return fmt.Errorf("error attempting to stop "+ - "grpc client: %v", err) + if err := hn.CloseConn(); err != nil { + return err } } return nil } +// CloseConn closes the grpc connection. +func (hn *HarnessNode) CloseConn() error { + err := status.Code(hn.conn.Close()) + switch err { + case codes.OK: + return nil + + // When the context is canceled above, we might get the + // following error as the context is no longer active. + case codes.Canceled: + return nil + + case codes.Unknown: + return fmt.Errorf("unknown error attempting to stop "+ + "grpc client: %v", err) + + default: + return fmt.Errorf("error attempting to stop "+ + "grpc client: %v", err) + } +} + // Shutdown stops the active lnd process and cleans up any temporary // directories created along the way. func (hn *HarnessNode) Shutdown() error {