Skip to content
This repository has been archived by the owner on Dec 12, 2023. It is now read-only.

Commit

Permalink
refactor: use AutoRemove instead of explicit
Browse files Browse the repository at this point in the history
  • Loading branch information
@adriantpaez
adriantpaez committed Sep 28, 2023
1 parent bfc9355 commit 4ac9272
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 55 deletions.
15 changes: 0 additions & 15 deletions internal/docker/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,21 +456,6 @@ func (d *DockerManager) Run(image string, options RunOptions) (err error) {
return err
}

// Ensure the container is removed after use
defer func() {
log.Debugf("Removing container %s", createResponse.ID)
removeErr := d.dockerClient.ContainerRemove(context.Background(), createResponse.ID, types.ContainerRemoveOptions{})
if removeErr != nil {
// If the main function did not return an error, but the deferred function did,
// the deferred function's error is returned.
if err == nil {
err = removeErr
} else {
log.Errorf("Error removing container %s: %v", createResponse.ID, removeErr)
}
}
}()

if options.Network != "" && options.Network != NetworkHost {
log.Debugf("Connecting container %s to network %s", createResponse.ID, options.Network)
err = d.NetworkConnect(createResponse.ID, options.Network)
Expand Down
33 changes: 0 additions & 33 deletions internal/docker/docker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1460,7 +1460,6 @@ func TestDockerManager_Run(t *testing.T) {
dockerClient.EXPECT().ContainerWait(gomock.Any(), "containerID", gomock.Any()).Return(waitCh, errCh),
dockerClient.EXPECT().ContainerStart(gomock.Any(), "containerID", gomock.Any()).Return(nil),
dockerClient.EXPECT().ContainerLogs(gomock.Any(), "containerID", types.ContainerLogsOptions{ShowStdout: true, ShowStderr: true}).Return(io.NopCloser(bytes.NewBuffer([]byte{})), nil),
dockerClient.EXPECT().ContainerRemove(gomock.Any(), "containerID", gomock.Any()).Return(nil),
)
return dockerClient
},
Expand All @@ -1480,40 +1479,13 @@ func TestDockerManager_Run(t *testing.T) {
options: RunOptions{Network: "my-network", Args: []string{"arg1", "arg2"}},
expectedError: errors.New("creation error"),
},
{
name: "Container remove error",
setupMock: func(ctrl *gomock.Controller) *mocks.MockAPIClient {
dockerClient := mocks.NewMockAPIClient(ctrl)

// Create channels
waitCh := make(chan container.WaitResponse, 1)
errCh := make(chan error, 1)

// Write to one of the channels
waitCh <- container.WaitResponse{StatusCode: 0}

gomock.InOrder(
dockerClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(container.CreateResponse{ID: "containerID"}, nil),
dockerClient.EXPECT().NetworkConnect(gomock.Any(), "my-network", gomock.Any(), gomock.Any()).Return(nil),
dockerClient.EXPECT().ContainerWait(gomock.Any(), "containerID", gomock.Any()).Return(waitCh, errCh),
dockerClient.EXPECT().ContainerStart(gomock.Any(), "containerID", gomock.Any()).Return(nil),
dockerClient.EXPECT().ContainerLogs(gomock.Any(), "containerID", types.ContainerLogsOptions{ShowStdout: true, ShowStderr: true}).Return(io.NopCloser(bytes.NewBuffer([]byte{})), nil),
dockerClient.EXPECT().ContainerRemove(gomock.Any(), "containerID", gomock.Any()).Return(errors.New("remove error")),
)
return dockerClient
},
image: "my-image",
options: RunOptions{Network: "my-network", Args: []string{"arg1", "arg2"}},
expectedError: errors.New("remove error"),
},
{
name: "NetworkConnect error",
setupMock: func(ctrl *gomock.Controller) *mocks.MockAPIClient {
dockerClient := mocks.NewMockAPIClient(ctrl)
gomock.InOrder(
dockerClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(container.CreateResponse{ID: "containerID"}, nil),
dockerClient.EXPECT().NetworkConnect(gomock.Any(), "my-network", gomock.Any(), gomock.Any()).Return(errors.New("network connection error")),
dockerClient.EXPECT().ContainerRemove(gomock.Any(), "containerID", gomock.Any()).Return(nil),
)
return dockerClient
},
Expand All @@ -1535,7 +1507,6 @@ func TestDockerManager_Run(t *testing.T) {
dockerClient.EXPECT().NetworkConnect(gomock.Any(), "my-network", gomock.Any(), gomock.Any()).Return(nil),
dockerClient.EXPECT().ContainerWait(gomock.Any(), "containerID", gomock.Any()).Return(waitCh, errCh),
dockerClient.EXPECT().ContainerStart(gomock.Any(), "containerID", gomock.Any()).Return(errors.New("start container error")),
dockerClient.EXPECT().ContainerRemove(gomock.Any(), "containerID", gomock.Any()).Return(nil),
)
return dockerClient
},
Expand All @@ -1561,7 +1532,6 @@ func TestDockerManager_Run(t *testing.T) {
dockerClient.EXPECT().ContainerWait(gomock.Any(), "containerID", gomock.Any()).Return(waitCh, errCh),
dockerClient.EXPECT().ContainerStart(gomock.Any(), "containerID", gomock.Any()).Return(nil),
dockerClient.EXPECT().ContainerLogs(gomock.Any(), "containerID", types.ContainerLogsOptions{ShowStdout: true, ShowStderr: true}).Return(io.NopCloser(bytes.NewBufferString("container logs")), nil),
dockerClient.EXPECT().ContainerRemove(gomock.Any(), "containerID", gomock.Any()).Return(nil),
)
return dockerClient
},
Expand All @@ -1587,7 +1557,6 @@ func TestDockerManager_Run(t *testing.T) {
dockerClient.EXPECT().ContainerWait(gomock.Any(), "containerID", gomock.Any()).Return(waitCh, errCh),
dockerClient.EXPECT().ContainerStart(gomock.Any(), "containerID", gomock.Any()).Return(nil),
dockerClient.EXPECT().ContainerLogs(gomock.Any(), "containerID", types.ContainerLogsOptions{ShowStdout: true, ShowStderr: true}).Return(io.NopCloser(bytes.NewBufferString("container logs")), nil),
dockerClient.EXPECT().ContainerRemove(gomock.Any(), "containerID", gomock.Any()).Return(nil),
)
return dockerClient
},
Expand Down Expand Up @@ -1618,7 +1587,6 @@ func TestDockerManager_Run(t *testing.T) {
dockerClient.EXPECT().ContainerWait(context.Background(), "containerID", container.WaitConditionNextExit).Return(waitCh, errCh),
dockerClient.EXPECT().ContainerStart(context.Background(), "containerID", types.ContainerStartOptions{}).Return(nil),
dockerClient.EXPECT().ContainerLogs(context.Background(), "containerID", types.ContainerLogsOptions{ShowStdout: true, ShowStderr: true}).Return(io.NopCloser(bytes.NewBuffer([]byte{})), nil),
dockerClient.EXPECT().ContainerRemove(context.Background(), "containerID", types.ContainerRemoveOptions{}).Return(nil),
)
return dockerClient
},
Expand Down Expand Up @@ -1675,7 +1643,6 @@ func TestDockerManager_Run(t *testing.T) {
dockerClient.EXPECT().ContainerWait(context.Background(), "containerID", container.WaitConditionNextExit).Return(waitCh, errCh),
dockerClient.EXPECT().ContainerStart(context.Background(), "containerID", types.ContainerStartOptions{}).Return(nil),
dockerClient.EXPECT().ContainerLogs(context.Background(), "containerID", types.ContainerLogsOptions{ShowStdout: true, ShowStderr: true}).Return(io.NopCloser(bytes.NewBuffer([]byte{})), nil),
dockerClient.EXPECT().ContainerRemove(context.Background(), "containerID", types.ContainerRemoveOptions{}).Return(nil),
)
return dockerClient
},
Expand Down
7 changes: 4 additions & 3 deletions pkg/daemon/egn_daemon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1002,9 +1002,10 @@ func (d *EgnDaemon) RunPlugin(instanceId string, pluginArgs []string, options Ru
})
}
return d.docker.Run(instance.Plugin.Image, docker.RunOptions{
Network: network,
Args: pluginArgs,
Mounts: mounts,
AutoRemove: true,
Network: network,
Args: pluginArgs,
Mounts: mounts,
})
}

