Merge pull request #5471 from joostjager/db-itest-prep

itest: prepare for multiple db backends

.travis.yml

@@ -59,7 +59,7 @@ jobs:
     - name: Bitcoind Integration with etcd (txindex enabled)
       script:
         - bash ./scripts/install_bitcoind.sh
-        - make itest-parallel backend=bitcoind etcd=1
+        - make itest-parallel backend=bitcoind dbbackend=etcd
 
     - name: Bitcoind Integration (txindex disabled)
       script:

lntest/harness.go

@@ -67,9 +67,8 @@ type NetworkHarness struct {
 	Alice *HarnessNode
 	Bob   *HarnessNode
 
-	// embeddedEtcd is set to true if new nodes are to be created with an
+	// dbBackend sets the database backend to use.
-	// embedded etcd backend instead of just bbolt.
+	dbBackend DatabaseBackend
-	embeddedEtcd bool
 
 	// Channel for transmitting stderr output from failed lightning node
 	// to main process.
@@ -84,12 +83,19 @@ type NetworkHarness struct {
 	mtx sync.Mutex
 }
 
+type DatabaseBackend int
+
+const (
+	BackendBbolt DatabaseBackend = iota
+	BackendEtcd
+)
+
 // NewNetworkHarness creates a new network test harness.
 // TODO(roasbeef): add option to use golang's build library to a binary of the
 // current repo. This will save developers from having to manually `go install`
 // within the repo each time before changes
 func NewNetworkHarness(r *rpctest.Harness, b BackendConfig, lndBinary string,
-	embeddedEtcd bool) (*NetworkHarness, error) {
+	dbBackend DatabaseBackend) (*NetworkHarness, error) {
 
 	feeService := startFeeService()
 
@@ -103,7 +109,7 @@ func NewNetworkHarness(r *rpctest.Harness, b BackendConfig, lndBinary string,
 		feeService:   feeService,
 		quit:         make(chan struct{}),
 		lndBinary:    lndBinary,
-		embeddedEtcd: embeddedEtcd,
+		dbBackend:    dbBackend,
 	}
 	return &n, nil
 }
@@ -306,11 +312,11 @@ func (n *NetworkHarness) NewNodeWithSeedEtcd(name string, etcdCfg *etcd.Config,
 	*HarnessNode, []string, []byte, error) {
 
 	// We don't want to use the embedded etcd instance.
-	const embeddedEtcd = false
+	const dbBackend = BackendBbolt
 
 	extraArgs := extraArgsEtcd(etcdCfg, name, cluster)
 	return n.newNodeWithSeed(
-		name, extraArgs, password, entropy, statelessInit, embeddedEtcd,
+		name, extraArgs, password, entropy, statelessInit, dbBackend,
 	)
 }
 
@@ -323,10 +329,10 @@ func (n *NetworkHarness) NewNodeEtcd(name string, etcdCfg *etcd.Config,
 	password []byte, cluster, wait bool) (*HarnessNode, error) {
 
 	// We don't want to use the embedded etcd instance.
-	const embeddedEtcd = false
+	const dbBackend = BackendBbolt
 
 	extraArgs := extraArgsEtcd(etcdCfg, name, cluster)
-	return n.newNode(name, extraArgs, true, password, embeddedEtcd, wait)
+	return n.newNode(name, extraArgs, true, password, dbBackend, wait)
 }
 
 // NewNode fully initializes a returns a new HarnessNode bound to the
@@ -336,7 +342,7 @@ func (n *NetworkHarness) NewNode(t *testing.T,
 	name string, extraArgs []string) *HarnessNode {
 
 	node, err := n.newNode(
-		name, extraArgs, false, nil, n.embeddedEtcd, true,
+		name, extraArgs, false, nil, n.dbBackend, true,
 	)
 	require.NoErrorf(t, err, "unable to create new node for %s", name)
 
@@ -352,16 +358,16 @@ func (n *NetworkHarness) NewNodeWithSeed(name string, extraArgs []string,
 	error) {
 
 	return n.newNodeWithSeed(
-		name, extraArgs, password, nil, statelessInit, n.embeddedEtcd,
+		name, extraArgs, password, nil, statelessInit, n.dbBackend,
 	)
 }
 
 func (n *NetworkHarness) newNodeWithSeed(name string, extraArgs []string,
-	password, entropy []byte, statelessInit, embeddedEtcd bool) (
+	password, entropy []byte, statelessInit bool, dbBackend DatabaseBackend) (
 	*HarnessNode, []string, []byte, error) {
 
 	node, err := n.newNode(
-		name, extraArgs, true, password, embeddedEtcd, true,
+		name, extraArgs, true, password, dbBackend, true,
 	)
 	if err != nil {
 		return nil, nil, nil, err
@@ -425,7 +431,7 @@ func (n *NetworkHarness) RestoreNodeWithSeed(name string, extraArgs []string,
 	opts ...NodeOption) (*HarnessNode, error) {
 
 	node, err := n.newNode(
-		name, extraArgs, true, password, n.embeddedEtcd, true, opts...,
+		name, extraArgs, true, password, n.dbBackend, true, opts...,
 	)
 	if err != nil {
 		return nil, err
@@ -456,7 +462,7 @@ func (n *NetworkHarness) RestoreNodeWithSeed(name string, extraArgs []string,
 // can be used immediately. Otherwise, the node will require an additional
 // initialization phase where the wallet is either created or restored.
 func (n *NetworkHarness) newNode(name string, extraArgs []string, hasSeed bool,
-	password []byte, embeddedEtcd, wait bool, opts ...NodeOption) (
+	password []byte, dbBackend DatabaseBackend, wait bool, opts ...NodeOption) (
 	*HarnessNode, error) {
 
 	cfg := &NodeConfig{
@@ -468,7 +474,7 @@ func (n *NetworkHarness) newNode(name string, extraArgs []string, hasSeed bool,
 		NetParams:         n.netParams,
 		ExtraArgs:         extraArgs,
 		FeeURL:            n.feeService.url,
-		Etcd:              embeddedEtcd,
+		DbBackend:         dbBackend,
 	}
 	for _, opt := range opts {
 		opt(cfg)
@@ -1548,9 +1554,9 @@ func FileExists(path string) bool {
 	return true
 }
 
-// CopyAll copies all files and directories from srcDir to dstDir recursively.
+// copyAll copies all files and directories from srcDir to dstDir recursively.
 // Note that this function does not support links.
-func CopyAll(dstDir, srcDir string) error {
+func copyAll(dstDir, srcDir string) error {
 	entries, err := ioutil.ReadDir(srcDir)
 	if err != nil {
 		return err
@@ -1571,7 +1577,7 @@ func CopyAll(dstDir, srcDir string) error {
 				return err
 			}
 
-			err = CopyAll(dstPath, srcPath)
+			err = copyAll(dstPath, srcPath)
 			if err != nil {
 				return err
 			}
@@ -1582,3 +1588,43 @@ func CopyAll(dstDir, srcDir string) error {
 
 	return nil
 }
+
+// BackupDb creates a backup of the current database.
+func (n *NetworkHarness) BackupDb(hn *HarnessNode) error {
+	if hn.backupDbDir != "" {
+		return errors.New("backup already created")
+	}
+
+	// Backup files.
+	tempDir, err := ioutil.TempDir("", "past-state")
+	if err != nil {
+		return fmt.Errorf("unable to create temp db folder: %v", err)
+	}
+
+	if err := copyAll(tempDir, hn.DBDir()); err != nil {
+		return fmt.Errorf("unable to copy database files: %v", err)
+	}
+
+	hn.backupDbDir = tempDir
+
+	return nil
+}
+
+// RestoreDb restores a database backup.
+func (n *NetworkHarness) RestoreDb(hn *HarnessNode) error {
+	if hn.backupDbDir == "" {
+		return errors.New("no database backup created")
+	}
+
+	// Restore files.
+	if err := copyAll(hn.DBDir(), hn.backupDbDir); err != nil {
+		return fmt.Errorf("unable to copy database files: %v", err)
+	}
+
+	if err := os.RemoveAll(hn.backupDbDir); err != nil {
+		return fmt.Errorf("unable to remove backup dir: %v", err)
+	}
+	hn.backupDbDir = ""
+
+	return nil
+}

lntest/itest/lnd_test.go

@@ -80,8 +80,8 @@ var (
 			"split test cases with the given (0-based) index",
 	)
 
-	// useEtcd test LND nodes use (embedded) etcd as remote db.
+	// dbBackendFlag specifies the backend to use
-	useEtcd = flag.Bool("etcd", false, "Use etcd backend for lnd.")
+	dbBackendFlag = flag.String("dbbackend", "bbolt", "Database backend (bbolt, etcd)")
 )
 
 // getTestCaseSplitTranche returns the sub slice of the test cases that should
@@ -6629,18 +6629,10 @@ func testRevokedCloseRetribution(net *lntest.NetworkHarness, t *harnessTest) {
 	// broadcast this soon to be revoked state.
 	bobStateNumPreCopy := bobChan.NumUpdates
 
-	// Create a temporary file to house Bob's database state at this
-	// particular point in history.
-	bobTempDbPath, err := ioutil.TempDir("", "bob-past-state")
-	if err != nil {
-		t.Fatalf("unable to create temp db folder: %v", err)
-	}
-	defer os.Remove(bobTempDbPath)
-
 	// With the temporary file created, copy Bob's current state into the
 	// temporary file we created above. Later after more updates, we'll
 	// restore this state.
-	if err := lntest.CopyAll(bobTempDbPath, net.Bob.DBDir()); err != nil {
+	if err := net.BackupDb(net.Bob); err != nil {
 		t.Fatalf("unable to copy database files: %v", err)
 	}
 
@@ -6666,7 +6658,7 @@ func testRevokedCloseRetribution(net *lntest.NetworkHarness, t *harnessTest) {
 	// state. With this, we essentially force Bob to travel back in time
 	// within the channel's history.
 	if err = net.RestartNode(net.Bob, func() error {
-		return lntest.CopyAll(net.Bob.DBDir(), bobTempDbPath)
+		return net.RestoreDb(net.Bob)
 	}); err != nil {
 		t.Fatalf("unable to restart node: %v", err)
 	}
@@ -6872,18 +6864,10 @@ func testRevokedCloseRetributionZeroValueRemoteOutput(net *lntest.NetworkHarness
 	// broadcast this soon to be revoked state.
 	carolStateNumPreCopy := carolChan.NumUpdates
 
-	// Create a temporary file to house Carol's database state at this
-	// particular point in history.
-	carolTempDbPath, err := ioutil.TempDir("", "carol-past-state")
-	if err != nil {
-		t.Fatalf("unable to create temp db folder: %v", err)
-	}
-	defer os.Remove(carolTempDbPath)
-
 	// With the temporary file created, copy Carol's current state into the
 	// temporary file we created above. Later after more updates, we'll
 	// restore this state.
-	if err := lntest.CopyAll(carolTempDbPath, carol.DBDir()); err != nil {
+	if err := net.BackupDb(carol); err != nil {
 		t.Fatalf("unable to copy database files: %v", err)
 	}
 
@@ -6908,7 +6892,7 @@ func testRevokedCloseRetributionZeroValueRemoteOutput(net *lntest.NetworkHarness
 	// state. With this, we essentially force Carol to travel back in time
 	// within the channel's history.
 	if err = net.RestartNode(carol, func() error {
-		return lntest.CopyAll(carol.DBDir(), carolTempDbPath)
+		return net.RestoreDb(carol)
 	}); err != nil {
 		t.Fatalf("unable to restart node: %v", err)
 	}
@@ -7185,18 +7169,10 @@ func testRevokedCloseRetributionRemoteHodl(net *lntest.NetworkHarness,
 	// to her channel.
 	checkCarolNumUpdatesAtLeast(1)
 
-	// Create a temporary file to house Carol's database state at this
-	// particular point in history.
-	carolTempDbPath, err := ioutil.TempDir("", "carol-past-state")
-	if err != nil {
-		t.Fatalf("unable to create temp db folder: %v", err)
-	}
-	defer os.Remove(carolTempDbPath)
-
 	// With the temporary file created, copy Carol's current state into the
 	// temporary file we created above. Later after more updates, we'll
 	// restore this state.
-	if err := lntest.CopyAll(carolTempDbPath, carol.DBDir()); err != nil {
+	if err := net.BackupDb(carol); err != nil {
 		t.Fatalf("unable to copy database files: %v", err)
 	}
 
@@ -7230,7 +7206,7 @@ func testRevokedCloseRetributionRemoteHodl(net *lntest.NetworkHarness,
 	// state. With this, we essentially force Carol to travel back in time
 	// within the channel's history.
 	if err = net.RestartNode(carol, func() error {
-		return lntest.CopyAll(carol.DBDir(), carolTempDbPath)
+		return net.RestoreDb(carol)
 	}); err != nil {
 		t.Fatalf("unable to restart node: %v", err)
 	}
@@ -7618,18 +7594,10 @@ func testRevokedCloseRetributionAltruistWatchtowerCase(
 	// broadcast this soon to be revoked state.
 	carolStateNumPreCopy := carolChan.NumUpdates
 
-	// Create a temporary file to house Carol's database state at this
-	// particular point in history.
-	carolTempDbPath, err := ioutil.TempDir("", "carol-past-state")
-	if err != nil {
-		t.Fatalf("unable to create temp db folder: %v", err)
-	}
-	defer os.Remove(carolTempDbPath)
-
 	// With the temporary file created, copy Carol's current state into the
 	// temporary file we created above. Later after more updates, we'll
 	// restore this state.
-	if err := lntest.CopyAll(carolTempDbPath, carol.DBDir()); err != nil {
+	if err := net.BackupDb(carol); err != nil {
 		t.Fatalf("unable to copy database files: %v", err)
 	}
 
@@ -7688,7 +7656,7 @@ func testRevokedCloseRetributionAltruistWatchtowerCase(
 	// state. With this, we essentially force Carol to travel back in time
 	// within the channel's history.
 	if err = net.RestartNode(carol, func() error {
-		return lntest.CopyAll(carol.DBDir(), carolTempDbPath)
+		return net.RestoreDb(carol)
 	}); err != nil {
 		t.Fatalf("unable to restart node: %v", err)
 	}
@@ -8158,18 +8126,10 @@ func testDataLossProtection(net *lntest.NetworkHarness, t *harnessTest) {
 		// revoke this state.
 		stateNumPreCopy := nodeChan.NumUpdates
 
-		// Create a temporary file to house the database state at this
-		// particular point in history.
-		tempDbPath, err := ioutil.TempDir("", node.Name()+"-past-state")
-		if err != nil {
-			t.Fatalf("unable to create temp db folder: %v", err)
-		}
-		defer os.Remove(tempDbPath)
-
 		// With the temporary file created, copy the current state into
 		// the temporary file we created above. Later after more
 		// updates, we'll restore this state.
-		if err := lntest.CopyAll(tempDbPath, node.DBDir()); err != nil {
+		if err := net.BackupDb(node); err != nil {
 			t.Fatalf("unable to copy database files: %v", err)
 		}
 
@@ -8196,7 +8156,7 @@ func testDataLossProtection(net *lntest.NetworkHarness, t *harnessTest) {
 		// force the node to travel back in time within the channel's
 		// history.
 		if err = net.RestartNode(node, func() error {
-			return lntest.CopyAll(node.DBDir(), tempDbPath)
+			return net.RestoreDb(node)
 		}); err != nil {
 			t.Fatalf("unable to restart node: %v", err)
 		}
@@ -11613,12 +11573,25 @@ func TestLightningNetworkDaemon(t *testing.T) {
 	require.NoError(t, miner.Client.NotifyNewTransactions(false))
 	require.NoError(t, chainBackend.ConnectMiner(), "connect miner")
 
+	// Parse database backend
+	var dbBackend lntest.DatabaseBackend
+	switch *dbBackendFlag {
+	case "bbolt":
+		dbBackend = lntest.BackendBbolt
+
+	case "etcd":
+		dbBackend = lntest.BackendEtcd
+
+	default:
+		require.Fail(t, "unknown db backend")
+	}
+
 	// Now we can set up our test harness (LND instance), with the chain
 	// backend we just created.
 	ht := newHarnessTest(t, nil)
 	binary := ht.getLndBinary()
 	lndHarness, err = lntest.NewNetworkHarness(
-		miner, chainBackend, binary, *useEtcd,
+		miner, chainBackend, binary, dbBackend,
 	)
 	if err != nil {
 		ht.Fatalf("unable to create lightning network harness: %v", err)

lntest/node.go

@@ -220,7 +220,7 @@ type NodeConfig struct {
 
 	FeeURL string
 
-	Etcd bool
+	DbBackend DatabaseBackend
 }
 
 func (cfg NodeConfig) P2PAddr() string {
@@ -308,7 +308,7 @@ func (cfg NodeConfig) genArgs() []string {
 		args = append(args, "--accept-amp")
 	}
 
-	if cfg.Etcd {
+	if cfg.DbBackend == BackendEtcd {
 		args = append(args, "--db.backend=etcd")
 		args = append(args, "--db.etcd.embedded")
 		args = append(
@@ -389,6 +389,9 @@ type HarnessNode struct {
 	WalletKitClient  walletrpc.WalletKitClient
 	Watchtower       watchtowerrpc.WatchtowerClient
 	WatchtowerClient wtclientrpc.WatchtowerClientClient
+
+	// backupDbDir is the path where a database backup is stored, if any.
+	backupDbDir string
 }
 
 // Assert *HarnessNode implements the lnrpc.LightningClient interface.
@@ -1098,6 +1101,13 @@ func (hn *HarnessNode) SetExtraArgs(extraArgs []string) {
 
 // cleanup cleans up all the temporary files created by the node's process.
 func (hn *HarnessNode) cleanup() error {
+	if hn.backupDbDir != "" {
+		err := os.RemoveAll(hn.backupDbDir)
+		if err != nil {
+			return fmt.Errorf("unable to remove backup dir: %v", err)
+		}
+	}
+
 	return os.RemoveAll(hn.Cfg.BaseDir)
 }
 

make/testing_flags.mk

@@ -49,9 +49,12 @@ ifneq ($(icase),)
 TEST_FLAGS += -test.run="TestLightningNetworkDaemon/.*-of-.*/.*/$(icase)"
 endif
 
-# Run itests with etcd backend.
+# Run itests with specified db backend.
-ifeq ($(etcd),1)
+ifneq ($(dbbackend),)
-ITEST_FLAGS += -etcd
+ITEST_FLAGS += -dbbackend=$(dbbackend)
+endif
+
+ifeq ($(dbbackend),etcd)
 DEV_TAGS += kvdb_etcd
 endif