get container ID via "docker-compose" call in clustertests

this should be a lot more reliable than trying to construct it based
on the project name as the exact construction can differ between
docker-compose versions.

There was also an issue with the backups succeeding when they should
fail in the test. There's an arcane maze of HTTP timeouts to navigate
here, but basically there are situations where the client will just
wait forever rather than erroring if the server is paused at the
right(wrong) time. I'm not convinced we've solved every possible case
of this, so we still may see the backup succeed even when it's
supposed to fail. The ultimate hammer is to add Client.Timeout, but
that's a very blunt instrument and I'm afraid it could cause a timeout
when really we just have a lot of data to download or something.

There may be a better way to say "only time out if you literally
haven't heard a peep from the server in this long", but I haven't been
able to figure it out yet.

I also fixed how the authclustertests are run as they weren't using
the PROJECT parameter correctly. Now they can run concurrently with
clustertests, and with other copies of authclustertests without having
conflicts.

.gitlab/.gitlab-ci.yml

@@ -278,9 +278,18 @@ clustertests:
     - shell
   rules:
     - if: '$CI_PIPELINE_SOURCE == "push" || $CI_PIPELINE_SOURCE == "schedule" || $CI_PIPELINE_SOURCE == "web"'
-  allow_failure: true
   script:
     - make clustertests
+
+authclustertests:
+  variables:
+    PROJECT: authclustertests_${CI_CONCURRENT_ID}
+  stage: integration
+  tags:
+    - shell
+  rules:
+    - if: '$CI_PIPELINE_SOURCE == "push" || $CI_PIPELINE_SOURCE == "schedule" || $CI_PIPELINE_SOURCE == "web"'
+  script:
     - make authclustertests
 
 external lookup tests:
@@ -444,4 +453,4 @@ s3 dump:
     - job: build for darwin amd64
     - job: build for darwin arm64
     - job: build for linux amd64
-    - job: build for linux arm64
+    - job: build for linux arm64

Makefile

@@ -159,13 +159,12 @@ clustertests: vendor
 	$(DOCKER_COMPOSE) -f internal/clustertests/docker-compose.yml down
 
 # Run the cluster tests with authentication enabled 
-DOCKER_COMPOSE_AUTH = docker-compose -p authclustertests
 authclustertests: vendor
-	$(DOCKER_COMPOSE_AUTH) -f internal/authclustertests/docker-compose.yml down
-	$(DOCKER_COMPOSE_AUTH) -f internal/authclustertests/docker-compose.yml build
-	$(DOCKER_COMPOSE_AUTH) -f internal/authclustertests/docker-compose.yml up -d pilosa1 pilosa2 pilosa3
-	$(DOCKER_COMPOSE_AUTH) -f internal/authclustertests/docker-compose.yml run client1
-	$(DOCKER_COMPOSE_AUTH) -f internal/authclustertests/docker-compose.yml down
+	$(DOCKER_COMPOSE) -f internal/authclustertests/docker-compose.yml down
+	$(DOCKER_COMPOSE) -f internal/authclustertests/docker-compose.yml build
+	$(DOCKER_COMPOSE) -f internal/authclustertests/docker-compose.yml up -d pilosa1 pilosa2 pilosa3
+	PROJECT=$(PROJECT) $(DOCKER_COMPOSE) -f internal/authclustertests/docker-compose.yml run client1
+	$(DOCKER_COMPOSE) -f internal/authclustertests/docker-compose.yml down
 
 # Install Pilosa
 install:

http/client.go

@@ -94,7 +94,6 @@ func WithClientRetryPeriod(period time.Duration) InternalClientOption {
 		rc.RetryWaitMin = min
 		rc.RetryMax = int(attempts)
 		rc.CheckRetry = retryWith400Policy
-		rc.Logger = logger.NopLogger
 		c.retryableClient = rc
 	}
 }

http/handler.go

