From eec447185163aa91e13268cbe5f5084aa1275686 Mon Sep 17 00:00:00 2001
From: Charlie Le <charlie_le@apple.com>
Date: Sun, 22 Sep 2024 12:51:57 -0700
Subject: [PATCH] Wait until container is removed after stopping

The E2E tests use `docker stop` to stop the container, but the container
can still be in the process of being removed after the stop command is
executed. This can cause tests that stop and start a container in rapid
succession with the same container name to fail with conflicting
container names.

Example error message in an integration test:

```
00:08:14 Stopping query-frontend
00:08:14 Stopping store-gateway
=== RUN
TestNewDistributorsCanPushToOldIngestersWithReplication/Backward_compatibility_upgradir
00:08:14 Starting store-gateway
00:08:14 store-gateway: docker: Error response from daemon: Conflict. The container name "/e2e-cortex-test-store-gateway" is already in use by container "b750a7b7e2e3eebc617ac832de2c8a643f884bbc79b0c8961ceebace8cad5c85". You have to remove (or rename) that container to be able to reuse that name.
```

Signed-off-by: Charlie Le <charlie_le@apple.com>
---
 integration/e2e/scenario_test.go | 14 ++++++++++++++
 integration/e2e/service.go       | 16 +++++++++++++---
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/integration/e2e/scenario_test.go b/integration/e2e/scenario_test.go
index 50b61dbc07..12249f3900 100644
--- a/integration/e2e/scenario_test.go
+++ b/integration/e2e/scenario_test.go
@@ -146,3 +146,17 @@ func TestScenario(t *testing.T) {
 	_, err = bkt.Get(context.Background(), "recipe")
 	require.Error(t, err)
 }
+
+// TestStartStop tests for ensuring that when the container is stopped, it can be started again.
+// This is to test that the stop waits for the container to be stopped and cleaned up before returning.
+func TestStartStop(t *testing.T) {
+	s, err := e2e.NewScenario("e2e-scenario-test")
+	require.NoError(t, err)
+
+	m1 := e2edb.NewMinio(9000, bktName)
+
+	for i := 0; i < 10; i++ {
+		require.NoError(t, s.Start(m1))
+		require.NoError(t, s.Stop(m1))
+	}
+}
diff --git a/integration/e2e/service.go b/integration/e2e/service.go
index 50fc0b8301..a9d38fee0b 100644
--- a/integration/e2e/service.go
+++ b/integration/e2e/service.go
@@ -151,6 +151,9 @@ func (s *ConcreteService) Stop() error {
 		logger.Log(string(out))
 		return err
 	}
+
+	s.Wait()
+
 	s.usedNetworkName = ""
 
 	return nil
@@ -168,15 +171,22 @@ func (s *ConcreteService) Kill() error {
 		return err
 	}
 
-	// Wait until the container actually stopped. However, this could fail if
-	// the container already exited, so we just ignore the error.
-	_, _ = RunCommandAndGetOutput("docker", "wait", s.containerName())
+	s.Wait()
 
 	s.usedNetworkName = ""
 
 	return nil
 }
 
+// Wait waits until the service is stopped.
+func (s *ConcreteService) Wait() {
+	// Wait until the container actually stopped. However, this could fail if
+	// the container already exited, so we just ignore the error.
+	if out, err := RunCommandAndGetOutput("docker", "wait", s.containerName()); err != nil {
+		logger.Log(string(out))
+	}
+}
+
 // Endpoint returns external (from host perspective) service endpoint (host:port) for given internal port.
 // External means that it will be accessible only from host, but not from docker containers.
 //