From 002c005821a04cd04d0ada497ab0435b34be8ac7 Mon Sep 17 00:00:00 2001 From: AdithyaKolla Date: Thu, 16 Nov 2023 11:42:48 -0800 Subject: [PATCH 1/7] add container state methods --- .../pkg/docker/dockerengine/dockerengine.go | 136 +++++++- .../docker/dockerengine/dockerengine_test.go | 295 +++++++++++++++++- 2 files changed, 407 insertions(+), 24 deletions(-) diff --git a/internal/pkg/docker/dockerengine/dockerengine.go b/internal/pkg/docker/dockerengine/dockerengine.go index 4e5f0268e91..a8d493d072a 100644 --- a/internal/pkg/docker/dockerengine/dockerengine.go +++ b/internal/pkg/docker/dockerengine/dockerengine.go @@ -44,6 +44,20 @@ const ( credStoreECRLogin = "ecr-login" // set on `credStore` attribute in docker configuration file ) +// Health states of a Container. +const ( + noHealthcheck = "none" // Indicates there is no healthcheck + starting = "starting" // Starting indicates that the container is not yet ready + healthy = "healthy" // Healthy indicates that the container is running correctly + unhealthy = "unhealthy" // Unhealthy indicates that the container has a problem +) + +// State of a docker container. +const ( + containerStatusRunning = "running" + containerStatusExited = "exited" +) + // DockerCmdClient represents the docker client to interact with the server via external commands. type DockerCmdClient struct { runner Cmd @@ -339,13 +353,113 @@ func (c DockerCmdClient) Run(ctx context.Context, options *RunOptions) error { // IsContainerRunning checks if a specific Docker container is running. func (c DockerCmdClient) IsContainerRunning(ctx context.Context, name string) (bool, error) { + state, err := c.containerState(ctx, name) + if err != nil { + return false, err + } + switch state.Status { + case containerStatusRunning: + return true, nil + case containerStatusExited: + return false, &ErrContainerExited{name: name, exitcode: state.ExitCode} + } + return false, nil +} + +// IsContainerComplete returns true if a docker container exits with an exitcode. +func (c DockerCmdClient) IsContainerComplete(ctx context.Context, containerName string) (bool, error) { + state, err := c.containerState(ctx, containerName) + if err != nil { + return false, err + } + return state.Status == containerStatusExited, nil +} + +// IsContainerSuccess returns true if a docker container exits with exitcode 0. +func (c DockerCmdClient) IsContainerSuccess(ctx context.Context, containerName string) (bool, error) { + state, err := c.containerState(ctx, containerName) + if err != nil { + return false, err + } + return state.Status == containerStatusExited && state.ExitCode == 0, nil +} + +// IsContainerHealthy returns true if a container health state is healthy. +func (c DockerCmdClient) IsContainerHealthy(ctx context.Context, containerName string) (bool, error) { + state, err := c.containerState(ctx, containerName) + if err != nil { + return false, err + } + if state.Status == containerStatusExited { + return false, &ErrContainerExited{name: containerName, exitcode: state.ExitCode} + } + if state.Health == nil { + return false, fmt.Errorf("healthcheck is not configured for container %s", containerName) + } + switch state.Health.Status { + case healthy: + return true, nil + case starting: + return false, nil + case unhealthy: + return false, fmt.Errorf("container %q is %q", containerName, unhealthy) + case noHealthcheck: + return false, fmt.Errorf("healthcheck configuration is set to %q for container %s", unhealthy, containerName) + default: + return false, fmt.Errorf("container %s had unexpected health status %q", containerName, state.Health.Status) + } +} + +// ContainerState holds the status, exit code, and health information of a Docker container. +type ContainerState struct { + Status string `json:"Status"` + ExitCode int `json:"ExitCode"` + Health *struct { + Status string `json:"Status"` + } +} + +// containerState retrieves the current state of a specified Docker container. +// It returns a ContainerState object and any error encountered during retrieval. +func (d *DockerCmdClient) containerState(ctx context.Context, containerName string) (ContainerState, error) { + containerID, err := d.containerID(ctx, containerName) + if err != nil { + return ContainerState{}, err + } + if containerID == "" { + return ContainerState{}, nil + } + // return ContainerState{}, nil buf := &bytes.Buffer{} - if err := c.runner.RunWithContext(ctx, "docker", []string{"ps", "-q", "--filter", "name=" + name}, exec.Stdout(buf)); err != nil { - return false, fmt.Errorf("run docker ps: %w", err) + if err := d.runner.RunWithContext(ctx, "docker", []string{"inspect", "--format", "{{json .State}}", containerID}, exec.Stdout(buf)); err != nil { + return ContainerState{}, fmt.Errorf("run docker inspect: %w", err) + } + var containerState ContainerState + if err := json.Unmarshal([]byte(strings.TrimSpace(buf.String())), &containerState); err != nil { + return ContainerState{}, fmt.Errorf("unmarshal state of container %q:%w", containerName, err) } + return containerState, nil +} + +// containerID gets the ID of a Docker container by its name. +func (d *DockerCmdClient) containerID(ctx context.Context, containerName string) (string, error) { + buf := &bytes.Buffer{} + if err := d.runner.RunWithContext(ctx, "docker", []string{"ps", "-a", "-q", "--filter", "name=" + containerName}, exec.Stdout(buf)); err != nil { + return "", fmt.Errorf("run docker ps: %w", err) + } + return strings.TrimSpace(buf.String()), nil +} + +// ErrContainerExited represents an error when a Docker container has exited. +// It includes the container name and exit code in the error message. +type ErrContainerExited struct { + name string + exitcode int +} - output := strings.TrimSpace(buf.String()) - return output != "", nil +// ErrContainerExited represents docker container exited with an exitcode. +func (e *ErrContainerExited) Error() string { + return fmt.Sprintf("container %q exited with code %d", e.name, e.exitcode) } // Stop calls `docker stop` to stop a running container. @@ -459,13 +573,13 @@ func PlatformString(os, arch string) string { func parseCredFromDockerConfig(config []byte) (*dockerConfig, error) { /* - Sample docker config file - { - "credsStore" : "ecr-login", - "credHelpers": { - "dummyaccountId.dkr.ecr.region.amazonaws.com": "ecr-login" - } - } + Sample docker config file + { + "credsStore" : "ecr-login", + "credHelpers": { + "dummyaccountId.dkr.ecr.region.amazonaws.com": "ecr-login" + } + } */ cred := dockerConfig{} err := json.Unmarshal(config, &cred) diff --git a/internal/pkg/docker/dockerengine/dockerengine_test.go b/internal/pkg/docker/dockerengine/dockerengine_test.go index ec01f8f6751..6803c847098 100644 --- a/internal/pkg/docker/dockerengine/dockerengine_test.go +++ b/internal/pkg/docker/dockerengine/dockerengine_test.go @@ -805,37 +805,81 @@ func TestDockerCommand_IsContainerRunning(t *testing.T) { mockError := errors.New("some error") mockContainerName := "mockContainer" mockUnknownContainerName := "mockUnknownContainer" - var mockCmd *MockCmd tests := map[string]struct { - setupMocks func(controller *gomock.Controller) + setupMocks func(controller *gomock.Controller) *MockCmd inContainerName string - - wantedErr error + wantRunning bool + wantedErr error }{ "error running docker info": { inContainerName: mockUnknownContainerName, - setupMocks: func(controller *gomock.Controller) { - mockCmd = NewMockCmd(controller) - mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-q", "--filter", "name=mockUnknownContainer"}, gomock.Any()).Return(mockError) + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockUnknownContainer"}, gomock.Any()).Return(mockError) + return mockCmd }, - wantedErr: fmt.Errorf("run docker ps: some error"), }, "successfully check if the container is running": { inContainerName: mockContainerName, - setupMocks: func(controller *gomock.Controller) { - mockCmd = NewMockCmd(controller) - mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-q", "--filter", "name=mockContainer"}, gomock.Any()).Return(nil) + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockContainer"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte("53d6417769ed")) + return nil + }) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"inspect", "--format", "{{json .State}}", "53d6417769ed"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte(` +{ + "Status": "running" +}`)) + return nil + }) + return mockCmd + }, + }, + "return that container is exited": { + inContainerName: mockContainerName, + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockContainer"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte("53d6417769ed")) + return nil + }) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"inspect", "--format", "{{json .State}}", "53d6417769ed"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte(` +{ + "Status": "exited" +}`)) + return nil + }) + return mockCmd }, + wantedErr: fmt.Errorf(`container "mockContainer" exited with code 0`), }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { controller := gomock.NewController(t) - tc.setupMocks(controller) s := DockerCmdClient{ - runner: mockCmd, + runner: tc.setupMocks(controller), } _, err := s.IsContainerRunning(context.Background(), tc.inContainerName) if tc.wantedErr != nil { @@ -890,3 +934,228 @@ func TestDockerCommand_Exec(t *testing.T) { }) } } + +func TestDockerCommand_IsContainerHealthy(t *testing.T) { + tests := map[string]struct { + mockContainerName string + mockHealthStatus string + setupMocks func(*gomock.Controller) *MockCmd + wantHealthy bool + wantErr error + }{ + "unhealthy container": { + mockContainerName: "mockContainer", + mockHealthStatus: "unhealthy", + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockContainer"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte("53d6417769ed")) + return nil + }) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"inspect", "--format", "{{json .State}}", "53d6417769ed"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte(` +{ + "Status": "running", + "Running": true, + "Health": { + "Status": "unhealthy" + } +}`)) + return nil + }) + return mockCmd + }, + wantHealthy: false, + wantErr: fmt.Errorf(`container "mockContainer" is "unhealthy"`), + }, + + "healthy container": { + mockContainerName: "mockContainer", + mockHealthStatus: "unhealthy", + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockContainer"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte("53d6417769ed")) + return nil + }) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"inspect", "--format", "{{json .State}}", "53d6417769ed"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte(` +{ + "Status": "running", + "Running": true, + "Health": { + "Status": "healthy" + } +}`)) + return nil + }) + return mockCmd + }, + wantHealthy: true, + wantErr: nil, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + s := DockerCmdClient{ + runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function + } + + healthy, err := s.IsContainerHealthy(context.Background(), tc.mockContainerName) + require.Equal(t, tc.wantHealthy, healthy) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestDockerCommand_IsContainerComplete(t *testing.T) { + tests := map[string]struct { + mockContainerName string + mockHealthStatus string + setupMocks func(*gomock.Controller) *MockCmd + wantComplete bool + wantErr error + }{ + "container successfully complete": { + mockContainerName: "mockContainer", + mockHealthStatus: "unhealthy", + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockContainer"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte("53d6417769ed")) + return nil + }) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"inspect", "--format", "{{json .State}}", "53d6417769ed"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte(` +{ + "Status": "exited", + "ExitCode": 143 +}`)) + return nil + }) + return mockCmd + }, + wantComplete: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + s := DockerCmdClient{ + runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function + } + + healthy, err := s.IsContainerComplete(context.Background(), tc.mockContainerName) + require.Equal(t, tc.wantComplete, healthy) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestDockerCommand_IsContainerSuccess(t *testing.T) { + tests := map[string]struct { + mockContainerName string + mockHealthStatus string + setupMocks func(*gomock.Controller) *MockCmd + wantSuccess bool + wantErr error + }{ + "container successfully complete": { + mockContainerName: "mockContainer", + mockHealthStatus: "unhealthy", + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockContainer"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte("53d6417769ed")) + return nil + }) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"inspect", "--format", "{{json .State}}", "53d6417769ed"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte(` +{ + "Status": "exited", + "ExitCode": 0 +}`)) + return nil + }) + return mockCmd + }, + wantSuccess: true, + }, + "error when fetching container state": { + mockContainerName: "mockContainer", + mockHealthStatus: "unhealthy", + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockContainer"}, gomock.Any()).Return(fmt.Errorf("some error")) + return mockCmd + }, + wantErr: fmt.Errorf("run docker ps: some error"), + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + s := DockerCmdClient{ + runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function + } + + healthy, err := s.IsContainerSuccess(context.Background(), tc.mockContainerName) + require.Equal(t, tc.wantSuccess, healthy) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + } else { + require.NoError(t, err) + } + }) + } +} From b52bd34e5e02ea073e60c8b5af4ecd80f93dfcbc Mon Sep 17 00:00:00 2001 From: AdithyaKolla Date: Thu, 16 Nov 2023 11:47:48 -0800 Subject: [PATCH 2/7] remove comment --- internal/pkg/docker/dockerengine/dockerengine_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/docker/dockerengine/dockerengine_test.go b/internal/pkg/docker/dockerengine/dockerengine_test.go index 6803c847098..ee464391e4e 100644 --- a/internal/pkg/docker/dockerengine/dockerengine_test.go +++ b/internal/pkg/docker/dockerengine/dockerengine_test.go @@ -1146,7 +1146,7 @@ func TestDockerCommand_IsContainerSuccess(t *testing.T) { defer ctrl.Finish() s := DockerCmdClient{ - runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function + runner: tc.setupMocks(ctrl), } healthy, err := s.IsContainerSuccess(context.Background(), tc.mockContainerName) From 722dfc5bf5b37f46b4505ffd3475d7dc40f8a4c8 Mon Sep 17 00:00:00 2001 From: AdithyaKolla Date: Thu, 16 Nov 2023 12:01:40 -0800 Subject: [PATCH 3/7] clean up --- internal/pkg/docker/dockerengine/dockerengine.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/pkg/docker/dockerengine/dockerengine.go b/internal/pkg/docker/dockerengine/dockerengine.go index a8d493d072a..4f4e61bacd8 100644 --- a/internal/pkg/docker/dockerengine/dockerengine.go +++ b/internal/pkg/docker/dockerengine/dockerengine.go @@ -429,7 +429,6 @@ func (d *DockerCmdClient) containerState(ctx context.Context, containerName stri if containerID == "" { return ContainerState{}, nil } - // return ContainerState{}, nil buf := &bytes.Buffer{} if err := d.runner.RunWithContext(ctx, "docker", []string{"inspect", "--format", "{{json .State}}", containerID}, exec.Stdout(buf)); err != nil { return ContainerState{}, fmt.Errorf("run docker inspect: %w", err) From e698350dac8a0fb47d34b292225d543bd2193cc7 Mon Sep 17 00:00:00 2001 From: AdithyaKolla Date: Thu, 16 Nov 2023 12:11:58 -0800 Subject: [PATCH 4/7] tweak error msg --- internal/pkg/docker/dockerengine/dockerengine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/docker/dockerengine/dockerengine.go b/internal/pkg/docker/dockerengine/dockerengine.go index 4f4e61bacd8..8908840a9a4 100644 --- a/internal/pkg/docker/dockerengine/dockerengine.go +++ b/internal/pkg/docker/dockerengine/dockerengine.go @@ -404,7 +404,7 @@ func (c DockerCmdClient) IsContainerHealthy(ctx context.Context, containerName s case unhealthy: return false, fmt.Errorf("container %q is %q", containerName, unhealthy) case noHealthcheck: - return false, fmt.Errorf("healthcheck configuration is set to %q for container %s", unhealthy, containerName) + return false, fmt.Errorf("healthcheck configuration is set to %q for container %s", noHealthcheck, containerName) default: return false, fmt.Errorf("container %s had unexpected health status %q", containerName, state.Health.Status) } From 2a0a714347de39153d88130ef264f7b7d6af69ca Mon Sep 17 00:00:00 2001 From: AdithyaKolla Date: Sat, 18 Nov 2023 18:47:41 -0800 Subject: [PATCH 5/7] fix var names in tests --- .../pkg/docker/dockerengine/dockerengine_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/pkg/docker/dockerengine/dockerengine_test.go b/internal/pkg/docker/dockerengine/dockerengine_test.go index ee464391e4e..7f47f8d8af9 100644 --- a/internal/pkg/docker/dockerengine/dockerengine_test.go +++ b/internal/pkg/docker/dockerengine/dockerengine_test.go @@ -1021,8 +1021,8 @@ func TestDockerCommand_IsContainerHealthy(t *testing.T) { runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function } - healthy, err := s.IsContainerHealthy(context.Background(), tc.mockContainerName) - require.Equal(t, tc.wantHealthy, healthy) + expected, err := s.IsContainerHealthy(context.Background(), tc.mockContainerName) + require.Equal(t, tc.wantHealthy, expected) if tc.wantErr != nil { require.EqualError(t, err, tc.wantErr.Error()) } else { @@ -1080,8 +1080,8 @@ func TestDockerCommand_IsContainerComplete(t *testing.T) { runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function } - healthy, err := s.IsContainerComplete(context.Background(), tc.mockContainerName) - require.Equal(t, tc.wantComplete, healthy) + expected, err := s.IsContainerComplete(context.Background(), tc.mockContainerName) + require.Equal(t, tc.wantComplete, expected) if tc.wantErr != nil { require.EqualError(t, err, tc.wantErr.Error()) } else { @@ -1149,8 +1149,8 @@ func TestDockerCommand_IsContainerSuccess(t *testing.T) { runner: tc.setupMocks(ctrl), } - healthy, err := s.IsContainerSuccess(context.Background(), tc.mockContainerName) - require.Equal(t, tc.wantSuccess, healthy) + expected, err := s.IsContainerSuccess(context.Background(), tc.mockContainerName) + require.Equal(t, tc.wantSuccess, expected) if tc.wantErr != nil { require.EqualError(t, err, tc.wantErr.Error()) } else { From 1085c2c31f4f40740193e0afa4d891ecd08974e9 Mon Sep 17 00:00:00 2001 From: AdithyaKolla Date: Sat, 18 Nov 2023 20:11:38 -0800 Subject: [PATCH 6/7] combine success and complete --- .../pkg/docker/dockerengine/dockerengine.go | 17 ++---- .../docker/dockerengine/dockerengine_test.go | 57 +++++-------------- 2 files changed, 19 insertions(+), 55 deletions(-) diff --git a/internal/pkg/docker/dockerengine/dockerengine.go b/internal/pkg/docker/dockerengine/dockerengine.go index 8908840a9a4..65535653731 100644 --- a/internal/pkg/docker/dockerengine/dockerengine.go +++ b/internal/pkg/docker/dockerengine/dockerengine.go @@ -366,22 +366,13 @@ func (c DockerCmdClient) IsContainerRunning(ctx context.Context, name string) (b return false, nil } -// IsContainerComplete returns true if a docker container exits with an exitcode. -func (c DockerCmdClient) IsContainerComplete(ctx context.Context, containerName string) (bool, error) { +// IsContainerCompleteOrSuccess returns true if a docker container exits with an exitcode. +func (c DockerCmdClient) IsContainerCompleteOrSuccess(ctx context.Context, containerName string) (bool, int, error) { state, err := c.containerState(ctx, containerName) if err != nil { - return false, err - } - return state.Status == containerStatusExited, nil -} - -// IsContainerSuccess returns true if a docker container exits with exitcode 0. -func (c DockerCmdClient) IsContainerSuccess(ctx context.Context, containerName string) (bool, error) { - state, err := c.containerState(ctx, containerName) - if err != nil { - return false, err + return false, 0, err } - return state.Status == containerStatusExited && state.ExitCode == 0, nil + return state.Status == containerStatusExited, state.ExitCode, nil } // IsContainerHealthy returns true if a container health state is healthy. diff --git a/internal/pkg/docker/dockerengine/dockerengine_test.go b/internal/pkg/docker/dockerengine/dockerengine_test.go index 7f47f8d8af9..30f8fc528b3 100644 --- a/internal/pkg/docker/dockerengine/dockerengine_test.go +++ b/internal/pkg/docker/dockerengine/dockerengine_test.go @@ -1032,13 +1032,14 @@ func TestDockerCommand_IsContainerHealthy(t *testing.T) { } } -func TestDockerCommand_IsContainerComplete(t *testing.T) { +func TestDockerCommand_IsContainerCompleteOrSuccess(t *testing.T) { tests := map[string]struct { - mockContainerName string - mockHealthStatus string - setupMocks func(*gomock.Controller) *MockCmd - wantComplete bool - wantErr error + mockContainerName string + mockHealthStatus string + setupMocks func(*gomock.Controller) *MockCmd + wantCompleteOrSuccess bool + wantExitCode int + wantErr error }{ "container successfully complete": { mockContainerName: "mockContainer", @@ -1067,39 +1068,10 @@ func TestDockerCommand_IsContainerComplete(t *testing.T) { }) return mockCmd }, - wantComplete: true, + wantCompleteOrSuccess: true, + wantExitCode: 143, }, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - s := DockerCmdClient{ - runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function - } - - expected, err := s.IsContainerComplete(context.Background(), tc.mockContainerName) - require.Equal(t, tc.wantComplete, expected) - if tc.wantErr != nil { - require.EqualError(t, err, tc.wantErr.Error()) - } else { - require.NoError(t, err) - } - }) - } -} - -func TestDockerCommand_IsContainerSuccess(t *testing.T) { - tests := map[string]struct { - mockContainerName string - mockHealthStatus string - setupMocks func(*gomock.Controller) *MockCmd - wantSuccess bool - wantErr error - }{ - "container successfully complete": { + "container successfully success": { mockContainerName: "mockContainer", mockHealthStatus: "unhealthy", setupMocks: func(controller *gomock.Controller) *MockCmd { @@ -1126,7 +1098,7 @@ func TestDockerCommand_IsContainerSuccess(t *testing.T) { }) return mockCmd }, - wantSuccess: true, + wantCompleteOrSuccess: true, }, "error when fetching container state": { mockContainerName: "mockContainer", @@ -1146,11 +1118,12 @@ func TestDockerCommand_IsContainerSuccess(t *testing.T) { defer ctrl.Finish() s := DockerCmdClient{ - runner: tc.setupMocks(ctrl), + runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function } - expected, err := s.IsContainerSuccess(context.Background(), tc.mockContainerName) - require.Equal(t, tc.wantSuccess, expected) + expected, expectedCode, err := s.IsContainerCompleteOrSuccess(context.Background(), tc.mockContainerName) + require.Equal(t, tc.wantCompleteOrSuccess, expected) + require.Equal(t, tc.wantExitCode, expectedCode) if tc.wantErr != nil { require.EqualError(t, err, tc.wantErr.Error()) } else { From 6fd2927d9b90350caf3143fcaf1aaef196202347 Mon Sep 17 00:00:00 2001 From: AdithyaKolla Date: Fri, 1 Dec 2023 11:46:55 -0800 Subject: [PATCH 7/7] addr fb: exit code -1 for running change error msg --- .../pkg/docker/dockerengine/dockerengine.go | 19 ++++--- .../docker/dockerengine/dockerengine_test.go | 51 ++++++++++++++----- 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/internal/pkg/docker/dockerengine/dockerengine.go b/internal/pkg/docker/dockerengine/dockerengine.go index 65535653731..227b73e5dfb 100644 --- a/internal/pkg/docker/dockerengine/dockerengine.go +++ b/internal/pkg/docker/dockerengine/dockerengine.go @@ -367,12 +367,15 @@ func (c DockerCmdClient) IsContainerRunning(ctx context.Context, name string) (b } // IsContainerCompleteOrSuccess returns true if a docker container exits with an exitcode. -func (c DockerCmdClient) IsContainerCompleteOrSuccess(ctx context.Context, containerName string) (bool, int, error) { +func (c DockerCmdClient) IsContainerCompleteOrSuccess(ctx context.Context, containerName string) (int, error) { state, err := c.containerState(ctx, containerName) if err != nil { - return false, 0, err + return 0, err } - return state.Status == containerStatusExited, state.ExitCode, nil + if state.Status == containerStatusRunning { + return -1, nil + } + return state.ExitCode, nil } // IsContainerHealthy returns true if a container health state is healthy. @@ -381,11 +384,11 @@ func (c DockerCmdClient) IsContainerHealthy(ctx context.Context, containerName s if err != nil { return false, err } - if state.Status == containerStatusExited { - return false, &ErrContainerExited{name: containerName, exitcode: state.ExitCode} + if state.Status != containerStatusRunning { + return false, fmt.Errorf("container %q is not in %q state", containerName, containerStatusRunning) } if state.Health == nil { - return false, fmt.Errorf("healthcheck is not configured for container %s", containerName) + return false, fmt.Errorf("healthcheck is not configured for container %q", containerName) } switch state.Health.Status { case healthy: @@ -395,9 +398,9 @@ func (c DockerCmdClient) IsContainerHealthy(ctx context.Context, containerName s case unhealthy: return false, fmt.Errorf("container %q is %q", containerName, unhealthy) case noHealthcheck: - return false, fmt.Errorf("healthcheck configuration is set to %q for container %s", noHealthcheck, containerName) + return false, fmt.Errorf("healthcheck is not configured for container %q", containerName) default: - return false, fmt.Errorf("container %s had unexpected health status %q", containerName, state.Health.Status) + return false, fmt.Errorf("container %q had unexpected health status %q", containerName, state.Health.Status) } } diff --git a/internal/pkg/docker/dockerengine/dockerengine_test.go b/internal/pkg/docker/dockerengine/dockerengine_test.go index 30f8fc528b3..d8cd130f254 100644 --- a/internal/pkg/docker/dockerengine/dockerengine_test.go +++ b/internal/pkg/docker/dockerengine/dockerengine_test.go @@ -1034,12 +1034,11 @@ func TestDockerCommand_IsContainerHealthy(t *testing.T) { func TestDockerCommand_IsContainerCompleteOrSuccess(t *testing.T) { tests := map[string]struct { - mockContainerName string - mockHealthStatus string - setupMocks func(*gomock.Controller) *MockCmd - wantCompleteOrSuccess bool - wantExitCode int - wantErr error + mockContainerName string + mockHealthStatus string + setupMocks func(*gomock.Controller) *MockCmd + wantExitCode int + wantErr error }{ "container successfully complete": { mockContainerName: "mockContainer", @@ -1068,10 +1067,9 @@ func TestDockerCommand_IsContainerCompleteOrSuccess(t *testing.T) { }) return mockCmd }, - wantCompleteOrSuccess: true, - wantExitCode: 143, + wantExitCode: 143, }, - "container successfully success": { + "container success": { mockContainerName: "mockContainer", mockHealthStatus: "unhealthy", setupMocks: func(controller *gomock.Controller) *MockCmd { @@ -1098,7 +1096,6 @@ func TestDockerCommand_IsContainerCompleteOrSuccess(t *testing.T) { }) return mockCmd }, - wantCompleteOrSuccess: true, }, "error when fetching container state": { mockContainerName: "mockContainer", @@ -1110,6 +1107,35 @@ func TestDockerCommand_IsContainerCompleteOrSuccess(t *testing.T) { }, wantErr: fmt.Errorf("run docker ps: some error"), }, + "return negative exitcode if container is running": { + mockContainerName: "mockContainer", + mockHealthStatus: "unhealthy", + setupMocks: func(controller *gomock.Controller) *MockCmd { + mockCmd := NewMockCmd(controller) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"ps", "-a", "-q", "--filter", "name=mockContainer"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte("53d6417769ed")) + return nil + }) + mockCmd.EXPECT().RunWithContext(gomock.Any(), "docker", []string{"inspect", "--format", "{{json .State}}", "53d6417769ed"}, gomock.Any()).DoAndReturn(func(ctx context.Context, name string, args []string, opts ...exec.CmdOption) error { + cmd := &osexec.Cmd{} + for _, opt := range opts { + opt(cmd) + } + cmd.Stdout.Write([]byte(` +{ + "Status": "running", + "ExitCode": 0 +}`)) + return nil + }) + return mockCmd + }, + wantExitCode: -1, + }, } for name, tc := range tests { @@ -1118,11 +1144,10 @@ func TestDockerCommand_IsContainerCompleteOrSuccess(t *testing.T) { defer ctrl.Finish() s := DockerCmdClient{ - runner: tc.setupMocks(ctrl), // Correctly invoke the setupMocks function + runner: tc.setupMocks(ctrl), } - expected, expectedCode, err := s.IsContainerCompleteOrSuccess(context.Background(), tc.mockContainerName) - require.Equal(t, tc.wantCompleteOrSuccess, expected) + expectedCode, err := s.IsContainerCompleteOrSuccess(context.Background(), tc.mockContainerName) require.Equal(t, tc.wantExitCode, expectedCode) if tc.wantErr != nil { require.EqualError(t, err, tc.wantErr.Error())