@@ -2866,8 +2866,8 @@ type ClientOption func(client *http.Client, dialer *net.Dialer) *http.Client
 
 func GetHTTPClient(t *tls.Config, opts ...ClientOption) *http.Client {
 	dialer := &net.Dialer{
-		Timeout:   30 * time.Second,
-		KeepAlive: 30 * time.Second,
+		Timeout:   5 * time.Second,
+		KeepAlive: 15 * time.Second,
 		DualStack: true,
 	}
 	transport := &http.Transport{
@@ -2875,9 +2875,10 @@ func GetHTTPClient(t *tls.Config, opts ...ClientOption) *http.Client {
 		DialContext:           dialer.DialContext,
 		MaxIdleConns:          1000,
 		MaxIdleConnsPerHost:   200,
-		IdleConnTimeout:       90 * time.Second,
+		IdleConnTimeout:       20 * time.Second,
 		TLSHandshakeTimeout:   10 * time.Second,
 		ExpectContinueTimeout: 1 * time.Second,
+		ResponseHeaderTimeout: 4 * time.Second,
 	}
 	if t != nil {
 		transport.TLSClientConfig = t

internal/clustertests/cluster_test.go

@@ -2,12 +2,14 @@
 package clustertest
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"io"
 	"net/http"
 	"os"
 	"os/exec"
+	"strings"
 	"testing"
 	"time"
 
@@ -17,24 +19,22 @@ import (
 	"github.com/molecula/featurebase/v3/disco"
 	picli "github.com/molecula/featurebase/v3/http"
 	"github.com/molecula/featurebase/v3/logger"
+	"github.com/pkg/errors"
 )
 
-// container turns a docker-compose service name into a container name
-// assuming the project name is set in the enviroment as PROJECT. This
-// refers to the "-p" argument to docker-compose. NOTE: this assumes
-// docker-compose joins the project name with a separating
-// underscore... this may not always be true as I've seen a dash used
-// as well, but I think it is true in recent versions.
-func container(svc string) string {
+// container turns a docker-compose service name into a container ID
+// by calling "docker-compose ps"
+func container(t *testing.T, svc string) string {
 	project := "clustertests"
-	if os.Getenv("ENABLE_AUTH") == "1" {
-		project = "authclustertests"
-	}
-
 	if p := os.Getenv("PROJECT"); p != "" {
 		project = p
 	}
-	return project + "_" + svc + "_1"
+	stdout, stderr, err := runCmd("docker-compose", "-p", project, "ps", "-q", svc)
+	if err != nil {
+		t.Fatalf("couldn't construct container name, err: %v, stderr:\n%s\nstdout:\n%s", err, stderr, stdout)
+	}
+	name := strings.Trim(stdout, "\n")
+	return name
 }
 
 func GetAuthToken(t *testing.T) string {
@@ -144,18 +144,14 @@ func TestClusterStuff(t *testing.T) {
 		}
 	}
 	t.Run("long pause", func(t *testing.T) {
-		pcmd := exec.Command("/pumba", "pause", container("pilosa3"), "--duration", "10s")
-		pcmd.Stdout = os.Stdout
-		pcmd.Stderr = os.Stderr
+		if err := sendCmd("docker", "pause", container(t, "pilosa3")); err != nil {
+			t.Fatalf("sending pause: %v", err)
+		}
 		t.Log("pausing pilosa3 for 10s")
-
-		if err := pcmd.Start(); err != nil {
-			t.Fatalf("starting pumba command: %v", err)
+		time.Sleep(time.Second * 10)
+		if err := sendCmd("docker", "unpause", container(t, "pilosa3")); err != nil {
+			t.Fatalf("sending unpause: %v", err)
 		}
-		if err := pcmd.Wait(); err != nil {
-			t.Fatalf("waiting on pumba pause cmd: %v", err)
-		}
-
 		t.Log("done with pause, waiting for stability")
 		waitForStatus(t, cli1.Status, string(disco.ClusterStateNormal), 30, time.Second, ctx)
 		t.Log("done waiting for stability")
@@ -174,7 +170,7 @@ func TestClusterStuff(t *testing.T) {
 
 	t.Run("backup", func(t *testing.T) {
 		// do backup with node 1 down, but restart it after a few seconds
-		if err := sendCmd("docker", "stop", container("pilosa1")); err != nil {
+		if err := sendCmd("docker", "stop", container(t, "pilosa1")); err != nil {
 			t.Fatalf("sending stop command: %v", err)
 		}
 		var backupCmd *exec.Cmd
@@ -192,7 +188,7 @@ func TestClusterStuff(t *testing.T) {
 			}
 		}
 		time.Sleep(time.Second * 5)
-		if err = sendCmd("docker", "start", container("pilosa1")); err != nil {
+		if err = sendCmd("docker", "start", container(t, "pilosa1")); err != nil {
 			t.Fatalf("sending start command: %v", err)
 		}
 
@@ -228,12 +224,12 @@ func TestClusterStuff(t *testing.T) {
 			}
 		}
 		time.Sleep(time.Millisecond * 50)
-		if err = sendCmd("docker", "stop", container("pilosa2")); err != nil {
+		if err = sendCmd("docker", "stop", container(t, "pilosa2")); err != nil {
 			t.Fatalf("sending stop command: %v", err)
 		}
 
 		time.Sleep(time.Second * 10)
-		if err = sendCmd("docker", "start", container("pilosa2")); err != nil {
+		if err = sendCmd("docker", "start", container(t, "pilosa2")); err != nil {
 			t.Fatalf("sending stop command: %v", err)
 		}
 		if err := restoreCmd.Wait(); err != nil {
@@ -255,26 +251,29 @@ func TestClusterStuff(t *testing.T) {
 			}
 		}
 		time.Sleep(time.Millisecond * 10) // want the backup to get started, then fail
-		if err = sendCmd("docker", "stop", container("pilosa1")); err != nil {
-			t.Fatalf("sending stop command: %v", err)
+		fmt.Println("pausing all featurebasen")
+		if err = sendCmd("docker", "pause", container(t, "pilosa1")); err != nil {
+			t.Fatalf("sending pause command: %v", err)
 		}
-		if err = sendCmd("docker", "stop", container("pilosa2")); err != nil {
-			t.Fatalf("sending stop command: %v", err)
+		if err = sendCmd("docker", "pause", container(t, "pilosa2")); err != nil {
+			t.Fatalf("sending pause command: %v", err)
 		}
-		if err = sendCmd("docker", "stop", container("pilosa3")); err != nil {
-			t.Fatalf("sending stop command: %v", err)
+		if err = sendCmd("docker", "pause", container(t, "pilosa3")); err != nil {
+			t.Fatalf("sending pause command: %v", err)
 		}
 
-		time.Sleep(time.Second * 5)
+		fmt.Println("sleeping long")
+		time.Sleep(time.Second * 10)
+		fmt.Println("restarting")
 
-		if err = sendCmd("docker", "start", container("pilosa1")); err != nil {
-			t.Fatalf("sending start command: %v", err)
+		if err = sendCmd("docker", "unpause", container(t, "pilosa1")); err != nil {
+			t.Fatalf("sending unpause command: %v", err)
 		}
-		if err = sendCmd("docker", "start", container("pilosa2")); err != nil {
-			t.Fatalf("sending start command: %v", err)
+		if err = sendCmd("docker", "unpause", container(t, "pilosa2")); err != nil {
+			t.Fatalf("sending unpause command: %v", err)
 		}
-		if err = sendCmd("docker", "start", container("pilosa3")); err != nil {
-			t.Fatalf("sending start command: %v", err)
+		if err = sendCmd("docker", "unpause", container(t, "pilosa3")); err != nil {
+			t.Fatalf("sending unpause command: %v", err)
 		}
 		if err = backupCmd.Wait(); err == nil {
 			t.Fatal("backup command should have errored but didn't")
@@ -308,3 +307,14 @@ func waitForStatus(t *testing.T, stator func(context.Context) (string, error), s
 		t.Fatalf("waited %s for status: %s, got: %s", waited.String(), status, s)
 	}
 }
+
+// runCmd is a helper which uses os.Exec to run a command and returns
+// stdout and stderr as separate strings, and any error returned from
+// Command.Run
+func runCmd(name string, args ...string) (sout, serr string, err error) {
+	cmd := exec.Command(name, args...)
+	stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{}
+	cmd.Stdout, cmd.Stderr = stdout, stderr
+	err = cmd.Run()
+	return stdout.String(), stderr.String(), errors.Wrap(err, "running command")
+}

internal/clustertests/pause_node_test.go

@@ -43,13 +43,13 @@ func sendCmd(cmd string, args ...string) error {
 	return nil
 }
 
-func unpauseNode(node string) error {
-	unpauseArgs := []string{"container", "unpause", container(node)}
+func unpauseNode(t *testing.T, node string) error {
+	unpauseArgs := []string{"container", "unpause", container(t, node)}
 	return sendCmd("docker", unpauseArgs...)
 }
 
-func pauseNode(node string) error {
-	pauseArgs := []string{"container", "pause", container(node)}
+func pauseNode(t *testing.T, node string) error {
+	pauseArgs := []string{"container", "pause", container(t, node)}
 	return sendCmd("docker", pauseArgs...)
 }
 
@@ -346,7 +346,7 @@ func TestPauseReplica(t *testing.T) {
 
 	// pause node
 	t.Logf("pause %s", nodeToPause)
-	err = pauseNode(nodeToPause)
+	err = pauseNode(t, nodeToPause)
 	if err != nil {
 		t.Fatalf("error on pause node %s: %v", nodeToPause, err)
 	}
@@ -369,7 +369,7 @@ func TestPauseReplica(t *testing.T) {
 
 	// wait for cluster status to get back to normal
 	t.Logf("unpause %s", nodeToPause)
-	err = unpauseNode(nodeToPause)
+	err = unpauseNode(t, nodeToPause)
 	if err != nil {
 		t.Fatalf("error on unpause node %s: %v", nodeToPause, err)
 	}