Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup docker images #5768

Merged
merged 5 commits into from
Jun 4, 2019
+270 −28
Merged

Cleanup docker images #5768

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3a96683
docker: DestroyTask was not cleaning up Docker images because it was …
May 29, 2019
3b82770
docker/tests:
May 30, 2019
60a3e95
update changelog
Jun 3, 2019
262c863
drivers/docker: modify container/image cleanup to be robust to contai…
Jun 3, 2019
7b6d233
docker/driver: downgraded log level for error in DestroyTask
Jun 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ BUG FIXES:
* client: Job validation now checks that the datacenter field does not contain empty strings [[GH-5665](https://github.com/hashicorp/nomad/pull/5665)]
* client: Fix network port mapping related environment variables when running with Nomad 0.8 servers [[GH-5587](https://github.com/hashicorp/nomad/issues/5587)]
* client: Fix issue with terminal state deployments being modified when allocation subsequently fails [[GH-5645](https://github.com/hashicorp/nomad/issues/5645)]
* driver/docker: Fix regression around image GC [[GH-5768](https://github.com/hashicorp/nomad/issues/5768)]
* metrics: Fixed stale metrics [[GH-5540](https://github.com/hashicorp/nomad/issues/5540)]
* vault: Fix renewal time to be 1/2 lease duration with jitter [[GH-5479](https://github.com/hashicorp/nomad/issues/5479)]

Expand Down
30 changes: 23 additions & 7 deletions drivers/docker/driver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,14 +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 && !force {
return fmt.Errorf("must call StopTask for the given task before Destroy or set force to true")
}
switch err.(type) {
case *docker.NoSuchContainer:
h.logger.Error("failed to inspect container state, will proceed with DestroyTask",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit odd to be Error log considering that we handle it quite well. I would downgrade it to info or debug with it indicating "container was deleted out-of-band, will proceed with DestroyTask"`.

"error", err)
default:
return fmt.Errorf("failed to inspect container state: %v", err)
}
} else {
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 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 {
Expand Down
Loading