Skip to content

Commit

Permalink
docker: Fix name conflict when creating a container
Browse files Browse the repository at this point in the history
Connected to #280
Connected to resin-io/hq#401

Signed-off-by: Petros Angelatos <petrosagg@gmail.com>
  • Loading branch information
petrosagg authored and Andrei Gherzan committed Nov 4, 2016
1 parent d939833 commit a637447
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
From 1e3303b4ae3c2c280a965d382e2633ef0e107fc3 Mon Sep 17 00:00:00 2001
From: Alexander Morozov <lk4d4@docker.com>
Date: Tue, 29 Mar 2016 13:52:18 -0700
Subject: [PATCH 7/8] daemon: register container as late as possible

fixes races between list and create

Signed-off-by: Alexander Morozov <lk4d4@docker.com>
Upstream-Status: Backport
Signed-off-by: Petros Angelatos <petrosagg@gmail.com>
---
daemon/create.go | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/daemon/create.go b/daemon/create.go
index 166af3b..2ba0d5b 100644
--- a/daemon/create.go
+++ b/daemon/create.go
@@ -73,8 +73,8 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig) (retC *containe
}
defer func() {
if retErr != nil {
- if err := daemon.ContainerRm(container.ID, &types.ContainerRmConfig{ForceRemove: true}); err != nil {
- logrus.Errorf("Clean up Error! Cannot destroy container %s: %v", container.ID, err)
+ if err := daemon.cleanupContainer(container, true); err != nil {
+ logrus.Errorf("failed to cleanup container on create error: %v", err)
}
}
}()
@@ -88,9 +88,6 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig) (retC *containe
return nil, err
}

- if err := daemon.Register(container); err != nil {
- return nil, err
- }
rootUID, rootGID, err := idtools.GetRootUIDGID(daemon.uidMaps, daemon.gidMaps)
if err != nil {
return nil, err
@@ -123,10 +120,13 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig) (retC *containe
return nil, err
}

- if err := container.ToDiskLocking(); err != nil {
+ if err := container.ToDisk(); err != nil {
logrus.Errorf("Error saving new container to disk: %v", err)
return nil, err
}
+ if err := daemon.Register(container); err != nil {
+ return nil, err
+ }
daemon.LogContainerEvent(container, "create")
return container, nil
}
--
2.10.2

Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
From 9e9d9b64dd9e4b0b6ad0baa406f86a08fa3cde0d Mon Sep 17 00:00:00 2001
From: Petros Angelatos <petrosagg@gmail.com>
Date: Tue, 1 Nov 2016 15:07:32 +0000
Subject: [PATCH 8/8] daemon: cleanup as early as possible

`cleanupContainer` can fail to delete a container name from nameIndex if
for example `container.ToDiskLocking()` fails. This is a problem because
users of this function rely on it always cleaning up to ensure the
in-memory data structures are consistent.

One user is Daemon.newContainer() which first updates the nameIndex, and
then registers the container, calling `cleanupContainer` whenever
anything fails inbetween.

Partial solution for #23371

Upstream-Status: Submitted [https://github.com/docker/docker/pull/27956]
Signed-off-by: Petros Angelatos <petrosagg@gmail.com>
---
daemon/delete.go | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/daemon/delete.go b/daemon/delete.go
index a086aed..d17f890 100644
--- a/daemon/delete.go
+++ b/daemon/delete.go
@@ -82,6 +82,19 @@ func (daemon *Daemon) rmLink(container *container.Container, name string) error
// cleanupContainer unregisters a container from the daemon, stops stats
// collection and cleanly removes contents and metadata from the filesystem.
func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemove bool) (err error) {
+ // If force removal is required, delete container from various
+ // indexes even if removal failed.
+ defer func() {
+ if err == nil || forceRemove {
+ daemon.nameIndex.Delete(container.ID)
+ daemon.linkIndex.delete(container)
+ selinuxFreeLxcContexts(container.ProcessLabel)
+ daemon.idIndex.Delete(container.ID)
+ daemon.containers.Delete(container.ID)
+ daemon.LogContainerEvent(container, "destroy")
+ }
+ }()
+
if container.IsRunning() {
if !forceRemove {
return derr.ErrorCodeRmRunning
@@ -109,19 +122,6 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
logrus.Errorf("Error saving dying container to disk: %v", err)
}

- // If force removal is required, delete container from various
- // indexes even if removal failed.
- defer func() {
- if err == nil || forceRemove {
- daemon.nameIndex.Delete(container.ID)
- daemon.linkIndex.delete(container)
- selinuxFreeLxcContexts(container.ProcessLabel)
- daemon.idIndex.Delete(container.ID)
- daemon.containers.Delete(container.ID)
- daemon.LogContainerEvent(container, "destroy")
- }
- }()
-
if err = os.RemoveAll(container.Root); err != nil {
return derr.ErrorCodeRmFS.WithArgs(container.ID, err)
}
--
2.10.2

4 changes: 3 additions & 1 deletion meta-resin-common/recipes-containers/docker/docker_git.bb
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ SRC_URI = "\
file://0004-Set-permission-on-atomic-file-write.patch \
file://0005-Update-layer-store-to-sync-transaction-files-before-.patch \
file://0006-Atomically-save-libtrust-key-file.patch \
file://0007-daemon-register-container-as-late-as-possible.patch \
file://0008-daemon-cleanup-as-early-as-possible.patch \
file://0009-graph-aufs-durably-write-layer-on-disk-before-return.patch \
file://0010-pkg-ioutils-sync-parent-directory-too.patch \
"
"

# Apache-2.0 for docker
LICENSE = "Apache-2.0"
Expand Down

0 comments on commit a637447

Please sign in to comment.