Skip to content
This repository was archived by the owner on Nov 27, 2023. It is now read-only.

Commit 76cd14a

Browse files
gtardifgtardif
authored
Merge pull request #1464 from ulyssessouza/up-build-restart
Refactor `up --build` behavior
2 parents 6aed81e + 378d02d commit 76cd14a

File tree

8 files changed

+63
-42
lines changed

8 files changed

+63
-42
lines changed

‎cli/cmd/compose/up.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929

3030
"github.com/compose-spec/compose-go/types"
3131
"github.com/docker/cli/cli"
32-
"github.com/docker/compose-cli/utils"
3332
"github.com/sirupsen/logrus"
3433
"github.com/spf13/cobra"
3534
"golang.org/x/sync/errgroup"
@@ -39,6 +38,7 @@ import (
3938
"github.com/docker/compose-cli/api/context/store"
4039
"github.com/docker/compose-cli/api/progress"
4140
"github.com/docker/compose-cli/cli/formatter"
41+
"github.com/docker/compose-cli/utils"
4242
)
4343

4444
// composeOptions hold options common to `up` and `run` to run compose project

‎local/compose/build.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ import (
2424
"path"
2525
"strings"
2626

27+
moby "github.com/docker/docker/api/types"
28+
2729
"github.com/docker/compose-cli/api/compose"
30+
composeprogress "github.com/docker/compose-cli/api/progress"
2831
"github.com/docker/compose-cli/utils"
2932

3033
"github.com/compose-spec/compose-go/types"
@@ -68,7 +71,7 @@ func (s *composeService) Build(ctx context.Context, project *types.Project, opti
6871
}
6972
}
7073

