Refactor cluster sync tests code (#343)

* remove useless log which obscures the output
* More refactoring for process cluster sync tests
* More refactoring for docker cluster sync tests

Author: nikita-vanyasin

docs/http_api.md

@@ -183,7 +183,7 @@ Status codes:
 
 ### GET `/id` 
 
-Internap API used to get the ID number of the starter. Not for external use.
+Internal API used to get the ID number of the starter. Not for external use.
 
 ### GET `/hello` 
 

service/arangosync_config_builder.go

@@ -41,7 +41,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/rs/zerolog"
@@ -168,15 +167,11 @@ func createArangoSyncArgs(log zerolog.Logger, config Config, clusterConfig Clust
 			optionPair{"--mq.type", config.SyncMQType},
 		)
 		if clusterJWTSecretFile != "" {
-			if !features.GetJWTFolderOption() {
-				opts = append(opts,
-					optionPair{"--cluster.jwt-secret", clusterJWTSecretFile},
-				)
-			} else {
-				opts = append(opts,
-					optionPair{"--cluster.jwt-secret", path.Join(clusterJWTSecretFile, definitions.ArangodJWTSecretActive)},
-				)
+			jwtSecretPath := clusterJWTSecretFile
+			if features.GetJWTFolderOption() {
+				jwtSecretPath = path.Join(clusterJWTSecretFile, definitions.ArangodJWTSecretActive)
 			}
+			opts = append(opts, optionPair{"--cluster.jwt-secret", jwtSecretPath})
 		}
 		if clusterEPs, err := clusterConfig.GetCoordinatorEndpoints(); err == nil {
 			if len(clusterEPs) == 0 {
@@ -197,8 +192,7 @@ func createArangoSyncArgs(log zerolog.Logger, config Config, clusterConfig Clust
 				return nil, maskAny(fmt.Errorf("No sync masters found"))
 			}
 			for _, ep := range syncMasterEPs {
-				opts = append(opts,
-					optionPair{"--master.endpoint", ep})
+				opts = append(opts, optionPair{"--master.endpoint", ep})
 			}
 		} else {
 			log.Error().Err(err).Msg("Cannot find sync master endpoints")
@@ -228,14 +222,3 @@ func createArangoSyncArgs(log zerolog.Logger, config Config, clusterConfig Clust
 
 	return args, nil
 }
-
-func isOptionSuitableForArangoSyncServer(serverType definitions.ServerType, optionName string) bool {
-	switch serverType {
-	case definitions.ServerTypeSyncMaster:
-	case definitions.ServerTypeSyncWorker:
-		if strings.HasPrefix(optionName, "mq.") || strings.HasPrefix(optionName, "cluster.") {
-			return false
-		}
-	}
-	return true
-}

test/docker_cluster_sync_test.go

@@ -23,12 +23,11 @@
 package test
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
-	"time"
+
+	"github.com/stretchr/testify/require"
 )
 
 // TestDockerClusterSync runs 3 arangodb starters in docker with arangosync enabled.
@@ -37,120 +36,36 @@ func TestDockerClusterSync(t *testing.T) {
 	needStarterMode(t, starterModeCluster)
 	needEnterprise(t)
 	ip := os.Getenv("IP")
-	if ip == "" {
-		t.Fatal("IP envvar must be set to IP address of this machine")
-	}
-
-	// Create certificates
-	certs := createSyncCertificates(t, ip, true)
-
-	volID1 := createDockerID("vol-starter-test-cluster-sync1-")
-	createDockerVolume(t, volID1)
-	defer removeDockerVolume(t, volID1)
-
-	volID2 := createDockerID("vol-starter-test-cluster-sync2-")
-	createDockerVolume(t, volID2)
-	defer removeDockerVolume(t, volID2)
-
-	volID3 := createDockerID("vol-starter-test-cluster-sync3-")
-	createDockerVolume(t, volID3)
-	defer removeDockerVolume(t, volID3)
+	require.NotEmpty(t, ip, "IP envvar must be set to IP address of this machine")
 
 	// Cleanup of left over tests
 	removeDockerContainersByLabel(t, "starter-test=true")
 	removeStarterCreatedDockerContainers(t)
 
-	start := time.Now()
+	// Create certificates
+	certs := createSyncCertificates(t, ip, true)
 
-	cID1 := createDockerID("starter-test-cluster-sync1-")
-	dockerRun1 := Spawn(t, strings.Join([]string{
-		"docker run -i",
-		"--label starter-test=true",
-		"--name=" + cID1,
-		"--rm",
-		createLicenseKeyOption(),
-		fmt.Sprintf("-p %d:%d", basePort, basePort),
-		fmt.Sprintf("-v %s:/data", volID1),
-		fmt.Sprintf("-v %s:/certs", certs.Dir),
-		"-v /var/run/docker.sock:/var/run/docker.sock",
-		"arangodb/arangodb-starter",
-		"--docker.container=" + cID1,
-		"--starter.address=$IP",
-		"--server.storage-engine=rocksdb",
-		"--starter.sync",
-		"--sync.server.keyfile=/certs/" + filepath.Base(certs.TLS.DCA.Keyfile),
-		"--sync.server.client-cafile=/certs/" + filepath.Base(certs.ClientAuth.CACertificate),
-		"--sync.master.jwt-secret=/certs/" + filepath.Base(certs.MasterSecret),
-		"--auth.jwt-secret=/certs/" + filepath.Base(certs.ClusterSecret),
-		"--sync.monitoring.token=" + syncMonitoringToken,
-		createEnvironmentStarterOptions(),
-	}, " "))
-	defer dockerRun1.Close()
-	defer removeDockerContainer(t, cID1)
+	volumeIDs, cleanVolumes := createDockerVolumes(t,
+		"vol-starter-test-cluster-sync1-",
+		"vol-starter-test-cluster-sync2-",
+		"vol-starter-test-cluster-sync3-",
+	)
+	defer cleanVolumes()
 
-	cID2 := createDockerID("starter-test-cluster-sync2-")
-	dockerRun2 := Spawn(t, strings.Join([]string{
-		"docker run -i",
-		"--label starter-test=true",
-		"--name=" + cID2,
-		"--rm",
-		createLicenseKeyOption(),
-		fmt.Sprintf("-p %d:%d", basePort+(1*portIncrement), basePort),
-		fmt.Sprintf("-v %s:/data", volID2),
-		fmt.Sprintf("-v %s:/certs", certs.Dir),
-		"-v /var/run/docker.sock:/var/run/docker.sock",
-		"arangodb/arangodb-starter",
-		"--docker.container=" + cID2,
+	starterArgs := []string{
 		"--starter.address=$IP",
 		"--server.storage-engine=rocksdb",
-		"--starter.sync",
-		"--sync.server.keyfile=/certs/" + filepath.Base(certs.TLS.DCA.Keyfile),
-		"--sync.server.client-cafile=/certs/" + filepath.Base(certs.ClientAuth.CACertificate),
-		"--sync.master.jwt-secret=/certs/" + filepath.Base(certs.MasterSecret),
 		"--auth.jwt-secret=/certs/" + filepath.Base(certs.ClusterSecret),
-		"--sync.monitoring.token=" + syncMonitoringToken,
-		createEnvironmentStarterOptions(),
-		fmt.Sprintf("--starter.join=$IP:%d", basePort),
-	}, " "))
-	defer dockerRun2.Close()
-	defer removeDockerContainer(t, cID2)
-
-	cID3 := createDockerID("starter-test-cluster-sync3-")
-	dockerRun3 := Spawn(t, strings.Join([]string{
-		"docker run -i",
-		"--label starter-test=true",
-		"--name=" + cID3,
-		"--rm",
-		createLicenseKeyOption(),
-		fmt.Sprintf("-p %d:%d", basePort+(2*portIncrement), basePort),
-		fmt.Sprintf("-v %s:/data", volID3),
-		fmt.Sprintf("-v %s:/certs", certs.Dir),
-		"-v /var/run/docker.sock:/var/run/docker.sock",
-		"arangodb/arangodb-starter",
-		"--docker.container=" + cID3,
-		"--starter.address=$IP",
-		"--server.storage-engine=rocksdb",
 		"--starter.sync",
 		"--sync.server.keyfile=/certs/" + filepath.Base(certs.TLS.DCA.Keyfile),
 		"--sync.server.client-cafile=/certs/" + filepath.Base(certs.ClientAuth.CACertificate),
 		"--sync.master.jwt-secret=/certs/" + filepath.Base(certs.MasterSecret),
-		"--auth.jwt-secret=/certs/" + filepath.Base(certs.ClusterSecret),
 		"--sync.monitoring.token=" + syncMonitoringToken,
 		createEnvironmentStarterOptions(),
-		fmt.Sprintf("--starter.join=$IP:%d", basePort),
-	}, " "))
-	defer dockerRun3.Close()
-	defer removeDockerContainer(t, cID3)
-
-	if ok := WaitUntilStarterReady(t, whatCluster, 3, dockerRun1, dockerRun2, dockerRun3); ok {
-		t.Logf("Cluster start took %s", time.Since(start))
-		testClusterWithSync(t, insecureStarterEndpoint(0*portIncrement), false)
-		testClusterWithSync(t, insecureStarterEndpoint(1*portIncrement), false)
-		testClusterWithSync(t, insecureStarterEndpoint(2*portIncrement), false)
 	}
 
-	waitForCallFunction(t,
-		ShutdownStarterCall(insecureStarterEndpoint(0*portIncrement)),
-		ShutdownStarterCall(insecureStarterEndpoint(1*portIncrement)),
-		ShutdownStarterCall(insecureStarterEndpoint(2*portIncrement)))
+	procs, cleanup := startClusterInDocker(t, certs.Dir, starterArgs, volumeIDs)
+	defer cleanup()
+
+	waitForClusterReadinessAndFinish(t, true, false, procs...)
 }

test/docker_util.go

@@ -30,8 +30,26 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/stretchr/testify/require"
 )
 
+// createDockerVolumes creates docker volume for each provided prefix and returns the list of volume ids and cleanup function
+func createDockerVolumes(t *testing.T, prefixes ...string) ([]string, func()) {
+	ids := make([]string, 0, len(prefixes))
+	for _, prefix := range prefixes {
+		id := createDockerID(prefix)
+		ids = append(ids, id)
+		createDockerVolume(t, id)
+	}
+	require.Len(t, ids, len(prefixes))
+	return ids, func() {
+		for _, id := range ids {
+			removeDockerVolume(t, id)
+		}
+	}
+}
+
 func createDockerVolume(t *testing.T, id string) {
 	c := Spawn(t, fmt.Sprintf("docker volume create %s", id))
 	defer c.Close()

test/process_cluster_sync_test.go

@@ -24,9 +24,7 @@ package test
 
 import (
 	"os"
-	"strings"
 	"testing"
-	"time"
 )
 
 // TestProcessClusterSync starts a master starter, followed by 2 slave starters,
@@ -41,68 +39,26 @@ func TestProcessClusterSync(t *testing.T) {
 	ip := "127.0.0.1"
 	certs := createSyncCertificates(t, ip, false)
 
-	dataDirMaster := SetUniqueDataDir(t)
-	defer os.RemoveAll(dataDirMaster)
+	peerDirs := []string{SetUniqueDataDir(t), SetUniqueDataDir(t), SetUniqueDataDir(t)}
+	defer func() {
+		for _, d := range peerDirs {
+			os.RemoveAll(d)
+		}
+	}()
 
-	start := time.Now()
-
-	master := Spawn(t, strings.Join([]string{
+	starterArgs := []string{
 		"${STARTER}",
 		"--starter.address=" + ip,
-		"--server.storage-engine=rocksdb",
-		"--starter.sync",
-		"--sync.server.keyfile=" + certs.TLS.DCA.Keyfile,
-		"--sync.server.client-cafile=" + certs.ClientAuth.CACertificate,
-		"--sync.master.jwt-secret=" + certs.MasterSecret,
 		"--auth.jwt-secret=" + certs.ClusterSecret,
-		"--sync.monitoring.token=" + syncMonitoringToken,
-		createEnvironmentStarterOptions(),
-	}, " "))
-	defer master.Close()
-
-	dataDirSlave1 := SetUniqueDataDir(t)
-	defer os.RemoveAll(dataDirSlave1)
-	slave1 := Spawn(t, strings.Join([]string{
-		"${STARTER}",
-		"--starter.join=" + ip,
-		"--starter.address=" + ip,
-		"--server.storage-engine=rocksdb",
 		"--starter.sync",
 		"--sync.server.keyfile=" + certs.TLS.DCA.Keyfile,
 		"--sync.server.client-cafile=" + certs.ClientAuth.CACertificate,
 		"--sync.master.jwt-secret=" + certs.MasterSecret,
-		"--auth.jwt-secret=" + certs.ClusterSecret,
 		"--sync.monitoring.token=" + syncMonitoringToken,
 		createEnvironmentStarterOptions(),
-	}, " "))
-	defer slave1.Close()
-
-	dataDirSlave2 := SetUniqueDataDir(t)
-	defer os.RemoveAll(dataDirSlave2)
-	slave2 := Spawn(t, strings.Join([]string{
-		"${STARTER}",
-		"--starter.join=" + ip,
-		"--starter.address=" + ip,
-		"--server.storage-engine=rocksdb",
-		"--starter.sync",
-		"--sync.server.keyfile=" + certs.TLS.DCA.Keyfile,
-		"--sync.server.client-cafile=" + certs.ClientAuth.CACertificate,
-		"--sync.master.jwt-secret=" + certs.MasterSecret,
-		"--auth.jwt-secret=" + certs.ClusterSecret,
-		"--sync.monitoring.token=" + syncMonitoringToken,
-		createEnvironmentStarterOptions(),
-	}, " "))
-	defer slave2.Close()
-
-	if ok := WaitUntilStarterReady(t, whatCluster, 3, master, slave1, slave2); ok {
-		t.Logf("Cluster start took %s", time.Since(start))
-		testClusterWithSync(t, insecureStarterEndpoint(0*portIncrement), false)
-		testClusterWithSync(t, insecureStarterEndpoint(1*portIncrement), false)
-		testClusterWithSync(t, insecureStarterEndpoint(2*portIncrement), false)
 	}
+	procs, cleanup := startCluster(t, ip, starterArgs, peerDirs)
+	defer cleanup()
 
-	if isVerbose {
-		t.Log("Waiting for termination")
-	}
-	SendIntrAndWait(t, master, slave1, slave2)
+	waitForClusterReadinessAndFinish(t, true, false, procs...)
 }