Skip to content

Commit

Permalink
docker: use docker errdefs instead of string comparisons when checkin…
Browse files Browse the repository at this point in the history
…g errors (#24075)
  • Loading branch information
pkazmierczak authored Sep 27, 2024
1 parent c1127db commit ec42aa2
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 19 deletions.
6 changes: 3 additions & 3 deletions drivers/docker/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import (
"fmt"
"io"
"regexp"
"strings"
"sync"
"time"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/image"
"github.com/docker/docker/api/types/registry"
"github.com/docker/docker/errdefs"
hclog "github.com/hashicorp/go-hclog"
"github.com/hashicorp/nomad/nomad/structs"
)
Expand Down Expand Up @@ -344,11 +344,11 @@ func (d *dockerCoordinator) removeImageImpl(id string, ctx context.Context) {
break
}

if strings.Contains(err.Error(), "No such image") {
if errdefs.IsNotFound(err) {
d.logger.Debug("unable to cleanup image, does not exist", "image_id", id)
return
}
if derr, ok := err.(*types.ErrorResponse); ok && strings.Contains(derr.Error(), "Conflict") {
if errdefs.IsConflict(err) {
d.logger.Debug("unable to cleanup image, still in use", "image_id", id)
return
}
Expand Down
3 changes: 2 additions & 1 deletion drivers/docker/docklog/docker_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

containerapi "github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/pkg/stdcopy"
"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-multierror"
Expand Down Expand Up @@ -135,7 +136,7 @@ func (d *dockerLogger) Start(opts *StartOpts) error {

container, err := client.ContainerInspect(ctx, opts.ContainerID)
if err != nil {
if !strings.Contains(err.Error(), "No such container") {
if !errdefs.IsNotFound(err) {
return
}
} else if !container.State.Running {
Expand Down
11 changes: 6 additions & 5 deletions drivers/docker/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
networkapi "github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/registry"
"github.com/docker/docker/client"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/pkg/stdcopy"
"github.com/hashicorp/consul-template/signals"
hclog "github.com/hashicorp/go-hclog"
Expand Down Expand Up @@ -408,7 +409,7 @@ CREATE:
d.logger.Error("failed to start container", "container_id", container.ID, "error", err)
dockerClient.ContainerRemove(d.ctx, container.ID, containerapi.RemoveOptions{Force: true})
// Some sort of docker race bug, recreating the container usually works
if strings.Contains(err.Error(), "OCI runtime create failed: container with id exists:") && startAttempts < 5 {
if errdefs.IsConflict(err) && startAttempts < 5 {
startAttempts++
d.logger.Debug("reattempting container create/start sequence", "attempt", startAttempts, "container_id", id)
goto CREATE
Expand Down Expand Up @@ -529,7 +530,7 @@ CREATE:

// If the container already exists determine whether it's already
// running or if it's dead and needs to be recreated.
if strings.Contains(strings.ToLower(createErr.Error()), "conflict. the container name") {
if errdefs.IsConflict(createErr) {

container, err := d.containerByName(config.Name)
if err != nil {
Expand Down Expand Up @@ -561,7 +562,7 @@ CREATE:
goto CREATE
}

} else if strings.Contains(strings.ToLower(createErr.Error()), "no such image") {
} else if errdefs.IsNotFound(createErr) {
// There is still a very small chance this is possible even with the
// coordinator so retry.
return nil, nstructs.NewRecoverableError(createErr, true)
Expand All @@ -588,7 +589,7 @@ func (d *Driver) startContainer(c types.ContainerJSON) error {

START:
startErr := dockerClient.ContainerStart(d.ctx, c.ID, containerapi.StartOptions{})
if startErr == nil || strings.Contains(startErr.Error(), "Container already running") {
if startErr == nil || errdefs.IsConflict(err) {
return nil
}

Expand Down Expand Up @@ -1641,7 +1642,7 @@ func (d *Driver) DestroyTask(taskID string, force bool) error {

c, err := dockerClient.ContainerInspect(d.ctx, h.containerID)
if err != nil {
if strings.Contains(err.Error(), NoSuchContainerError) {
if _, ok := err.(errdefs.ErrNotFound); ok {
h.logger.Info("container was removed out of band, will proceed with DestroyTask",
"error", err)
} else {
Expand Down
5 changes: 3 additions & 2 deletions drivers/docker/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
networkapi "github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/registry"
"github.com/docker/docker/client"
"github.com/docker/docker/errdefs"
"github.com/docker/go-connections/nat"
hclog "github.com/hashicorp/go-hclog"
"github.com/hashicorp/nomad/ci"
Expand Down Expand Up @@ -376,7 +377,7 @@ func TestDockerDriver_Start_StoppedContainer(t *testing.T) {
must.NoError(t, err)

if _, err := client.ContainerCreate(context.Background(), opts, nil, nil, nil, containerName); err != nil {
if !strings.Contains(err.Error(), "Conflict") {
if !errdefs.IsConflict(err) {
t.Fatalf("error creating initial container: %v", err)
}
}
Expand Down Expand Up @@ -2890,7 +2891,7 @@ func waitForExist(t *testing.T, client *client.Client, containerID string) {
tu.WaitForResult(func() (bool, error) {
container, err := client.ContainerInspect(context.Background(), containerID)
if err != nil {
if !strings.Contains(err.Error(), NoSuchContainerError) {
if !errdefs.IsNotFound(err) {
return false, err
}
}
Expand Down
14 changes: 6 additions & 8 deletions drivers/docker/handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import (
"fmt"
"os"
"runtime"
"strings"
"sync"
"time"

"github.com/armon/circbuf"
containerapi "github.com/docker/docker/api/types/container"
"github.com/docker/docker/client"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/pkg/stdcopy"
"github.com/hashicorp/consul-template/signals"
"github.com/hashicorp/go-hclog"
Expand Down Expand Up @@ -192,12 +192,12 @@ func (h *taskHandle) Kill(killTimeout time.Duration, signal string) error {

if err := h.Signal(ctx, signal); err != nil {
// Container has already been removed.
if strings.Contains(err.Error(), NoSuchContainerError) {
if errdefs.IsNotFound(err) {
h.logger.Debug("attempted to signal nonexistent container")
return nil
}
// Container has already been stopped.
if strings.Contains(err.Error(), ContainerNotRunningError) {
if errdefs.IsNotModified(err) {
h.logger.Debug("attempted to signal a not-running container")
return nil
}
Expand All @@ -218,12 +218,12 @@ func (h *taskHandle) Kill(killTimeout time.Duration, signal string) error {

if err != nil {
// Container has already been removed.
if strings.Contains(err.Error(), NoSuchContainerError) {
if errdefs.IsNotFound(err) {
h.logger.Debug("attempted to stop nonexistent container")
return nil
}
// Container has already been stopped.
if strings.Contains(err.Error(), ContainerNotRunningError) {
if errdefs.IsNotModified(err) {
h.logger.Debug("attempted to stop an not-running container")
return nil
}
Expand Down Expand Up @@ -335,9 +335,7 @@ func (h *taskHandle) run() {
if err := h.dockerClient.ContainerStop(ctx, h.containerID, containerapi.StopOptions{
Timeout: pointer.Of(0),
}); err != nil {
noSuchContainer := strings.Contains(err.Error(), NoSuchContainerError)
containerNotRunning := strings.Contains(err.Error(), ContainerNotRunningError)
if !containerNotRunning && !noSuchContainer {
if !errdefs.IsNotModified(err) && !errdefs.IsNotFound(err) {
h.logger.Error("error stopping container", "error", err)
}
}
Expand Down

0 comments on commit ec42aa2

Please sign in to comment.