Merge pull request #1464 from ulyssessouza/up-build-restart

Refactor `up --build` behavior

cli/cmd/compose/up.go

@@ -29,7 +29,6 @@ import (
 
 	"github.com/compose-spec/compose-go/types"
 	"github.com/docker/cli/cli"
-	"github.com/docker/compose-cli/utils"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"golang.org/x/sync/errgroup"
@@ -39,6 +38,7 @@ import (
 	"github.com/docker/compose-cli/api/context/store"
 	"github.com/docker/compose-cli/api/progress"
 	"github.com/docker/compose-cli/cli/formatter"
+	"github.com/docker/compose-cli/utils"
 )
 
 // composeOptions hold options common to `up` and `run` to run compose project

local/compose/build.go

@@ -24,7 +24,10 @@ import (
 	"path"
 	"strings"
 
+	moby "github.com/docker/docker/api/types"
+
 	"github.com/docker/compose-cli/api/compose"
+	composeprogress "github.com/docker/compose-cli/api/progress"
 	"github.com/docker/compose-cli/utils"
 
 	"github.com/compose-spec/compose-go/types"
@@ -68,7 +71,7 @@ func (s *composeService) Build(ctx context.Context, project *types.Project, opti
 		}
 	}
 
-	err := s.build(ctx, project, opts, options.Progress)
+	err := s.build(ctx, project, opts, Containers{}, options.Progress)
 	if err == nil {
 		if len(imagesToBuild) > 0 {
 			utils.DisplayScanSuggestMsg()
@@ -78,7 +81,7 @@ func (s *composeService) Build(ctx context.Context, project *types.Project, opti
 	return err
 }
 
-func (s *composeService) ensureImagesExists(ctx context.Context, project *types.Project, quietPull bool) error {
+func (s *composeService) ensureImagesExists(ctx context.Context, project *types.Project, observedState Containers, quietPull bool) error {
 	opts := map[string]build.Options{}
 	imagesToBuild := []string{}
 	for _, service := range project.Services {
@@ -128,7 +131,7 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types.
 		mode = progress.PrinterModeQuiet
 	}
 
-	err := s.build(ctx, project, opts, mode)
+	err := s.build(ctx, project, opts, observedState, mode)
 	if err == nil {
 		if len(imagesToBuild) > 0 {
 			utils.DisplayScanSuggestMsg()
@@ -148,7 +151,7 @@ func (s *composeService) localImagePresent(ctx context.Context, imageName string
 	return true, nil
 }
 
-func (s *composeService) build(ctx context.Context, project *types.Project, opts map[string]build.Options, mode string) error {
+func (s *composeService) build(ctx context.Context, project *types.Project, opts map[string]build.Options, observedState Containers, mode string) error {
 	info, err := s.apiClient.Info(ctx)
 	if err != nil {
 		return err
@@ -187,6 +190,19 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opts
 	if err == nil {
 		err = errW
 	}
+
+	cw := composeprogress.ContextWriter(ctx)
+	for _, c := range observedState {
+		for imageName := range opts {
+			if c.Image == imageName {
+				err = s.removeContainers(ctx, cw, []moby.Container{c}, nil)
+				if err != nil {
+					return err
+				}
+			}
+		}
+	}
+
 	return err
 }
 

local/compose/convergence.go

@@ -125,7 +125,7 @@ func (s *composeService) ensureService(ctx context.Context, project *types.Proje
 			w.Event(progress.CreatedEvent(name))
 		default:
 			eg.Go(func() error {
-				return s.restartContainer(ctx, container)
+				return s.startContainer(ctx, container)
 			})
 		}
 	}
@@ -269,7 +269,7 @@ func setDependentLifecycle(project *types.Project, service string, strategy stri
 	}
 }
 
-func (s *composeService) restartContainer(ctx context.Context, container moby.Container) error {
+func (s *composeService) startContainer(ctx context.Context, container moby.Container) error {
 	w := progress.ContextWriter(ctx)
 	w.Event(progress.NewEvent(getContainerProgressName(container), progress.Working, "Restart"))
 	err := s.apiClient.ContainerStart(ctx, container.ID, moby.ContainerStartOptions{})

local/compose/create.go

@@ -47,7 +47,15 @@ func (s *composeService) Create(ctx context.Context, project *types.Project, opt
 		opts.Services = project.ServiceNames()
 	}
 
-	err := s.ensureImagesExists(ctx, project, opts.QuietPull)
+	var observedState Containers
+	observedState, err := s.getContainers(ctx, project.Name, oneOffInclude, true)
+	if err != nil {
+		return err
+	}
+	containerState := NewContainersState(observedState)
+	ctx = context.WithValue(ctx, ContainersKey{}, containerState)
+
+	err = s.ensureImagesExists(ctx, project, observedState, opts.QuietPull)
 	if err != nil {
 		return err
 	}
@@ -67,14 +75,6 @@ func (s *composeService) Create(ctx context.Context, project *types.Project, opt
 		return err
 	}
 
-	var observedState Containers
-	observedState, err = s.getContainers(ctx, project.Name, oneOffInclude, true)
-	if err != nil {
-		return err
-	}
-	containerState := NewContainersState(observedState)
-	ctx = context.WithValue(ctx, ContainersKey{}, containerState)
-
 	allServices := project.AllServices()
 	allServiceNames := []string{}
 	for _, service := range allServices {

local/compose/down.go

@@ -43,6 +43,7 @@ func (s *composeService) Down(ctx context.Context, projectName string, options c
 	if err != nil {
 		return err
 	}
+	ctx = context.WithValue(ctx, ContainersKey{}, NewContainersState(containers))
 
 	if options.Project == nil {
 		project, err := s.projectFromContainerLabels(containers, projectName)
@@ -167,6 +168,11 @@ func (s *composeService) removeContainers(ctx context.Context, w progress.Writer
 				w.Event(progress.ErrorMessageEvent(eventName, "Error while Removing"))
 				return err
 			}
+			contextContainerState, err := GetContextContainerState(ctx)
+			if err != nil {
+				return err
+			}
+			contextContainerState.Remove(toDelete.ID)
 			w.Event(progress.RemovedEvent(eventName))
 			return nil
 		})

local/compose/down_test.go

@@ -20,11 +20,12 @@ import (
 	"context"
 	"testing"
 
+	"github.com/docker/docker/api/types/filters"
+
 	"github.com/docker/compose-cli/api/compose"
 	"github.com/docker/compose-cli/local/mocks"
 
 	apitypes "github.com/docker/docker/api/types"
-	"github.com/docker/docker/api/types/filters"
 	"github.com/golang/mock/gomock"
 	"gotest.tools/v3/assert"
 )
@@ -35,23 +36,22 @@ func TestDown(t *testing.T) {
 	api := mocks.NewMockAPIClient(mockCtrl)
 	tested.apiClient = api
 
-	ctx := context.Background()
+	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt()).Return(
-	api.EXPECT().ContainerList(ctx, projectFilterListOpt()).Return(
+		[]apitypes.Container{testContainer("service1", "123"), testContainer("service2", "456"), testContainer("service2", "789"), testContainer("service_orphan", "321")}, nil)
-		[]apitypes.Container{testContainer("service1", "123"), testContainer("service1", "456"), testContainer("service2", "789"), testContainer("service_orphan", "321")}, nil)
 
-	api.EXPECT().ContainerStop(ctx, "123", nil).Return(nil)
+	api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil)
-	api.EXPECT().ContainerStop(ctx, "456", nil).Return(nil)
+	api.EXPECT().ContainerStop(gomock.Any(), "456", nil).Return(nil)
-	api.EXPECT().ContainerStop(ctx, "789", nil).Return(nil)
+	api.EXPECT().ContainerStop(gomock.Any(), "789", nil).Return(nil)
 
-	api.EXPECT().ContainerRemove(ctx, "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
+	api.EXPECT().ContainerRemove(gomock.Any(), "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
-	api.EXPECT().ContainerRemove(ctx, "456", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
+	api.EXPECT().ContainerRemove(gomock.Any(), "456", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
-	api.EXPECT().ContainerRemove(ctx, "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
+	api.EXPECT().ContainerRemove(gomock.Any(), "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
 
-	api.EXPECT().NetworkList(ctx, apitypes.NetworkListOptions{Filters: filters.NewArgs(projectFilter(testProject))}).Return([]apitypes.NetworkResource{{ID: "myProject_default"}}, nil)
+	api.EXPECT().NetworkList(gomock.Any(), apitypes.NetworkListOptions{Filters: filters.NewArgs(projectFilter(testProject))}).Return([]apitypes.NetworkResource{{ID: "myProject_default"}}, nil)
 
-	api.EXPECT().NetworkRemove(ctx, "myProject_default").Return(nil)
+	api.EXPECT().NetworkRemove(gomock.Any(), "myProject_default").Return(nil)
 
-	err := tested.Down(ctx, testProject, compose.DownOptions{})
+	err := tested.Down(context.Background(), testProject, compose.DownOptions{})
 	assert.NilError(t, err)
 }
 
@@ -61,22 +61,21 @@ func TestDownRemoveOrphans(t *testing.T) {
 	api := mocks.NewMockAPIClient(mockCtrl)
 	tested.apiClient = api
 
-	ctx := context.Background()
+	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt()).Return(
-	api.EXPECT().ContainerList(ctx, projectFilterListOpt()).Return(
 		[]apitypes.Container{testContainer("service1", "123"), testContainer("service2", "789"), testContainer("service_orphan", "321")}, nil)
 
-	api.EXPECT().ContainerStop(ctx, "123", nil).Return(nil)
+	api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil)
-	api.EXPECT().ContainerStop(ctx, "789", nil).Return(nil)
+	api.EXPECT().ContainerStop(gomock.Any(), "789", nil).Return(nil)
-	api.EXPECT().ContainerStop(ctx, "321", nil).Return(nil)
+	api.EXPECT().ContainerStop(gomock.Any(), "321", nil).Return(nil)
 
-	api.EXPECT().ContainerRemove(ctx, "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
+	api.EXPECT().ContainerRemove(gomock.Any(), "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
-	api.EXPECT().ContainerRemove(ctx, "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
+	api.EXPECT().ContainerRemove(gomock.Any(), "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
-	api.EXPECT().ContainerRemove(ctx, "321", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
+	api.EXPECT().ContainerRemove(gomock.Any(), "321", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
 
-	api.EXPECT().NetworkList(ctx, apitypes.NetworkListOptions{Filters: filters.NewArgs(projectFilter(testProject))}).Return([]apitypes.NetworkResource{{ID: "myProject_default"}}, nil)
+	api.EXPECT().NetworkList(gomock.Any(), apitypes.NetworkListOptions{Filters: filters.NewArgs(projectFilter(testProject))}).Return([]apitypes.NetworkResource{{ID: "myProject_default"}}, nil)
 
-	api.EXPECT().NetworkRemove(ctx, "myProject_default").Return(nil)
+	api.EXPECT().NetworkRemove(gomock.Any(), "myProject_default").Return(nil)
 
-	err := tested.Down(ctx, testProject, compose.DownOptions{RemoveOrphans: true})
+	err := tested.Down(context.Background(), testProject, compose.DownOptions{RemoveOrphans: true})
 	assert.NilError(t, err)
 }

local/compose/run.go

@@ -53,7 +53,7 @@ func (s *composeService) RunOneOffContainer(ctx context.Context, project *types.
 	service.Labels = service.Labels.Add(slugLabel, slug)
 	service.Labels = service.Labels.Add(oneoffLabel, "True")
 
-	if err := s.ensureImagesExists(ctx, project, false); err != nil { // all dependencies already checked, but might miss service img
+	if err := s.ensureImagesExists(ctx, project, observedState, false); err != nil { // all dependencies already checked, but might miss service img
 		return 0, err
 	}
 	if err := s.waitDependencies(ctx, project, service); err != nil {

local/compose/status.go

@@ -66,7 +66,7 @@ func (s *containersState) Remove(id string) types.Container {
 	var c types.Container
 	var newObserved Containers
 	for _, o := range *s.observedContainers {
-		if o.ID != id {
+		if o.ID == id {
 			c = o
 			continue
 		}