Fix issue when many containers are created but only one is deleted

container/container_test.go

@@ -10,9 +10,57 @@ import (
 
 	"github.com/mesg-foundation/core/config"
 	"github.com/mesg-foundation/core/container/dockertest"
+	"github.com/mesg-foundation/core/utils/docker/mocks"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
+func newTesting(t *testing.T) (*DockerContainer, *mocks.CommonAPIClient) {
+	m := &mocks.CommonAPIClient{}
+	mockNew(m)
+
+	c, err := New(ClientOption(m))
+	require.NoError(t, err)
+
+	return c, m
+}
+
+func mockNew(m *mocks.CommonAPIClient) {
+	m.On("NegotiateAPIVersion", mock.Anything).Once().Return()
+	m.On("Info", mock.Anything).Once().Return(types.Info{Swarm: swarm.Info{NodeID: "1"}}, nil)
+	m.On("NetworkInspect", mock.Anything, "core", types.NetworkInspectOptions{}).Once().
+		Return(types.NetworkResource{ID: "1"}, nil)
+}
+
+// TODO: support all status types.
+func mockStatus(t *testing.T, m *mocks.CommonAPIClient, namespace string, wantedStatus StatusType) {
+	var (
+		containerID = "1"
+	)
+
+	m.On("ContainerList", mock.AnythingOfType("*context.timerCtx"), types.ContainerListOptions{
+		Filters: filters.NewArgs(filters.KeyValuePair{
+			Key:   "label",
+			Value: "com.docker.stack.namespace=" + namespace,
+		}),
+		Limit: 1,
+	}).Once().
+		Return([]types.Container{{ID: "1"}}, nil)
+
+	containerInspect := m.On("ContainerInspect", mock.AnythingOfType("*context.timerCtx"), containerID).Once()
+	serviceInspect := m.On("ServiceInspectWithRaw", mock.Anything, namespace, types.ServiceInspectOptions{}).Once()
+	switch wantedStatus {
+	case RUNNING:
+		containerInspect.Return(types.ContainerJSON{}, nil)
+		serviceInspect.Return(swarm.Service{}, nil, nil)
+	case STOPPED:
+		containerInspect.Return(types.ContainerJSON{}, dockertest.NotFoundErr{})
+		serviceInspect.Return(swarm.Service{}, nil, dockertest.NotFoundErr{})
+	default:
+		t.Errorf("unhandled status %v", wantedStatus)
+	}
+}
+
 func TestNew(t *testing.T) {
 	dt := dockertest.New()
 	c, err := New(ClientOption(dt.Client()))

container/service.go

@@ -70,11 +70,23 @@ func (c *DockerContainer) StopService(namespace []string) error {
 
 	ctx, cancel := context.WithTimeout(context.Background(), c.callTimeout)
 	defer cancel()
-	container, err := c.FindContainer(namespace)
-	if err != nil && !docker.IsErrNotFound(err) {
+	if err := c.client.ServiceRemove(ctx, c.Namespace(namespace)); err != nil && !docker.IsErrNotFound(err) {
 		return err
 	}
-	if err := c.client.ServiceRemove(ctx, c.Namespace(namespace)); err != nil && !docker.IsErrNotFound(err) {
+	if err := c.deletePendingContainer(namespace); err != nil {
+		return err
+	}
+	return c.waitForStatus(namespace, STOPPED)
+}
+
+func (c *DockerContainer) deletePendingContainer(namespace []string) error {
+	ctx, cancel := context.WithTimeout(context.Background(), c.callTimeout)
+	defer cancel()
+	container, err := c.FindContainer(namespace)
+	if err != nil {
+		if docker.IsErrNotFound(err) {
+			return nil
+		}
 		return err
 	}
 	// TOFIX: Hack to force Docker to remove the containers.
@@ -86,7 +98,7 @@ func (c *DockerContainer) StopService(namespace []string) error {
 		c.client.ContainerStop(ctx, container.ID, &timeout)
 		c.client.ContainerRemove(ctx, container.ID, types.ContainerRemoveOptions{})
 	}
-	return c.waitForStatus(namespace, STOPPED)
+	return c.deletePendingContainer(namespace)
 }
 
 // ServiceLogs returns the logs of a service.

container/service_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/mesg-foundation/core/container/dockertest"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -36,32 +37,42 @@ func TestStartService(t *testing.T) {
 }
 
 func TestStopService(t *testing.T) {
-	t.Skip("put back when dockertest will be replaced with testify.Mock")
-
-	namespace := []string{"namespace"}
-	dt := dockertest.New()
-	c, _ := New(ClientOption(dt.Client()))
-	containerID := "1"
-
-	dt.ProvideContainerList([]types.Container{
-		{ID: containerID},
-	}, nil)
-	dt.ProvideContainerInspect(types.ContainerJSON{
-		ContainerJSONBase: &types.ContainerJSONBase{ID: containerID},
-	}, nil)
-
-	dt.ProvideContainerList(nil, dockertest.NotFoundErr{})
-	dt.ProvideServiceInspectWithRaw(swarm.Service{}, nil, dockertest.NotFoundErr{})
-
-	require.Nil(t, c.StopService(namespace))
-	require.Equal(t, c.Namespace(namespace), (<-dt.LastServiceRemove()).ServiceID)
+	var (
+		c, m          = newTesting(t)
+		containerID   = "1"
+		namespace     = []string{"2"}
+		fullNamespace = c.Namespace(namespace)
+	)
+
+	mockStatus(t, m, fullNamespace, RUNNING)
+	m.On("ServiceRemove", mock.Anything, fullNamespace).Once().Return(nil)
+	m.On("ContainerList", mock.AnythingOfType("*context.timerCtx"), types.ContainerListOptions{
+		Filters: filters.NewArgs(filters.KeyValuePair{
+			Key:   "label",
+			Value: "com.docker.stack.namespace=" + fullNamespace,
+		}),
+		Limit: 1,
+	}).Once().
+		Return([]types.Container{{ID: containerID}}, nil)
+	m.On("ContainerInspect", mock.AnythingOfType("*context.timerCtx"), containerID).Once().
+		Return(types.ContainerJSON{
+			ContainerJSONBase: &types.ContainerJSONBase{ID: containerID},
+		}, nil)
+	m.On("ContainerStop", mock.Anything, containerID, mock.AnythingOfType("*time.Duration")).Once().Return(nil)
+	m.On("ContainerRemove", mock.Anything, containerID, types.ContainerRemoveOptions{}).Once().Return(nil)
+	m.On("ContainerList", mock.AnythingOfType("*context.timerCtx"), types.ContainerListOptions{
+		Filters: filters.NewArgs(filters.KeyValuePair{
+			Key:   "label",
+			Value: "com.docker.stack.namespace=" + fullNamespace,
+		}),
+		Limit: 1,
+	}).Once().
+		Return(nil, dockertest.NotFoundErr{})
+	mockWaitForStatus(t, m, fullNamespace, STOPPED)
 
-	ls := <-dt.LastContainerStop()
-	require.Equal(t, containerID, ls.Container)
+	require.NoError(t, c.StopService(namespace))
 
-	lr := <-dt.LastContainerRemove()
-	require.Equal(t, containerID, lr.Container)
-	require.Equal(t, types.ContainerRemoveOptions{}, lr.Options)
+	m.AssertExpectations(t)
 }
 
 func TestStopNotExistingService(t *testing.T) {

container/wait_test.go

@@ -4,11 +4,25 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/mesg-foundation/core/container/dockertest"
+	"github.com/mesg-foundation/core/utils/docker/mocks"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
+func mockWaitForStatus(t *testing.T, m *mocks.CommonAPIClient, namespace string, wantedStatus StatusType) {
+	m.On("TaskList", mock.Anything, types.TaskListOptions{
+		Filters: filters.NewArgs(filters.KeyValuePair{
+			Key:   "label",
+			Value: "com.docker.stack.namespace=" + namespace,
+		}),
+	}).Once().
+		Return([]swarm.Task{}, nil)
+	mockStatus(t, m, namespace, wantedStatus)
+}
+
 func TestWaitForStatusRunning(t *testing.T) {
 	namespace := []string{"namespace"}
 	containerID := "1"