From 2af897c76f22d909c4cdc99db41edd97503a131f Mon Sep 17 00:00:00 2001 From: Chris Baker Date: Mon, 3 Jun 2019 19:17:57 +0000 Subject: [PATCH] drivers/docker: modify container/image cleanup to be robust to containers removed out of band --- drivers/docker/driver.go | 38 +++++++++++--------- drivers/docker/driver_test.go | 65 +++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 16 deletions(-) diff --git a/drivers/docker/driver.go b/drivers/docker/driver.go index 7b2fa73a4..7eb82a08d 100644 --- a/drivers/docker/driver.go +++ b/drivers/docker/driver.go @@ -1107,24 +1107,30 @@ func (d *Driver) DestroyTask(taskID string, force bool) error { c, err := h.client.InspectContainer(h.containerID) if err != nil { - return fmt.Errorf("failed to inspect container state: %v", err) - } - - if c.State.Running { - if !force { - return fmt.Errorf("must call StopTask for the given task before Destroy or set force to true") - } - if err := h.client.StopContainer(h.containerID, 0); err != nil { - h.logger.Warn("failed to stop container during destroy", "error", err) - } - } - - if h.removeContainerOnExit { - if err := h.client.RemoveContainer(docker.RemoveContainerOptions{ID: h.containerID, RemoveVolumes: true, Force: true}); err != nil { - h.logger.Error("error removing container", "error", err) + switch err.(type) { + case *docker.NoSuchContainer: + h.logger.Error("failed to inspect container state, will proceed with DestroyTask", + "error", err) + default: + return fmt.Errorf("failed to inspect container state: %v", err) } } else { - h.logger.Debug("not removing container due to config") + if c.State.Running { + if !force { + return fmt.Errorf("must call StopTask for the given task before Destroy or set force to true") + } + if err := h.client.StopContainer(h.containerID, 0); err != nil { + h.logger.Warn("failed to stop container during destroy", "error", err) + } + } + + if h.removeContainerOnExit { + if err := h.client.RemoveContainer(docker.RemoveContainerOptions{ID: h.containerID, RemoveVolumes: true, Force: true}); err != nil { + h.logger.Error("error removing container", "error", err) + } + } else { + h.logger.Debug("not removing container due to config") + } } if err := d.cleanupImage(h); err != nil { diff --git a/drivers/docker/driver_test.go b/drivers/docker/driver_test.go index 8cac5f780..445950d67 100644 --- a/drivers/docker/driver_test.go +++ b/drivers/docker/driver_test.go @@ -1570,6 +1570,71 @@ func TestDockerDriver_DisableImageGC(t *testing.T) { dockerDriver.coordinator.imageLock.Unlock() } +func TestDockerDriver_MissingContainer_Cleanup(t *testing.T) { + testutil.DockerCompatible(t) + + task, cfg, _ := dockerTask(t) + cfg.Command = "echo" + cfg.Args = []string{"hello"} + require.NoError(t, task.EncodeConcreteDriverConfig(cfg)) + + client := newTestDockerClient(t) + driver := dockerDriverHarness(t, map[string]interface{}{ + "gc": map[string]interface{}{ + "container": true, + "image": true, + "image_delay": "0s", + }, + }) + cleanup := driver.MkAllocDir(task, true) + defer cleanup() + + cleanSlate(client, cfg.Image) + + copyImage(t, task.TaskDir(), "busybox.tar") + _, _, err := driver.StartTask(task) + require.NoError(t, err) + + dockerDriver, ok := driver.Impl().(*Driver) + require.True(t, ok) + h, ok := dockerDriver.tasks.Get(task.ID) + require.True(t, ok) + + waitCh, err := dockerDriver.WaitTask(context.Background(), task.ID) + require.NoError(t, err) + select { + case res := <-waitCh: + if !res.Successful() { + t.Fatalf("err: %v", res) + } + + case <-time.After(time.Duration(tu.TestMultiplier()*5) * time.Second): + t.Fatalf("timeout") + } + + // remove the container out-of-band + require.NoError(t, client.RemoveContainer(docker.RemoveContainerOptions{ + ID: h.containerID, + })) + + require.NoError(t, dockerDriver.DestroyTask(task.ID, false)) + + // Ensure image was removed + tu.WaitForResult(func() (bool, error) { + if _, err := client.InspectImage(cfg.Image); err == nil { + return false, fmt.Errorf("image exists but should have been removed. Does another %v container exist?", cfg.Image) + } + + return true, nil + }, func(err error) { + require.NoError(t, err) + }) + + // Ensure that task handle was removed + _, ok = dockerDriver.tasks.Get(task.ID) + require.False(t, ok) +} + func TestDockerDriver_Stats(t *testing.T) { if !tu.IsCI() { t.Parallel()