From ab1e08f1a4d267558c371b99b0e10647742e6515 Mon Sep 17 00:00:00 2001 From: Dhia Ayachi Date: Wed, 11 Oct 2023 11:26:07 -0400 Subject: [PATCH] fix flaking container tests (#19134) --- .../libs/cluster/encryption.go | 144 ++++++++++-------- .../test/ratelimit/ratelimit_test.go | 9 +- 2 files changed, 82 insertions(+), 71 deletions(-) diff --git a/test/integration/consul-container/libs/cluster/encryption.go b/test/integration/consul-container/libs/cluster/encryption.go index 79e7133fbb..1d7ae072bf 100644 --- a/test/integration/consul-container/libs/cluster/encryption.go +++ b/test/integration/consul-container/libs/cluster/encryption.go @@ -8,6 +8,7 @@ import ( "crypto/rand" "encoding/base64" "fmt" + "github.com/hashicorp/consul/sdk/testutil/retry" "io" "path/filepath" "testing" @@ -47,89 +48,98 @@ func (c *BuildContext) createTLSCAFiles(t *testing.T) { // TODO: cleanup anything with the prefix? - // Create a volume to hold the data. - err = utils.DockerExec([]string{"volume", "create", c.certVolume}, io.Discard) - require.NoError(t, err, "could not create docker volume to hold cert data: %s", c.certVolume) + retry.Run(t, func(r *retry.R) { + // Create a volume to hold the data. + err = utils.DockerExec([]string{"volume", "create", c.certVolume}, io.Discard) + require.NoError(r, err, "could not create docker volume to hold cert data: %s", c.certVolume) + }) t.Cleanup(func() { _ = utils.DockerExec([]string{"volume", "rm", c.certVolume}, io.Discard) }) - err = utils.DockerExec([]string{"run", - "--rm", - "-i", - "--net=none", - "-v", c.certVolume + ":/data", - "busybox:latest", - "sh", "-c", - // Need this so the permissions stick; docker seems to treat unused volumes differently. - `touch /data/VOLUME_PLACEHOLDER && chown -R ` + consulUserArg + ` /data`, - }, io.Discard) - require.NoError(t, err, "could not initialize docker volume for cert data: %s", c.certVolume) - - err = utils.DockerExec([]string{"run", - "--rm", - "-i", - "--net=none", - "-u", consulUserArg, - "-v", c.certVolume + ":/data", - "-w", "/data", - "--entrypoint", "", - c.DockerImage(), - "consul", "tls", "ca", "create", - }, io.Discard) - require.NoError(t, err, "could not create TLS certificate authority in docker volume: %s", c.certVolume) - + retry.Run(t, func(r *retry.R) { + err := utils.DockerExec([]string{"run", + "--rm", + "-i", + "--net=none", + "-v", c.certVolume + ":/data", + "busybox:latest", + "sh", "-c", + // Need this so the permissions stick; docker seems to treat unused volumes differently. + `touch /data/VOLUME_PLACEHOLDER && chown -R ` + consulUserArg + ` /data`, + }, io.Discard) + require.NoError(r, err, "could not initialize docker volume for cert data: %s", c.certVolume) + }) + retry.Run(t, func(r *retry.R) { + err = utils.DockerExec([]string{"run", + "--rm", + "-i", + "--net=none", + "-u", consulUserArg, + "-v", c.certVolume + ":/data", + "-w", "/data", + "--entrypoint", "", + c.DockerImage(), + "consul", "tls", "ca", "create", + }, io.Discard) + require.NoError(r, err, "could not create TLS certificate authority in docker volume: %s", c.certVolume) + }) var w bytes.Buffer - err = utils.DockerExec([]string{"run", - "--rm", - "-i", - "--net=none", - "-u", consulUserArg, - "-v", c.certVolume + ":/data", - "-w", "/data", - "--entrypoint", "", - c.DockerImage(), - "cat", filepath.Join("/data", ConsulCACertPEM), - }, &w) - require.NoError(t, err, "could not extract TLS CA certificate authority public key from docker volume: %s", c.certVolume) + retry.Run(t, func(r *retry.R) { + err = utils.DockerExec([]string{"run", + "--rm", + "-i", + "--net=none", + "-u", consulUserArg, + "-v", c.certVolume + ":/data", + "-w", "/data", + "--entrypoint", "", + c.DockerImage(), + "cat", filepath.Join("/data", ConsulCACertPEM), + }, &w) + require.NoError(r, err, "could not extract TLS CA certificate authority public key from docker volume: %s", c.certVolume) + }) c.caCert = w.String() } func (c *BuildContext) createTLSCertFiles(t *testing.T, dc string) (keyFileName, certFileName string) { require.NotEmpty(t, "the CA has not been initialized yet") - - err := utils.DockerExec([]string{"run", - "--rm", - "-i", - "--net=none", - "-u", consulUserArg, - "-v", c.certVolume + ":/data", - "-w", "/data", - "--entrypoint", "", - c.DockerImage(), - "consul", "tls", "cert", "create", "-server", "-dc", dc, - }, io.Discard) - require.NoError(t, err, "could not create TLS server certificate dc=%q in docker volume: %s", dc, c.certVolume) + retry.Run(t, func(r *retry.R) { + err := utils.DockerExec([]string{"run", + "--rm", + "-i", + "--net=none", + "-u", consulUserArg, + "-v", c.certVolume + ":/data", + "-w", "/data", + "--entrypoint", "", + c.DockerImage(), + "consul", "tls", "cert", "create", "-server", "-dc", dc, + }, io.Discard) + require.NoError(r, err, "could not create TLS server certificate dc=%q in docker volume: %s", dc, c.certVolume) + }) prefix := fmt.Sprintf("%s-server-%s", dc, "consul") certFileName = fmt.Sprintf("%s-%d.pem", prefix, c.tlsCertIndex) keyFileName = fmt.Sprintf("%s-%d-key.pem", prefix, c.tlsCertIndex) - for _, fn := range []string{certFileName, keyFileName} { - err = utils.DockerExec([]string{"run", - "--rm", - "-i", - "--net=none", - "-u", consulUserArg, - "-v", c.certVolume + ":/data:ro", - "-w", "/data", - "--entrypoint", "", - c.DockerImage(), - "stat", filepath.Join("/data", fn), - }, io.Discard) - require.NoError(t, err, "Generated TLS cert file %q does not exist in volume", fn) - } + retry.Run(t, func(r *retry.R) { + for _, fn := range []string{certFileName, keyFileName} { + err := utils.DockerExec([]string{"run", + "--rm", + "-i", + "--net=none", + "-u", consulUserArg, + "-v", c.certVolume + ":/data:ro", + "-w", "/data", + "--entrypoint", "", + c.DockerImage(), + "stat", filepath.Join("/data", fn), + }, io.Discard) + require.NoError(r, err, "Generated TLS cert file %q does not exist in volume", fn) + } + }) return keyFileName, certFileName } diff --git a/test/integration/consul-container/test/ratelimit/ratelimit_test.go b/test/integration/consul-container/test/ratelimit/ratelimit_test.go index ed35bcf646..e598e0ceb8 100644 --- a/test/integration/consul-container/test/ratelimit/ratelimit_test.go +++ b/test/integration/consul-container/test/ratelimit/ratelimit_test.go @@ -32,6 +32,7 @@ const ( // - logs for exceeding func TestServerRequestRateLimit(t *testing.T) { + type action struct { function func(client *api.Client) error rateLimitOperation string @@ -166,12 +167,12 @@ func TestServerRequestRateLimit(t *testing.T) { for _, op := range tc.operations { timer := &retry.Timer{Timeout: 15 * time.Second, Wait: 500 * time.Millisecond} retry.RunWith(timer, t, func(r *retry.R) { - checkForMetric(t, cluster, op.action.rateLimitOperation, op.action.rateLimitType, tc.mode, op.expectMetric) + checkForMetric(r, cluster, op.action.rateLimitOperation, op.action.rateLimitType, tc.mode, op.expectMetric) // validate logs // putting this last as there are cases where logs // were not present in consumer when assertion was made. - checkLogsForMessage(t, clusterConfig.LogConsumer.Msgs, + checkLogsForMessage(r, clusterConfig.LogConsumer.Msgs, fmt.Sprintf("[DEBUG] agent.server.rpc-rate-limit: RPC exceeded allowed rate limit: rpc=%s", op.action.rateLimitOperation), op.action.rateLimitOperation, "exceeded", op.expectExceededLog) @@ -190,7 +191,7 @@ func setupClusterAndClient(t *testing.T, config *libtopology.ClusterConfig, isSe return cluster, client } -func checkForMetric(t *testing.T, cluster *libcluster.Cluster, operationName string, expectedLimitType string, expectedMode string, expectMetric bool) { +func checkForMetric(t require.TestingT, cluster *libcluster.Cluster, operationName string, expectedLimitType string, expectedMode string, expectMetric bool) { // validate metrics server, err := cluster.GetClient(nil, true) require.NoError(t, err) @@ -228,7 +229,7 @@ func checkForMetric(t *testing.T, cluster *libcluster.Cluster, operationName str } } -func checkLogsForMessage(t *testing.T, logs []string, msg string, operationName string, logType string, logShouldExist bool) { +func checkLogsForMessage(t require.TestingT, logs []string, msg string, operationName string, logType string, logShouldExist bool) { if logShouldExist { found := false for _, log := range logs {