Browse code

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 <david.calavera@gmail.com>

David Calavera authored on 2015/09/15 02:21:17
Showing 4 changed files
... ...
@@ -1208,14 +1208,19 @@ func (container *Container) removeMountPoints(rm bool) error {
1208 1208
 		}
1209 1209
 		container.daemon.volumes.Decrement(m.Volume)
1210 1210
 		if rm {
1211
-			if err := container.daemon.volumes.Remove(m.Volume); err != nil {
1212
-				rmErrors = append(rmErrors, fmt.Sprintf("%v\n", err))
1213
-				continue
1211
+			err := container.daemon.volumes.Remove(m.Volume)
1212
+			// ErrVolumeInUse is ignored because having this
1213
+			// volume being referenced by othe container is
1214
+			// not an error, but an implementation detail.
1215
+			// This prevents docker from logging "ERROR: Volume in use"
1216
+			// where there is another container using the volume.
1217
+			if err != nil && err != ErrVolumeInUse {
1218
+				rmErrors = append(rmErrors, err.Error())
1214 1219
 			}
1215 1220
 		}
1216 1221
 	}
1217 1222
 	if len(rmErrors) > 0 {
1218
-		return fmt.Errorf("Error removing volumes:\n%v", rmErrors)
1223
+		return fmt.Errorf("Error removing volumes:\n%v", strings.Join(rmErrors, "\n"))
1219 1224
 	}
1220 1225
 	return nil
1221 1226
 }
... ...
@@ -101,6 +101,13 @@ func (daemon *Daemon) Create(config *runconfig.Config, hostConfig *runconfig.Hos
101 101
 	if err := daemon.setHostConfig(container, hostConfig); err != nil {
102 102
 		return nil, nil, err
103 103
 	}
104
+	defer func() {
105
+		if retErr != nil {
106
+			if err := container.removeMountPoints(true); err != nil {
107
+				logrus.Error(err)
108
+			}
109
+		}
110
+	}()
104 111
 	if err := container.Mount(); err != nil {
105 112
 		return nil, nil, err
106 113
 	}
... ...
@@ -56,7 +56,7 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error
56 56
 	}
57 57
 
58 58
 	if err := container.removeMountPoints(config.RemoveVolume); err != nil {
59
-		logrus.Errorf("%v", err)
59
+		logrus.Error(err)
60 60
 	}
61 61
 
62 62
 	return nil
... ...
@@ -9,6 +9,7 @@ import (
9 9
 	"strings"
10 10
 	"sync"
11 11
 
12
+	"github.com/Sirupsen/logrus"
12 13
 	"github.com/docker/docker/api/types"
13 14
 	"github.com/docker/docker/pkg/chrootarchive"
14 15
 	"github.com/docker/docker/pkg/system"
... ...
@@ -139,6 +140,7 @@ func (s *volumeStore) Create(name, driverName string, opts map[string]string) (v
139 139
 		return v, nil
140 140
 	}
141 141
 	s.mu.Unlock()
142
+	logrus.Debugf("Registering new volume reference: driver %s, name %s", driverName, name)
142 143
 
143 144
 	vd, err := getVolumeDriver(driverName)
144 145
 	if err != nil {
... ...
@@ -173,6 +175,7 @@ func (s *volumeStore) Remove(v volume.Volume) error {
173 173
 	s.mu.Lock()
174 174
 	defer s.mu.Unlock()
175 175
 	name := v.Name()
176
+	logrus.Debugf("Removing volume reference: driver %s, name %s", v.DriverName(), name)
176 177
 	vc, exists := s.vols[name]
177 178
 	if !exists {
178 179
 		return ErrNoSuchVolume
... ...
@@ -197,6 +200,7 @@ func (s *volumeStore) Remove(v volume.Volume) error {
197 197
 func (s *volumeStore) Increment(v volume.Volume) {
198 198
 	s.mu.Lock()
199 199
 	defer s.mu.Unlock()
200
+	logrus.Debugf("Incrementing volume reference: driver %s, name %s", v.DriverName(), v.Name())
200 201
 
201 202
 	vc, exists := s.vols[v.Name()]
202 203
 	if !exists {
... ...
@@ -211,6 +215,7 @@ func (s *volumeStore) Increment(v volume.Volume) {
211 211
 func (s *volumeStore) Decrement(v volume.Volume) {
212 212
 	s.mu.Lock()
213 213
 	defer s.mu.Unlock()
214
+	logrus.Debugf("Decrementing volume reference: driver %s, name %s", v.DriverName(), v.Name())
214 215
 
215 216
 	vc, exists := s.vols[v.Name()]
216 217
 	if !exists {