From 0ed81c0bcfff543e1935000307d5ebaa1a9193ab Mon Sep 17 00:00:00 2001 From: richardjcai Date: Tue, 6 Sep 2022 13:11:18 -0400 Subject: [PATCH 1/2] require specifying ports when using multiple nodes --- testserver/testserver.go | 38 +++++++++++++++++++++++++++++++---- testserver/testserver_test.go | 33 ++++++++++++++++++++++++++---- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/testserver/testserver.go b/testserver/testserver.go index 94308f5..7dcad38 100644 --- a/testserver/testserver.go +++ b/testserver/testserver.go @@ -216,6 +216,7 @@ type testServerArgs struct { storeOnDisk bool // to save database in disk storeMemSize float64 // the proportion of available memory allocated to test server httpPort int + listenAddrPorts []int testConfig TestConfig nonStableDB bool cockroachBinary string // path to cockroach executable file @@ -293,6 +294,16 @@ func ExposeConsoleOpt(port int) TestServerOpt { } } +// AddPortOpt is a TestServer option that can be passed to NewTestServer to +// specify the ports for the Cockroach nodes. +// In the case of restarting nodes, it is up to the user of TestServer to make +// sure the port used here cannot be re-used. +func AddPortOpt(port int) TestServerOpt { + return func(args *testServerArgs) { + args.listenAddrPorts = append(args.listenAddrPorts, port) + } +} + // StopDownloadInMiddleOpt is a TestServer option used only in testing. // It is used to test the flock over downloaded CRDB binary. // It should not be used in production. @@ -337,6 +348,19 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) { serverArgs.cockroachBinary = customBinaryEnv } + // For backwards compatibility, in the 3 node case where no args are + // specified, default to ports 26257, 26258, 26259. + if serverArgs.numNodes == 3 && len(serverArgs.listenAddrPorts) == 0 { + serverArgs.listenAddrPorts = []int{26257, 26258, 26259} + } else if serverArgs.numNodes != 1 && len(serverArgs.listenAddrPorts) != serverArgs.numNodes { + panic(fmt.Sprintf("need to specify a port for each node using AddPortOpt, got %d nodes, need %d ports", + serverArgs.numNodes, len(serverArgs.listenAddrPorts))) + } + + if len(serverArgs.listenAddrPorts) == 0 || len(serverArgs.listenAddrPorts) == 1 { + serverArgs.listenAddrPorts = []int{0} + } + var err error if serverArgs.cockroachBinary != "" { log.Printf("Using custom cockroach binary: %s", serverArgs.cockroachBinary) @@ -445,19 +469,25 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) { nodes := make([]nodeInfo, serverArgs.numNodes) var initArgs []string + portArgsStr := make([]string, 3) + hostPort := serverArgs.listenAddrPorts[0] + for i, port := range serverArgs.listenAddrPorts { + portArgsStr[i] = fmt.Sprint(port) + } for i := 0; i < serverArgs.numNodes; i++ { nodes[i].state = stateNew nodes[i].listeningURLFile = filepath.Join(baseDir, fmt.Sprintf("listen-url%d", i)) if serverArgs.numNodes > 1 { + joinArg := fmt.Sprintf("--join=localhost:%s", strings.Join(portArgsStr, ",localhost:")) nodes[i].startCmdArgs = []string{ serverArgs.cockroachBinary, startCmd, secureOpt, storeArg + strconv.Itoa(i), - fmt.Sprintf("--listen-addr=localhost:%d", 26257+i), - fmt.Sprintf("--http-addr=localhost:%d", 8080+i), + fmt.Sprintf("--listen-addr=localhost:%d", serverArgs.listenAddrPorts[i]), + fmt.Sprintf("--http-addr=localhost:%d", 0), "--listening-url-file=" + nodes[i].listeningURLFile, - fmt.Sprintf("--join=localhost:%d,localhost:%d,localhost:%d", 26257, 26258, 26259), + joinArg, } } else { nodes[0].startCmdArgs = []string{ @@ -480,7 +510,7 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) { serverArgs.cockroachBinary, "init", secureOpt, - "--host=localhost:26259", + fmt.Sprintf("--host=localhost:%d", hostPort), } states := make([]int, serverArgs.numNodes) diff --git a/testserver/testserver_test.go b/testserver/testserver_test.go index 963084e..d66f0de 100644 --- a/testserver/testserver_test.go +++ b/testserver/testserver_test.go @@ -186,13 +186,27 @@ func TestRunServer(t *testing.T) { { name: "Insecure 3 Node", instantiation: func(t *testing.T) (*sql.DB, func()) { - return testserver.NewDBForTest(t, testserver.ThreeNodeOpt()) + return testserver.NewDBForTest(t, testserver.ThreeNodeOpt(), + testserver.AddPortOpt(26257), + testserver.AddPortOpt(26258), + testserver.AddPortOpt(26259)) }, }, { name: "Insecure 3 Node On Disk", instantiation: func(t *testing.T) (*sql.DB, func()) { - return testserver.NewDBForTest(t, testserver.ThreeNodeOpt(), testserver.StoreOnDiskOpt()) + return testserver.NewDBForTest(t, testserver.ThreeNodeOpt(), + testserver.StoreOnDiskOpt(), + testserver.AddPortOpt(26257), + testserver.AddPortOpt(26258), + testserver.AddPortOpt(26259)) + }, + }, + { + name: "Insecure 3 Node On Disk No Ports Specified", + instantiation: func(t *testing.T) (*sql.DB, func()) { + return testserver.NewDBForTest(t, testserver.ThreeNodeOpt(), + testserver.StoreOnDiskOpt()) }, }, } { @@ -348,7 +362,12 @@ func TestFlockOnDownloadedCRDB(t *testing.T) { } func TestRestartNode(t *testing.T) { - ts, err := testserver.NewTestServer(testserver.ThreeNodeOpt(), testserver.StoreOnDiskOpt()) + ts, err := testserver.NewTestServer( + testserver.ThreeNodeOpt(), + testserver.StoreOnDiskOpt(), + testserver.AddPortOpt(26257), + testserver.AddPortOpt(26258), + testserver.AddPortOpt(26259)) require.NoError(t, err) defer ts.Stop() for i := 0; i < 3; i++ { @@ -440,7 +459,9 @@ func TestUpgradeNode(t *testing.T) { } defer func() { - require.NoError(t, exec.Command("rm", "-rf", "./temp_binaries").Start()) + if err := exec.Command("rm", "-rf", "./temp_binaries").Run(); err != nil { + t.Log(err) + } }() getBinary(oldVersionBinary) @@ -448,6 +469,7 @@ func TestUpgradeNode(t *testing.T) { absPathOldBinary, err := filepath.Abs(fmt.Sprintf("./temp_binaries/%s/cockroach", oldVersionBinary)) require.NoError(t, err) + absPathNewBinary, err := filepath.Abs(fmt.Sprintf("./temp_binaries/%s/cockroach", newVersionBinary)) require.NoError(t, err) @@ -456,6 +478,9 @@ func TestUpgradeNode(t *testing.T) { testserver.CockroachBinaryPathOpt(absPathOldBinary), testserver.UpgradeCockroachBinaryPathOpt(absPathNewBinary), testserver.StoreOnDiskOpt(), + testserver.AddPortOpt(26257), + testserver.AddPortOpt(26258), + testserver.AddPortOpt(26259), ) require.NoError(t, err) defer ts.Stop() From 1da40a5e0f5b0ce6adcb4b33e2e576ebdfef80bf Mon Sep 17 00:00:00 2001 From: richardjcai Date: Tue, 6 Sep 2022 18:46:14 -0400 Subject: [PATCH 2/2] allow specifying http ports for multi-node setup --- testserver/testserver.go | 35 ++++++++++++++++++++++--------- testserver/testserver_test.go | 39 +++++++++++++++++++++-------------- 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/testserver/testserver.go b/testserver/testserver.go index 7dcad38..fda4c1e 100644 --- a/testserver/testserver.go +++ b/testserver/testserver.go @@ -215,7 +215,7 @@ type testServerArgs struct { rootPW string // if nonempty, set as pw for root storeOnDisk bool // to save database in disk storeMemSize float64 // the proportion of available memory allocated to test server - httpPort int + httpPorts []int listenAddrPorts []int testConfig TestConfig nonStableDB bool @@ -288,17 +288,27 @@ func NonStableDbOpt() TestServerOpt { // ExposeConsoleOpt is a TestServer option that can be passed to NewTestServer to // expose the console of the server on the given port. +// Warning: This is kept around for backwards compatibility, use AddHttpPortOpt +// instead. func ExposeConsoleOpt(port int) TestServerOpt { return func(args *testServerArgs) { - args.httpPort = port + args.httpPorts = []int{port} } } -// AddPortOpt is a TestServer option that can be passed to NewTestServer to +// AddHttpPortOpt is a TestServer option that can be passed to NewTestServer to +// specify the http ports for the Cockroach nodes. +func AddHttpPortOpt(port int) TestServerOpt { + return func(args *testServerArgs) { + args.httpPorts = append(args.httpPorts, port) + } +} + +// AddListenAddrPortOpt is a TestServer option that can be passed to NewTestServer to // specify the ports for the Cockroach nodes. // In the case of restarting nodes, it is up to the user of TestServer to make // sure the port used here cannot be re-used. -func AddPortOpt(port int) TestServerOpt { +func AddListenAddrPortOpt(port int) TestServerOpt { return func(args *testServerArgs) { args.listenAddrPorts = append(args.listenAddrPorts, port) } @@ -353,7 +363,7 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) { if serverArgs.numNodes == 3 && len(serverArgs.listenAddrPorts) == 0 { serverArgs.listenAddrPorts = []int{26257, 26258, 26259} } else if serverArgs.numNodes != 1 && len(serverArgs.listenAddrPorts) != serverArgs.numNodes { - panic(fmt.Sprintf("need to specify a port for each node using AddPortOpt, got %d nodes, need %d ports", + panic(fmt.Sprintf("need to specify a port for each node using AddListenAddrPortOpt, got %d nodes, need %d ports", serverArgs.numNodes, len(serverArgs.listenAddrPorts))) } @@ -469,23 +479,28 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) { nodes := make([]nodeInfo, serverArgs.numNodes) var initArgs []string - portArgsStr := make([]string, 3) + joinAddrs := make([]string, 3) hostPort := serverArgs.listenAddrPorts[0] for i, port := range serverArgs.listenAddrPorts { - portArgsStr[i] = fmt.Sprint(port) + joinAddrs[i] = fmt.Sprintf("localhost:%d", port) } + + if len(serverArgs.httpPorts) == 0 { + serverArgs.httpPorts = make([]int, serverArgs.numNodes) + } + for i := 0; i < serverArgs.numNodes; i++ { nodes[i].state = stateNew nodes[i].listeningURLFile = filepath.Join(baseDir, fmt.Sprintf("listen-url%d", i)) if serverArgs.numNodes > 1 { - joinArg := fmt.Sprintf("--join=localhost:%s", strings.Join(portArgsStr, ",localhost:")) + joinArg := fmt.Sprintf("--join=%s", strings.Join(joinAddrs, ",")) nodes[i].startCmdArgs = []string{ serverArgs.cockroachBinary, startCmd, secureOpt, storeArg + strconv.Itoa(i), fmt.Sprintf("--listen-addr=localhost:%d", serverArgs.listenAddrPorts[i]), - fmt.Sprintf("--http-addr=localhost:%d", 0), + fmt.Sprintf("--http-addr=localhost:%d", serverArgs.httpPorts[i]), "--listening-url-file=" + nodes[i].listeningURLFile, joinArg, } @@ -497,7 +512,7 @@ func NewTestServer(opts ...TestServerOpt) (TestServer, error) { secureOpt, "--host=localhost", "--port=0", - "--http-port=" + strconv.Itoa(serverArgs.httpPort), + "--http-port=" + strconv.Itoa(serverArgs.httpPorts[0]), storeArg, "--listening-url-file=" + nodes[i].listeningURLFile, } diff --git a/testserver/testserver_test.go b/testserver/testserver_test.go index d66f0de..583706c 100644 --- a/testserver/testserver_test.go +++ b/testserver/testserver_test.go @@ -187,9 +187,9 @@ func TestRunServer(t *testing.T) { name: "Insecure 3 Node", instantiation: func(t *testing.T) (*sql.DB, func()) { return testserver.NewDBForTest(t, testserver.ThreeNodeOpt(), - testserver.AddPortOpt(26257), - testserver.AddPortOpt(26258), - testserver.AddPortOpt(26259)) + testserver.AddListenAddrPortOpt(26257), + testserver.AddListenAddrPortOpt(26258), + testserver.AddListenAddrPortOpt(26259)) }, }, { @@ -197,9 +197,9 @@ func TestRunServer(t *testing.T) { instantiation: func(t *testing.T) (*sql.DB, func()) { return testserver.NewDBForTest(t, testserver.ThreeNodeOpt(), testserver.StoreOnDiskOpt(), - testserver.AddPortOpt(26257), - testserver.AddPortOpt(26258), - testserver.AddPortOpt(26259)) + testserver.AddListenAddrPortOpt(26257), + testserver.AddListenAddrPortOpt(26258), + testserver.AddListenAddrPortOpt(26259)) }, }, { @@ -209,6 +209,17 @@ func TestRunServer(t *testing.T) { testserver.StoreOnDiskOpt()) }, }, + { + name: "Insecure 3 Node With Http Ports", + instantiation: func(t *testing.T) (*sql.DB, func()) { + return testserver.NewDBForTest(t, testserver.ThreeNodeOpt(), + testserver.StoreOnDiskOpt(), + testserver.AddHttpPortOpt(8080), + testserver.AddHttpPortOpt(8081), + testserver.AddHttpPortOpt(8082), + ) + }, + }, } { t.Run(tc.name, func(t *testing.T) { db, stop := tc.instantiation(t) @@ -365,9 +376,9 @@ func TestRestartNode(t *testing.T) { ts, err := testserver.NewTestServer( testserver.ThreeNodeOpt(), testserver.StoreOnDiskOpt(), - testserver.AddPortOpt(26257), - testserver.AddPortOpt(26258), - testserver.AddPortOpt(26259)) + testserver.AddListenAddrPortOpt(26257), + testserver.AddListenAddrPortOpt(26258), + testserver.AddListenAddrPortOpt(26259)) require.NoError(t, err) defer ts.Stop() for i := 0; i < 3; i++ { @@ -459,9 +470,7 @@ func TestUpgradeNode(t *testing.T) { } defer func() { - if err := exec.Command("rm", "-rf", "./temp_binaries").Run(); err != nil { - t.Log(err) - } + require.NoError(t, exec.Command("rm", "-rf", "./temp_binaries").Run()) }() getBinary(oldVersionBinary) @@ -478,9 +487,9 @@ func TestUpgradeNode(t *testing.T) { testserver.CockroachBinaryPathOpt(absPathOldBinary), testserver.UpgradeCockroachBinaryPathOpt(absPathNewBinary), testserver.StoreOnDiskOpt(), - testserver.AddPortOpt(26257), - testserver.AddPortOpt(26258), - testserver.AddPortOpt(26259), + testserver.AddListenAddrPortOpt(26257), + testserver.AddListenAddrPortOpt(26258), + testserver.AddListenAddrPortOpt(26259), ) require.NoError(t, err) defer ts.Stop()