From 378d02daddb2a422a58747aa36671fe80d74e0d4 Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Thu, 25 Mar 2021 02:52:29 -0300 Subject: [PATCH] Refactor `up --build` This restarts containers which had it's images rebuilt Signed-off-by: Ulysses Souza --- cli/cmd/compose/up.go | 2 +- local/compose/build.go | 24 +++++++++++++++--- local/compose/convergence.go | 4 +-- local/compose/create.go | 18 +++++++------- local/compose/down.go | 6 +++++ local/compose/down_test.go | 47 ++++++++++++++++++------------------ local/compose/run.go | 2 +- local/compose/status.go | 2 +- 8 files changed, 63 insertions(+), 42 deletions(-) diff --git a/cli/cmd/compose/up.go b/cli/cmd/compose/up.go index 25f427808..5424d18a7 100644 --- a/cli/cmd/compose/up.go +++ b/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 diff --git a/local/compose/build.go b/local/compose/build.go index 058ed42b7..6d2714955 100644 --- a/local/compose/build.go +++ b/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 } diff --git a/local/compose/convergence.go b/local/compose/convergence.go index ecef62099..37f6baf11 100644 --- a/local/compose/convergence.go +++ b/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{}) diff --git a/local/compose/create.go b/local/compose/create.go index 30a708fb6..1225ae925 100644 --- a/local/compose/create.go +++ b/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 { diff --git a/local/compose/down.go b/local/compose/down.go index 380fd7abb..ac0efa03e 100644 --- a/local/compose/down.go +++ b/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 }) diff --git a/local/compose/down_test.go b/local/compose/down_test.go index 7182a9fff..00282b023 100644 --- a/local/compose/down_test.go +++ b/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(ctx, projectFilterListOpt()).Return( - []apitypes.Container{testContainer("service1", "123"), testContainer("service1", "456"), testContainer("service2", "789"), testContainer("service_orphan", "321")}, nil) + api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt()).Return( + []apitypes.Container{testContainer("service1", "123"), testContainer("service2", "456"), testContainer("service2", "789"), testContainer("service_orphan", "321")}, nil) - api.EXPECT().ContainerStop(ctx, "123", nil).Return(nil) - api.EXPECT().ContainerStop(ctx, "456", nil).Return(nil) - api.EXPECT().ContainerStop(ctx, "789", nil).Return(nil) + api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil) + api.EXPECT().ContainerStop(gomock.Any(), "456", 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(ctx, "456", apitypes.ContainerRemoveOptions{Force: true}).Return(nil) - api.EXPECT().ContainerRemove(ctx, "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil) + api.EXPECT().ContainerRemove(gomock.Any(), "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil) + api.EXPECT().ContainerRemove(gomock.Any(), "456", 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(ctx, projectFilterListOpt()).Return( + api.EXPECT().ContainerList(gomock.Any(), 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(ctx, "789", nil).Return(nil) - api.EXPECT().ContainerStop(ctx, "321", nil).Return(nil) + api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil) + api.EXPECT().ContainerStop(gomock.Any(), "789", 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(ctx, "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil) - api.EXPECT().ContainerRemove(ctx, "321", apitypes.ContainerRemoveOptions{Force: true}).Return(nil) + api.EXPECT().ContainerRemove(gomock.Any(), "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil) + api.EXPECT().ContainerRemove(gomock.Any(), "789", 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) } diff --git a/local/compose/run.go b/local/compose/run.go index 5f6b13e5d..cc7a7b104 100644 --- a/local/compose/run.go +++ b/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 { diff --git a/local/compose/status.go b/local/compose/status.go index db95a0f08..32af151c2 100644 --- a/local/compose/status.go +++ b/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 }