From 139f2c0b0f8ee08b6ff0e0af184f4f5cc16b0ad8 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 31 Jan 2023 07:53:18 -0600 Subject: [PATCH] docker: set force=true on remove image to handle images referenced by 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 --- .changelog/15962.txt | 3 +++ drivers/docker/coordinator.go | 6 ++++-- drivers/docker/coordinator_test.go | 2 +- website/content/docs/drivers/docker.mdx | 3 ++- 4 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 .changelog/15962.txt diff --git a/.changelog/15962.txt b/.changelog/15962.txt new file mode 100644 index 000000000..602aedc96 --- /dev/null +++ b/.changelog/15962.txt @@ -0,0 +1,3 @@ +```release-note:bug +docker: Fixed a bug where images referenced by multiple tags would not be GC'd +``` diff --git a/drivers/docker/coordinator.go b/drivers/docker/coordinator.go index 7a99acdb3..edbaf0f63 100644 --- a/drivers/docker/coordinator.go +++ b/drivers/docker/coordinator.go @@ -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. @@ -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 } diff --git a/drivers/docker/coordinator_test.go b/drivers/docker/coordinator_test.go index 755c6b99e..0e08551e1 100644 --- a/drivers/docker/coordinator_test.go +++ b/drivers/docker/coordinator_test.go @@ -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]++ diff --git a/website/content/docs/drivers/docker.mdx b/website/content/docs/drivers/docker.mdx index 7375d10cb..bcdffb1a2 100644 --- a/website/content/docs/drivers/docker.mdx +++ b/website/content/docs/drivers/docker.mdx @@ -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,