workaround race condition in ContainerList

Signed-off-by: Nicolas De Loof <[email protected]>

Author: ndeloof

cmd/compose/remove.go

@@ -64,16 +64,6 @@ func runRemove(ctx context.Context, backend api.Service, opts removeOptions, ser
 		return err
 	}
 
-	if opts.stop {
-		err := backend.Stop(ctx, name, api.StopOptions{
-			Services: services,
-			Project:  project,
-		})
-		if err != nil {
-			return err
-		}
-	}
-
 	return backend.Remove(ctx, name, api.RemoveOptions{
 		Services: services,
 		Force:    opts.force,

pkg/compose/remove.go

@@ -31,6 +31,17 @@ import (
 
 func (s *composeService) Remove(ctx context.Context, projectName string, options api.RemoveOptions) error {
 	projectName = strings.ToLower(projectName)
+
+	if options.Stop {
+		err := s.Stop(ctx, projectName, api.StopOptions{
+			Services: options.Services,
+			Project:  options.Project,
+		})
+		if err != nil {
+			return err
+		}
+	}
+
 	containers, err := s.getContainers(ctx, projectName, oneOffExclude, true, options.Services...)
 	if err != nil {
 		if api.IsNotFoundError(err) {
@@ -44,14 +55,27 @@ func (s *composeService) Remove(ctx context.Context, projectName string, options
 		containers = containers.filter(isService(options.Project.ServiceNames()...))
 	}
 
-	stoppedContainers := containers.filter(func(c moby.Container) bool {
-		return c.State != ContainerRunning || (options.Stop && s.dryRun)
-	})
+	var stoppedContainers Containers
+	for _, container := range containers {
+		// We have to inspect containers, as State reported by getContainers suffers a race condition
+		inspected, err := s.apiClient().ContainerInspect(ctx, container.ID)
+		if api.IsNotFoundError(err) {
+			// Already removed. Maybe configured with auto-remove
+			continue
+		}
+		if err != nil {
+			return err
+		}
+		if !inspected.State.Running || (options.Stop && s.dryRun) {
+			stoppedContainers = append(stoppedContainers, container)
+		}
+	}
 
 	var names []string
 	stoppedContainers.forEach(func(c moby.Container) {
 		names = append(names, getCanonicalContainerName(c))
 	})
+	fmt.Fprintln(s.stderr(), names)
 
 	if len(names) == 0 {
 		fmt.Fprintln(s.stderr(), "No stopped containers")

pkg/e2e/compose_test.go

@@ -169,23 +169,34 @@ func TestRm(t *testing.T) {
 		c.RunDockerComposeCmd(t, "-f", "./fixtures/simple-composefile/compose.yaml", "-p", projectName, "up", "-d")
 	})
 
-	t.Run("rm -sf", func(t *testing.T) {
+	t.Run("rm --stop --force simple", func(t *testing.T) {
 		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/simple-composefile/compose.yaml", "-p", projectName, "rm",
-			"-sf", "simple")
+			"--stop", "--force", "simple")
 		res.Assert(t, icmd.Expected{Err: "Removed", ExitCode: 0})
 	})
 
-	t.Run("check containers after rm -sf", func(t *testing.T) {
+	t.Run("check containers after rm", func(t *testing.T) {
 		res := c.RunDockerCmd(t, "ps", "--all")
-		assert.Assert(t, !strings.Contains(res.Combined(), projectName+"_simple"), res.Combined())
+		assert.Assert(t, !strings.Contains(res.Combined(), projectName+"-simple"), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), projectName+"-another"), res.Combined())
 	})
 
-	t.Run("rm -sf <none>", func(t *testing.T) {
+	t.Run("up (again)", func(t *testing.T) {
+		c.RunDockerComposeCmd(t, "-f", "./fixtures/simple-composefile/compose.yaml", "-p", projectName, "up", "-d")
+	})
+
+	t.Run("rm ---stop --force <none>", func(t *testing.T) {
 		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/simple-composefile/compose.yaml", "-p", projectName, "rm",
-			"-sf", "simple")
+			"--stop", "--force")
 		res.Assert(t, icmd.Expected{ExitCode: 0})
 	})
 
+	t.Run("check containers after rm", func(t *testing.T) {
+		res := c.RunDockerCmd(t, "ps", "--all")
+		assert.Assert(t, !strings.Contains(res.Combined(), projectName+"-simple"), res.Combined())
+		assert.Assert(t, !strings.Contains(res.Combined(), projectName+"-another"), res.Combined())
+	})
+
 	t.Run("down", func(t *testing.T) {
 		c.RunDockerComposeCmd(t, "-p", projectName, "down")
 	})