From b98b8df886de71da64a313bf9a17b15012a85691 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 25 May 2020 14:07:22 +0200 Subject: [PATCH 1/2] allocateNetwork: fix network sandbox not cleaned up on failure The defer function was checking for the local `err` variable, not on the error that was returned by the function. As a result, the sandbox would never be cleaned up for containers that used "none" networking, and a failiure occured during setup. Signed-off-by: Sebastiaan van Stijn --- daemon/container_operations.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/container_operations.go b/daemon/container_operations.go index bd5dea4df9..1b449a86c5 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -526,7 +526,7 @@ func (daemon *Daemon) updateContainerNetworkSettings(container *container.Contai } } -func (daemon *Daemon) allocateNetwork(container *container.Container) error { +func (daemon *Daemon) allocateNetwork(container *container.Container) (retErr error) { start := time.Now() controller := daemon.netController @@ -594,7 +594,7 @@ func (daemon *Daemon) allocateNetwork(container *container.Container) error { } updateSandboxNetworkSettings(container, sb) defer func() { - if err != nil { + if retErr != nil { sb.Delete() } }() From 84ef60cba26f99a07a6d2c4058a1c9d35a0142a9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 25 May 2020 14:03:47 +0200 Subject: [PATCH 2/2] allocateNetwork: don't assign unneeded variables allocateNetwork() can return early, in which case these variables were unused. Signed-off-by: Sebastiaan van Stijn --- daemon/container_operations.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 1b449a86c5..292685a927 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -527,13 +527,15 @@ func (daemon *Daemon) updateContainerNetworkSettings(container *container.Contai } func (daemon *Daemon) allocateNetwork(container *container.Container) (retErr error) { - start := time.Now() - controller := daemon.netController - if daemon.netController == nil { return nil } + var ( + start = time.Now() + controller = daemon.netController + ) + // Cleanup any stale sandbox left over due to ungraceful daemon shutdown if err := controller.SandboxDestroy(container.ID); err != nil { logrus.Errorf("failed to cleanup up stale network sandbox for container %s", container.ID)