address review comments

hashicorp · Feb 10, 2023 · 1408557 · 1408557
1 parent 87a5846
commit 1408557
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 51 deletions.
diff --git a/test/integration/consul-container/libs/service/connect.go b/test/integration/consul-container/libs/service/connect.go
@@ -32,8 +32,19 @@ type ConnectContainer struct {
 
 var _ Service = (*ConnectContainer)(nil)
 
-func (g ConnectContainer) Exec(ctx context.Context, cmd []string) (int, io.Reader, error) {
-	return g.container.Exec(ctx, cmd)
+func (g ConnectContainer) Exec(ctx context.Context, cmd []string) (string, error) {
+	exitCode, reader, err := g.container.Exec(ctx, cmd)
+	if err != nil {
+		return "", fmt.Errorf("exec with error %s", err)
+	}
+	if exitCode != 0 {
+		return "", fmt.Errorf("exec with exit code %d", exitCode)
+	}
+	buf, err := io.ReadAll(reader)
+	if err != nil {
+		return "", fmt.Errorf("error reading from exec output: %w", err)
+	}
+	return string(buf), nil
 }
 
 func (g ConnectContainer) Export(partition, peer string, client *api.Client) error {

diff --git a/test/integration/consul-container/libs/service/examples.go b/test/integration/consul-container/libs/service/examples.go
@@ -29,8 +29,19 @@ type exampleContainer struct {
 
 var _ Service = (*exampleContainer)(nil)
 
-func (g exampleContainer) Exec(ctx context.Context, cmd []string) (int, io.Reader, error) {
-	return g.container.Exec(ctx, cmd)
+func (g exampleContainer) Exec(ctx context.Context, cmd []string) (string, error) {
+	exitCode, reader, err := g.container.Exec(ctx, cmd)
+	if err != nil {
+		return "", fmt.Errorf("exec with error %s", err)
+	}
+	if exitCode != 0 {
+		return "", fmt.Errorf("exec with exit code %d", exitCode)
+	}
+	buf, err := io.ReadAll(reader)
+	if err != nil {
+		return "", fmt.Errorf("error reading from exec output: %w", err)
+	}
+	return string(buf), nil
 }
 
 func (g exampleContainer) Export(partition, peerName string, client *api.Client) error {

diff --git a/test/integration/consul-container/libs/service/gateway.go b/test/integration/consul-container/libs/service/gateway.go
@@ -30,8 +30,19 @@ type gatewayContainer struct {
 
 var _ Service = (*gatewayContainer)(nil)
 
-func (g gatewayContainer) Exec(ctx context.Context, cmd []string) (int, io.Reader, error) {
-	return g.container.Exec(ctx, cmd)
+func (g gatewayContainer) Exec(ctx context.Context, cmd []string) (string, error) {
+	exitCode, reader, err := g.container.Exec(ctx, cmd)
+	if err != nil {
+		return "", fmt.Errorf("exec with error %s", err)
+	}
+	if exitCode != 0 {
+		return "", fmt.Errorf("exec with exit code %d", exitCode)
+	}
+	buf, err := io.ReadAll(reader)
+	if err != nil {
+		return "", fmt.Errorf("error reading from exec output: %w", err)
+	}
+	return string(buf), nil
 }
 
 func (g gatewayContainer) Export(partition, peer string, client *api.Client) error {

diff --git a/test/integration/consul-container/libs/service/service.go b/test/integration/consul-container/libs/service/service.go
@@ -3,13 +3,12 @@ package service
 import (
 	"context"
 	"github.com/hashicorp/consul/api"
-	"io"
 )
 
 // Service represents a process that will be registered with the
 // Consul catalog, including Consul components such as sidecars and gateways
 type Service interface {
-	Exec(ctx context.Context, cmd []string) (int, io.Reader, error)
+	Exec(ctx context.Context, cmd []string) (string, error)
 	// Export a service to the peering cluster
 	Export(partition, peer string, client *api.Client) error
 	GetAddr() (string, int)

diff --git a/test/integration/consul-container/test/troubleshoot/troubleshoot_proxy_test.go b/test/integration/consul-container/test/troubleshoot/troubleshoot_proxy_test.go
@@ -3,18 +3,20 @@ package troubleshoot
 import (
 	"context"
 	"fmt"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
 	libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
 	libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
 	"github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
 	"github.com/hashicorp/consul/test/integration/consul-container/test/observability"
-	"github.com/stretchr/testify/require"
-	"io"
-	"testing"
-	"time"
 )
 
 func TestTroubleshootProxy_Success(t *testing.T) {
-
+	t.Parallel()
 	cluster, _, _ := topology.NewPeeringCluster(t, 1, &libcluster.BuildOptions{
 		Datacenter:           "dc1",
 		InjectAutoEncryption: true,
@@ -25,22 +27,22 @@ func TestTroubleshootProxy_Success(t *testing.T) {
 	clientSidecar, ok := clientService.(*libservice.ConnectContainer)
 	require.True(t, ok)
 	_, port := clientSidecar.GetInternalAdminAddr()
-	// wait for envoy
-	time.Sleep(10 * time.Second)
-	_, outputReader, err := clientSidecar.Exec(context.Background(), []string{"consul", "troubleshoot", "proxy",
-		"-envoy-admin-endpoint", fmt.Sprintf("localhost:%v", port),
-		"-upstream-envoy-id", libservice.StaticServerServiceName})
-	require.NoError(t, err)
-	buf, err := io.ReadAll(outputReader)
-	require.NoError(t, err)
-	require.Contains(t, string(buf), "certificates are valid")
-	require.Contains(t, string(buf), fmt.Sprintf("listener for upstream \"%s\" found", libservice.StaticServerServiceName))
-	require.Contains(t, string(buf), fmt.Sprintf("route for upstream \"%s\" found", libservice.StaticServerServiceName))
-	require.Contains(t, string(buf), "\nhealthy endpoints for cluster")
+
+	require.Eventually(t, func() bool {
+		output, err := clientSidecar.Exec(context.Background(), []string{"consul", "troubleshoot", "proxy",
+			"-envoy-admin-endpoint", fmt.Sprintf("localhost:%v", port),
+			"-upstream-envoy-id", libservice.StaticServerServiceName})
+		require.NoError(t, err)
+		certsValid := strings.Contains(output, "certificates are valid")
+		listenersExist := strings.Contains(output, fmt.Sprintf("listener for upstream \"%s\" found", libservice.StaticServerServiceName))
+		routesExist := strings.Contains(output, fmt.Sprintf("route for upstream \"%s\" found", libservice.StaticServerServiceName))
+		healthyEndpoints := strings.Contains(output, "\nhealthy endpoints for cluster")
+		return certsValid && listenersExist && routesExist && healthyEndpoints
+	}, 60*time.Second, 10*time.Second)
 }
 
 func TestTroubleshootProxy_FailHealthCheck(t *testing.T) {
-
+	t.Parallel()
 	cluster, _, _ := topology.NewPeeringCluster(t, 1, &libcluster.BuildOptions{
 		Datacenter:           "dc1",
 		InjectAutoEncryption: true,
@@ -53,21 +55,27 @@ func TestTroubleshootProxy_FailHealthCheck(t *testing.T) {
 
 	_, clientAdminPort := clientSidecar.GetInternalAdminAddr()
 
-	// wait for envoy
-	time.Sleep(5 * time.Second)
+	require.Eventually(t, func() bool {
+		output, err := clientSidecar.Exec(context.Background(), []string{"consul", "troubleshoot", "proxy",
+			"-envoy-admin-endpoint", fmt.Sprintf("localhost:%v", clientAdminPort),
+			"-upstream-envoy-id", libservice.StaticServerServiceName})
+		require.NoError(t, err)
+		return strings.Contains(output, "no healthy endpoints for cluster")
+	}, 60*time.Second, 10*time.Second)
+
 	err := serverService.Terminate()
 	require.NoError(t, err)
 
-	time.Sleep(10 * time.Second)
-	fmt.Println("waited 10 seconds")
-	_, outputReader, err := clientSidecar.Exec(context.Background(), []string{"consul", "troubleshoot", "proxy",
-		"-envoy-admin-endpoint", fmt.Sprintf("localhost:%v", clientAdminPort),
-		"-upstream-envoy-id", libservice.StaticServerServiceName})
-	require.NoError(t, err)
-	buf, err := io.ReadAll(outputReader)
-	require.NoError(t, err)
-	require.Contains(t, string(buf), "certificates are valid")
-	require.Contains(t, string(buf), fmt.Sprintf("listener for upstream \"%s\" found", libservice.StaticServerServiceName))
-	require.Contains(t, string(buf), fmt.Sprintf("route for upstream \"%s\" found", libservice.StaticServerServiceName))
-	require.Contains(t, string(buf), "no healthy endpoints for cluster")
+	require.Eventually(t, func() bool {
+		output, err := clientSidecar.Exec(context.Background(), []string{"consul", "troubleshoot", "proxy",
+			"-envoy-admin-endpoint", fmt.Sprintf("localhost:%v", clientAdminPort),
+			"-upstream-envoy-id", libservice.StaticServerServiceName})
+		require.NoError(t, err)
+
+		certsValid := strings.Contains(output, "certificates are valid")
+		listenersExist := strings.Contains(output, fmt.Sprintf("listener for upstream \"%s\" found", libservice.StaticServerServiceName))
+		routesExist := strings.Contains(output, fmt.Sprintf("route for upstream \"%s\" found", libservice.StaticServerServiceName))
+		endpointUnhealthy := strings.Contains(output, "no healthy endpoints for cluster")
+		return certsValid && listenersExist && routesExist && endpointUnhealthy
+	}, 60*time.Second, 10*time.Second)
 }
diff --git a/test/integration/consul-container/test/troubleshoot/troubleshoot_upstream_test.go b/test/integration/consul-container/test/troubleshoot/troubleshoot_upstream_test.go
@@ -3,18 +3,21 @@ package troubleshoot
 import (
 	"context"
 	"fmt"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
 	libcluster "github.com/hashicorp/consul/test/integration/consul-container/libs/cluster"
 	libservice "github.com/hashicorp/consul/test/integration/consul-container/libs/service"
 	"github.com/hashicorp/consul/test/integration/consul-container/libs/topology"
 	"github.com/hashicorp/consul/test/integration/consul-container/test/observability"
-	"github.com/stretchr/testify/require"
-	"io"
-	"testing"
-	"time"
 )
 
 func TestTroubleshootUpstream_Success(t *testing.T) {
 
+	t.Parallel()
 	cluster, _, _ := topology.NewPeeringCluster(t, 1, &libcluster.BuildOptions{
 		Datacenter:           "dc1",
 		InjectAutoEncryption: true,
@@ -25,11 +28,10 @@ func TestTroubleshootUpstream_Success(t *testing.T) {
 	clientSidecar, ok := clientService.(*libservice.ConnectContainer)
 	require.True(t, ok)
 	_, port := clientSidecar.GetInternalAdminAddr()
-	// wait for envoy
-	time.Sleep(5 * time.Second)
-	_, outputReader, err := clientSidecar.Exec(context.Background(), []string{"consul", "troubleshoot", "upstreams", "-envoy-admin-endpoint", fmt.Sprintf("localhost:%v", port)})
-	require.NoError(t, err)
-	buf, err := io.ReadAll(outputReader)
-	require.NoError(t, err)
-	require.Contains(t, string(buf), libservice.StaticServerServiceName)
+
+	require.Eventually(t, func() bool {
+		output, err := clientSidecar.Exec(context.Background(), []string{"consul", "troubleshoot", "upstreams", "-envoy-admin-endpoint", fmt.Sprintf("localhost:%v", port)})
+		require.NoError(t, err)
+		return strings.Contains(output, libservice.StaticServerServiceName)
+	}, 30*time.Second, 5*time.Second)
 }