Browse code

Move temporary download file to download descriptor scope

This will allow it to be reused between download attempts in a
subsequent commit.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

Aaron Lehmann authored on 2016/01/26 11:20:18
Showing 5 changed files
... ...
@@ -2,7 +2,6 @@ package distribution
2 2
 
3 3
 import (
4 4
 	"fmt"
5
-	"os"
6 5
 
7 6
 	"github.com/Sirupsen/logrus"
8 7
 	"github.com/docker/docker/api"
... ...
@@ -187,16 +186,3 @@ func validateRepoName(name string) error {
187 187
 	}
188 188
 	return nil
189 189
 }
190
-
191
-// tmpFileClose creates a closer function for a temporary file that closes the file
192
-// and also deletes it.
193
-func tmpFileCloser(tmpFile *os.File) func() error {
194
-	return func() error {
195
-		tmpFile.Close()
196
-		if err := os.RemoveAll(tmpFile.Name()); err != nil {
197
-			logrus.Errorf("Failed to remove temp file: %s", tmpFile.Name())
198
-		}
199
-
200
-		return nil
201
-	}
202
-}
... ...
@@ -7,6 +7,7 @@ import (
7 7
 	"io/ioutil"
8 8
 	"net"
9 9
 	"net/url"
10
+	"os"
10 11
 	"strings"
11 12
 	"time"
12 13
 
... ...
@@ -279,6 +280,7 @@ type v1LayerDescriptor struct {
279 279
 	layersDownloaded *bool
280 280
 	layerSize        int64
281 281
 	session          *registry.Session
282
+	tmpFile          *os.File
282 283
 }
283 284
 
284 285
 func (ld *v1LayerDescriptor) Key() string {
... ...
@@ -308,7 +310,7 @@ func (ld *v1LayerDescriptor) Download(ctx context.Context, progressOutput progre
308 308
 	}
309 309
 	*ld.layersDownloaded = true
310 310
 
311
-	tmpFile, err := ioutil.TempFile("", "GetImageBlob")
311
+	ld.tmpFile, err = ioutil.TempFile("", "GetImageBlob")
312 312
 	if err != nil {
313 313
 		layerReader.Close()
314 314
 		return nil, 0, err
... ...
@@ -317,17 +319,28 @@ func (ld *v1LayerDescriptor) Download(ctx context.Context, progressOutput progre
317 317
 	reader := progress.NewProgressReader(ioutils.NewCancelReadCloser(ctx, layerReader), progressOutput, ld.layerSize, ld.ID(), "Downloading")
318 318
 	defer reader.Close()
319 319
 
320
-	_, err = io.Copy(tmpFile, reader)
320
+	_, err = io.Copy(ld.tmpFile, reader)
321 321
 	if err != nil {
322
+		ld.Close()
322 323
 		return nil, 0, err
323 324
 	}
324 325
 
325 326
 	progress.Update(progressOutput, ld.ID(), "Download complete")
326 327
 
327
-	logrus.Debugf("Downloaded %s to tempfile %s", ld.ID(), tmpFile.Name())
328
+	logrus.Debugf("Downloaded %s to tempfile %s", ld.ID(), ld.tmpFile.Name())
328 329
 
329
-	tmpFile.Seek(0, 0)
330
-	return ioutils.NewReadCloserWrapper(tmpFile, tmpFileCloser(tmpFile)), ld.layerSize, nil
330
+	ld.tmpFile.Seek(0, 0)
331
+	return ld.tmpFile, ld.layerSize, nil
332
+}
333
+
334
+func (ld *v1LayerDescriptor) Close() {
335
+	if ld.tmpFile != nil {
336
+		ld.tmpFile.Close()
337
+		if err := os.RemoveAll(ld.tmpFile.Name()); err != nil {
338
+			logrus.Errorf("Failed to remove temp file: %s", ld.tmpFile.Name())
339
+		}
340
+		ld.tmpFile = nil
341
+	}
331 342
 }
332 343
 
333 344
 func (ld *v1LayerDescriptor) Registered(diffID layer.DiffID) {
... ...
@@ -114,6 +114,7 @@ type v2LayerDescriptor struct {
114 114
 	repoInfo          *registry.RepositoryInfo
115 115
 	repo              distribution.Repository
116 116
 	V2MetadataService *metadata.V2MetadataService
117
+	tmpFile           *os.File
117 118
 }
118 119
 
119 120
 func (ld *v2LayerDescriptor) Key() string {
... ...
@@ -131,6 +132,18 @@ func (ld *v2LayerDescriptor) DiffID() (layer.DiffID, error) {
131 131
 func (ld *v2LayerDescriptor) Download(ctx context.Context, progressOutput progress.Output) (io.ReadCloser, int64, error) {
132 132
 	logrus.Debugf("pulling blob %q", ld.digest)
133 133
 
134
+	var err error
135
+
136
+	if ld.tmpFile == nil {
137
+		ld.tmpFile, err = createDownloadFile()
138
+	} else {
139
+		_, err = ld.tmpFile.Seek(0, os.SEEK_SET)
140
+	}
141
+	if err != nil {
142
+		return nil, 0, xfer.DoNotRetry{Err: err}
143
+	}
144
+
145
+	tmpFile := ld.tmpFile
134 146
 	blobs := ld.repo.Blobs(ctx)
135 147
 
136 148
 	layerDownload, err := blobs.Open(ctx, ld.digest)
... ...
@@ -164,17 +177,13 @@ func (ld *v2LayerDescriptor) Download(ctx context.Context, progressOutput progre
164 164
 		return nil, 0, xfer.DoNotRetry{Err: err}
165 165
 	}
166 166
 
167
-	tmpFile, err := ioutil.TempFile("", "GetImageBlob")
168
-	if err != nil {
169
-		return nil, 0, xfer.DoNotRetry{Err: err}
170
-	}
171
-
172 167
 	_, err = io.Copy(tmpFile, io.TeeReader(reader, verifier))
173 168
 	if err != nil {
174 169
 		tmpFile.Close()
175 170
 		if err := os.Remove(tmpFile.Name()); err != nil {
176 171
 			logrus.Errorf("Failed to remove temp file: %s", tmpFile.Name())
177 172
 		}
173
+		ld.tmpFile = nil
178 174
 		return nil, 0, retryOnError(err)
179 175
 	}
180 176
 
... ...
@@ -188,6 +197,7 @@ func (ld *v2LayerDescriptor) Download(ctx context.Context, progressOutput progre
188 188
 		if err := os.Remove(tmpFile.Name()); err != nil {
189 189
 			logrus.Errorf("Failed to remove temp file: %s", tmpFile.Name())
190 190
 		}
191
+		ld.tmpFile = nil
191 192
 
192 193
 		return nil, 0, xfer.DoNotRetry{Err: err}
193 194
 	}
... ...
@@ -202,9 +212,19 @@ func (ld *v2LayerDescriptor) Download(ctx context.Context, progressOutput progre
202 202
 		if err := os.Remove(tmpFile.Name()); err != nil {
203 203
 			logrus.Errorf("Failed to remove temp file: %s", tmpFile.Name())
204 204
 		}
205
+		ld.tmpFile = nil
205 206
 		return nil, 0, xfer.DoNotRetry{Err: err}
206 207
 	}
207
-	return ioutils.NewReadCloserWrapper(tmpFile, tmpFileCloser(tmpFile)), size, nil
208
+	return tmpFile, size, nil
209
+}
210
+
211
+func (ld *v2LayerDescriptor) Close() {
212
+	if ld.tmpFile != nil {
213
+		ld.tmpFile.Close()
214
+		if err := os.RemoveAll(ld.tmpFile.Name()); err != nil {
215
+			logrus.Errorf("Failed to remove temp file: %s", ld.tmpFile.Name())
216
+		}
217
+	}
208 218
 }
209 219
 
210 220
 func (ld *v2LayerDescriptor) Registered(diffID layer.DiffID) {
... ...
@@ -711,3 +731,7 @@ func fixManifestLayers(m *schema1.Manifest) error {
711 711
 
712 712
 	return nil
713 713
 }
714
+
715
+func createDownloadFile() (*os.File, error) {
716
+	return ioutil.TempFile("", "GetImageBlob")
717
+}
... ...
@@ -59,6 +59,10 @@ type DownloadDescriptor interface {
59 59
 	DiffID() (layer.DiffID, error)
60 60
 	// Download is called to perform the download.
61 61
 	Download(ctx context.Context, progressOutput progress.Output) (io.ReadCloser, int64, error)
62
+	// Close is called when the download manager is finished with this
63
+	// descriptor and will not call Download again or read from the reader
64
+	// that Download returned.
65
+	Close()
62 66
 }
63 67
 
64 68
 // DownloadDescriptorWithRegistered is a DownloadDescriptor that has an
... ...
@@ -229,6 +233,8 @@ func (ldm *LayerDownloadManager) makeDownloadFunc(descriptor DownloadDescriptor,
229 229
 				retries        int
230 230
 			)
231 231
 
232
+			defer descriptor.Close()
233
+
232 234
 			for {
233 235
 				downloadReader, size, err = descriptor.Download(d.Transfer.Context(), progressOutput)
234 236
 				if err == nil {
... ...
@@ -199,6 +199,9 @@ func (d *mockDownloadDescriptor) Download(ctx context.Context, progressOutput pr
199 199
 	return d.mockTarStream(), 0, nil
200 200
 }
201 201
 
202
+func (d *mockDownloadDescriptor) Close() {
203
+}
204
+
202 205
 func downloadDescriptors(currentDownloads *int32) []DownloadDescriptor {
203 206
 	return []DownloadDescriptor{
204 207
 		&mockDownloadDescriptor{