Skip to content

Commit

Permalink
docker: set force=true on remove image to handle images referenced by…
Browse files Browse the repository at this point in the history
… multiple tags (#15962)

* docker: set force=true on remove image to handle images referenced by multiple tags

This PR changes our call of docker client RemoveImage() to RemoveImageExtended with
the Force=true option set. This fixes a bug where an image referenced by more than
one tag could never be garbage collected by Nomad. The Force option only applies to
stopped containers; it does not affect running workloads.

* docker: add note about image_delay and multiple tags
  • Loading branch information
shoenig committed Jan 31, 2023
1 parent ecf5a51 commit 7f3bdd4
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .changelog/15962.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
docker: Fixed a bug where images referenced by multiple tags would not be GC'd
```
6 changes: 4 additions & 2 deletions drivers/docker/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (p *pullFuture) set(imageID string, err error) {
type DockerImageClient interface {
PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error
InspectImage(id string) (*docker.Image, error)
RemoveImage(id string) error
RemoveImageExtended(id string, opts docker.RemoveImageOptions) error
}

// LogEventFn is a callback which allows Drivers to emit task events.
Expand Down Expand Up @@ -320,7 +320,9 @@ func (d *dockerCoordinator) removeImageImpl(id string, ctx context.Context) {
d.imageLock.Unlock()

for i := 0; i < 3; i++ {
err := d.client.RemoveImage(id)
err := d.client.RemoveImageExtended(id, docker.RemoveImageOptions{
Force: true, // necessary to GC images referenced by multiple tags
})
if err == nil {
break
}
Expand Down
2 changes: 1 addition & 1 deletion drivers/docker/coordinator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (m *mockImageClient) InspectImage(id string) (*docker.Image, error) {
}, nil
}

func (m *mockImageClient) RemoveImage(id string) error {
func (m *mockImageClient) RemoveImageExtended(id string, options docker.RemoveImageOptions) error {
m.lock.Lock()
defer m.lock.Unlock()
m.removed[id]++
Expand Down
3 changes: 2 additions & 1 deletion website/content/docs/drivers/docker.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,8 @@ host system.
here](https://golang.org/pkg/time/#ParseDuration), that defaults to `3m`.
The delay controls how long Nomad will wait between an image being unused
and deleting it. If a task is received that uses the same image within
the delay, the image will be reused.
the delay, the image will be reused. If an image is referenced by more than
one tag, `image_delay` may not work correctly.

- `container` - Defaults to `true`. This option can be used to disable Nomad
from removing a container when the task exits. Under a name conflict,
Expand Down

0 comments on commit 7f3bdd4

Please sign in to comment.