From 2c6c07752c8d89fa72249c24285ede1b4e579b24 Mon Sep 17 00:00:00 2001 From: David Calavera Date: Mon, 14 Sep 2015 13:21:17 -0400 Subject: [PATCH] Remove volume references when container creation fails. Volumes are accounted when a container is created. If the creation fails we should remove the reference from the counter. Do not log ErrVolumeInUse as an error, having other volume references is not an error. Signed-off-by: David Calavera --- daemon/container_unix.go | 13 +++++++++---- daemon/create.go | 7 +++++++ daemon/delete.go | 2 +- daemon/volumes.go | 5 +++++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/daemon/container_unix.go b/daemon/container_unix.go index 4eca92ed30..46e3417f2f 100644 --- a/daemon/container_unix.go +++ b/daemon/container_unix.go @@ -1208,14 +1208,19 @@ func (container *Container) removeMountPoints(rm bool) error { } container.daemon.volumes.Decrement(m.Volume) if rm { - if err := container.daemon.volumes.Remove(m.Volume); err != nil { - rmErrors = append(rmErrors, fmt.Sprintf("%v\n", err)) - continue + err := container.daemon.volumes.Remove(m.Volume) + // ErrVolumeInUse is ignored because having this + // volume being referenced by othe container is + // not an error, but an implementation detail. + // This prevents docker from logging "ERROR: Volume in use" + // where there is another container using the volume. + if err != nil && err != ErrVolumeInUse { + rmErrors = append(rmErrors, err.Error()) } } } if len(rmErrors) > 0 { - return fmt.Errorf("Error removing volumes:\n%v", rmErrors) + return fmt.Errorf("Error removing volumes:\n%v", strings.Join(rmErrors, "\n")) } return nil } diff --git a/daemon/create.go b/daemon/create.go index e5349c6c33..6c63259cef 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -101,6 +101,13 @@ func (daemon *Daemon) Create(config *runconfig.Config, hostConfig *runconfig.Hos if err := daemon.setHostConfig(container, hostConfig); err != nil { return nil, nil, err } + defer func() { + if retErr != nil { + if err := container.removeMountPoints(true); err != nil { + logrus.Error(err) + } + } + }() if err := container.Mount(); err != nil { return nil, nil, err } diff --git a/daemon/delete.go b/daemon/delete.go index 19ee408fe0..1c8d6ac28a 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -56,7 +56,7 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error } if err := container.removeMountPoints(config.RemoveVolume); err != nil { - logrus.Errorf("%v", err) + logrus.Error(err) } return nil diff --git a/daemon/volumes.go b/daemon/volumes.go index 82935de720..134283bc81 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -9,6 +9,7 @@ import ( "strings" "sync" + "github.com/Sirupsen/logrus" "github.com/docker/docker/api/types" "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/system" @@ -139,6 +140,7 @@ func (s *volumeStore) Create(name, driverName string, opts map[string]string) (v return v, nil } s.mu.Unlock() + logrus.Debugf("Registering new volume reference: driver %s, name %s", driverName, name) vd, err := getVolumeDriver(driverName) if err != nil { @@ -173,6 +175,7 @@ func (s *volumeStore) Remove(v volume.Volume) error { s.mu.Lock() defer s.mu.Unlock() name := v.Name() + logrus.Debugf("Removing volume reference: driver %s, name %s", v.DriverName(), name) vc, exists := s.vols[name] if !exists { return ErrNoSuchVolume @@ -197,6 +200,7 @@ func (s *volumeStore) Remove(v volume.Volume) error { func (s *volumeStore) Increment(v volume.Volume) { s.mu.Lock() defer s.mu.Unlock() + logrus.Debugf("Incrementing volume reference: driver %s, name %s", v.DriverName(), v.Name()) vc, exists := s.vols[v.Name()] if !exists { @@ -211,6 +215,7 @@ func (s *volumeStore) Increment(v volume.Volume) { func (s *volumeStore) Decrement(v volume.Volume) { s.mu.Lock() defer s.mu.Unlock() + logrus.Debugf("Decrementing volume reference: driver %s, name %s", v.DriverName(), v.Name()) vc, exists := s.vols[v.Name()] if !exists {