Expand Down
10 changes: 6 additions & 4 deletions pkg/daemon/egn_daemon_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2892,8 +2892,9 @@ func TestRunPlugin(t *testing.T) {
}, nil),
d.dockerManager.EXPECT().ContainerNetworks("abc123").Return([]string{"network-el"}, nil),
d.dockerManager.EXPECT().Run(common.PluginImage.FullImage(), docker.RunOptions{
Network: "network-el",
Args: []string{"arg1", "arg2"},
AutoRemove: true,
Network: "network-el",
Args: []string{"arg1", "arg2"},
Mounts: []docker.Mount{
{
Type: docker.VolumeTypeBind,
Expand Down Expand Up @@ -2938,8 +2939,9 @@ func TestRunPlugin(t *testing.T) {
gomock.InOrder(
d.locker.EXPECT().New(filepath.Join(d.dataDir.Path(), "nodes", "mock-avs-default", ".lock")).Return(d.locker),
d.dockerManager.EXPECT().Run(common.PluginImage.FullImage(), docker.RunOptions{
Network: docker.NetworkHost,
Args: []string{"arg1", "arg2"},
AutoRemove: true,
Network: docker.NetworkHost,
Args: []string{"arg1", "arg2"},
Mounts: []docker.Mount{
{
Type: docker.VolumeTypeBind,
Expand Down

0 comments on commit 4ac9272

Please sign in to comment.