71-
err := s.build(ctx, project, opts, options.Progress)
74+
err := s.build(ctx, project, opts, Containers{}, options.Progress)
7275
if err == nil {
7376
if len(imagesToBuild) > 0 {
7477
utils.DisplayScanSuggestMsg()
@@ -78,7 +81,7 @@ func (s *composeService) Build(ctx context.Context, project *types.Project, opti
7881
return err
7982
}
8083

81-
func (s *composeService) ensureImagesExists(ctx context.Context, project *types.Project, quietPull bool) error {
84+
func (s *composeService) ensureImagesExists(ctx context.Context, project *types.Project, observedState Containers, quietPull bool) error {
8285
opts := map[string]build.Options{}
8386
imagesToBuild := []string{}
8487
for _, service := range project.Services {
@@ -128,7 +131,7 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types.
128131
mode = progress.PrinterModeQuiet
129132
}
130133

131-
err := s.build(ctx, project, opts, mode)
134+
err := s.build(ctx, project, opts, observedState, mode)
132135
if err == nil {
133136
if len(imagesToBuild) > 0 {
134137
utils.DisplayScanSuggestMsg()
@@ -148,7 +151,7 @@ func (s *composeService) localImagePresent(ctx context.Context, imageName string
148151
return true, nil
149152
}
150153

151-
func (s *composeService) build(ctx context.Context, project *types.Project, opts map[string]build.Options, mode string) error {
154+
func (s *composeService) build(ctx context.Context, project *types.Project, opts map[string]build.Options, observedState Containers, mode string) error {
152155
info, err := s.apiClient.Info(ctx)
153156
if err != nil {
154157
return err
@@ -187,6 +190,19 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opts
187190
if err == nil {
188191
err = errW
189192
}
193+
194+
cw := composeprogress.ContextWriter(ctx)
195+
for _, c := range observedState {
196+
for imageName := range opts {
197+
if c.Image == imageName {
198+
err = s.removeContainers(ctx, cw, []moby.Container{c}, nil)
199+
if err != nil {
200+
return err
201+
}
202+
}
203+
}
204+
}
205+
190206
return err
191207
}
192208

‎local/compose/convergence.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func (s *composeService) ensureService(ctx context.Context, project *types.Proje
125125
w.Event(progress.CreatedEvent(name))
126126
default:
127127
eg.Go(func() error {
128-
return s.restartContainer(ctx, container)
128+
return s.startContainer(ctx, container)
129129
})
130130
}
131131
}
@@ -269,7 +269,7 @@ func setDependentLifecycle(project *types.Project, service string, strategy stri
269269
}
270270
}
271271

272-
func (s *composeService) restartContainer(ctx context.Context, container moby.Container) error {
272+
func (s *composeService) startContainer(ctx context.Context, container moby.Container) error {
273273
w := progress.ContextWriter(ctx)
274274
w.Event(progress.NewEvent(getContainerProgressName(container), progress.Working, "Restart"))
275275
err := s.apiClient.ContainerStart(ctx, container.ID, moby.ContainerStartOptions{})

‎local/compose/create.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,15 @@ func (s *composeService) Create(ctx context.Context, project *types.Project, opt
4747
opts.Services = project.ServiceNames()
4848
}
4949

50-
err := s.ensureImagesExists(ctx, project, opts.QuietPull)
50+
var observedState Containers
51+
observedState, err := s.getContainers(ctx, project.Name, oneOffInclude, true)
52+
if err != nil {
53+
return err
54+
}
55+
containerState := NewContainersState(observedState)
56+
ctx = context.WithValue(ctx, ContainersKey{}, containerState)
57+
58+
err = s.ensureImagesExists(ctx, project, observedState, opts.QuietPull)
5159
if err != nil {
5260
return err
5361
}
@@ -67,14 +75,6 @@ func (s *composeService) Create(ctx context.Context, project *types.Project, opt
6775
return err
6876
}
6977

70-
var observedState Containers
71-
observedState, err = s.getContainers(ctx, project.Name, oneOffInclude, true)
72-
if err != nil {
73-
return err
74-
}
75-
containerState := NewContainersState(observedState)
76-
ctx = context.WithValue(ctx, ContainersKey{}, containerState)
77-
7878
allServices := project.AllServices()
7979
allServiceNames := []string{}
8080
for _, service := range allServices {

‎local/compose/down.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ func (s *composeService) Down(ctx context.Context, projectName string, options c
4343
if err != nil {
4444
return err
4545
}
46+
ctx = context.WithValue(ctx, ContainersKey{}, NewContainersState(containers))
4647

4748
if options.Project == nil {
4849
project, err := s.projectFromContainerLabels(containers, projectName)
@@ -167,6 +168,11 @@ func (s *composeService) removeContainers(ctx context.Context, w progress.Writer
167168
w.Event(progress.ErrorMessageEvent(eventName, "Error while Removing"))
168169
return err
169170
}
171+
contextContainerState, err := GetContextContainerState(ctx)
172+
if err != nil {
173+
return err
174+
}
175+
contextContainerState.Remove(toDelete.ID)
170176
w.Event(progress.RemovedEvent(eventName))
171177
return nil
172178
})

‎local/compose/down_test.go

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@ import (
2020
"context"
2121
"testing"
2222

23+
"github.com/docker/docker/api/types/filters"
24+
2325
"github.com/docker/compose-cli/api/compose"
2426
"github.com/docker/compose-cli/local/mocks"
2527

2628
apitypes "github.com/docker/docker/api/types"
27-
"github.com/docker/docker/api/types/filters"
2829
"github.com/golang/mock/gomock"
2930
"gotest.tools/v3/assert"
3031
)
@@ -35,23 +36,22 @@ func TestDown(t *testing.T) {
3536
api := mocks.NewMockAPIClient(mockCtrl)
3637
tested.apiClient = api
3738

38-
ctx := context.Background()
39-
api.EXPECT().ContainerList(ctx, projectFilterListOpt()).Return(
40-
[]apitypes.Container{testContainer("service1", "123"), testContainer("service1", "456"), testContainer("service2", "789"), testContainer("service_orphan", "321")}, nil)
39+
api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt()).Return(
40+
[]apitypes.Container{testContainer("service1", "123"), testContainer("service2", "456"), testContainer("service2", "789"), testContainer("service_orphan", "321")}, nil)
4141

42-
api.EXPECT().ContainerStop(ctx, "123", nil).Return(nil)
43-
api.EXPECT().ContainerStop(ctx, "456", nil).Return(nil)
44-
api.EXPECT().ContainerStop(ctx, "789", nil).Return(nil)
42+
api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil)
43+
api.EXPECT().ContainerStop(gomock.Any(), "456", nil).Return(nil)
44+
api.EXPECT().ContainerStop(gomock.Any(), "789", nil).Return(nil)
4545

46-
api.EXPECT().ContainerRemove(ctx, "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
47-
api.EXPECT().ContainerRemove(ctx, "456", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
48-
api.EXPECT().ContainerRemove(ctx, "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
46+
api.EXPECT().ContainerRemove(gomock.Any(), "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
47+
api.EXPECT().ContainerRemove(gomock.Any(), "456", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
48+
api.EXPECT().ContainerRemove(gomock.Any(), "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
4949

50-
api.EXPECT().NetworkList(ctx, apitypes.NetworkListOptions{Filters: filters.NewArgs(projectFilter(testProject))}).Return([]apitypes.NetworkResource{{ID: "myProject_default"}}, nil)
50+
api.EXPECT().NetworkList(gomock.Any(), apitypes.NetworkListOptions{Filters: filters.NewArgs(projectFilter(testProject))}).Return([]apitypes.NetworkResource{{ID: "myProject_default"}}, nil)
5151

52-
api.EXPECT().NetworkRemove(ctx, "myProject_default").Return(nil)
52+
api.EXPECT().NetworkRemove(gomock.Any(), "myProject_default").Return(nil)
5353

54-
err := tested.Down(ctx, testProject, compose.DownOptions{})
54+
err := tested.Down(context.Background(), testProject, compose.DownOptions{})
5555
assert.NilError(t, err)
5656
}
5757

@@ -61,22 +61,21 @@ func TestDownRemoveOrphans(t *testing.T) {
6161
api := mocks.NewMockAPIClient(mockCtrl)
6262
tested.apiClient = api
6363

64-
ctx := context.Background()
65-
api.EXPECT().ContainerList(ctx, projectFilterListOpt()).Return(
64+
api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt()).Return(
6665
[]apitypes.Container{testContainer("service1", "123"), testContainer("service2", "789"), testContainer("service_orphan", "321")}, nil)
6766

68-
api.EXPECT().ContainerStop(ctx, "123", nil).Return(nil)
69-
api.EXPECT().ContainerStop(ctx, "789", nil).Return(nil)
70-
api.EXPECT().ContainerStop(ctx, "321", nil).Return(nil)
67+
api.EXPECT().ContainerStop(gomock.Any(), "123", nil).Return(nil)
68+
api.EXPECT().ContainerStop(gomock.Any(), "789", nil).Return(nil)
69+
api.EXPECT().ContainerStop(gomock.Any(), "321", nil).Return(nil)
7170

72-
api.EXPECT().ContainerRemove(ctx, "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
73-
api.EXPECT().ContainerRemove(ctx, "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
74-
api.EXPECT().ContainerRemove(ctx, "321", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
71+
api.EXPECT().ContainerRemove(gomock.Any(), "123", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
72+
api.EXPECT().ContainerRemove(gomock.Any(), "789", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
73+
api.EXPECT().ContainerRemove(gomock.Any(), "321", apitypes.ContainerRemoveOptions{Force: true}).Return(nil)
7574

76-
api.EXPECT().NetworkList(ctx, apitypes.NetworkListOptions{Filters: filters.NewArgs(projectFilter(testProject))}).Return([]apitypes.NetworkResource{{ID: "myProject_default"}}, nil)
75+
api.EXPECT().NetworkList(gomock.Any(), apitypes.NetworkListOptions{Filters: filters.NewArgs(projectFilter(testProject))}).Return([]apitypes.NetworkResource{{ID: "myProject_default"}}, nil)
7776

78-
api.EXPECT().NetworkRemove(ctx, "myProject_default").Return(nil)
77+
api.EXPECT().NetworkRemove(gomock.Any(), "myProject_default").Return(nil)
7978

80-
err := tested.Down(ctx, testProject, compose.DownOptions{RemoveOrphans: true})
79+
err := tested.Down(context.Background(), testProject, compose.DownOptions{RemoveOrphans: true})
8180
assert.NilError(t, err)
8281
}

‎local/compose/run.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func (s *composeService) RunOneOffContainer(ctx context.Context, project *types.
5353
service.Labels = service.Labels.Add(slugLabel, slug)
5454
service.Labels = service.Labels.Add(oneoffLabel, "True")
5555

56-
if err := s.ensureImagesExists(ctx, project, false); err != nil { // all dependencies already checked, but might miss service img
56+
if err := s.ensureImagesExists(ctx, project, observedState, false); err != nil { // all dependencies already checked, but might miss service img
5757
return 0, err
5858
}
5959
if err := s.waitDependencies(ctx, project, service); err != nil {

‎local/compose/status.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ func (s *containersState) Remove(id string) types.Container {
6666
var c types.Container
6767
var newObserved Containers
6868
for _, o := range *s.observedContainers {
69-
if o.ID != id {
69+
if o.ID == id {
7070
c = o
7171
continue
7272
}

0 commit comments

Comments
 (0)