diff --git a/distribution/xfer/download.go b/distribution/xfer/download.go index 32f2338e96..e77c564513 100644 --- a/distribution/xfer/download.go +++ b/distribution/xfer/download.go @@ -267,7 +267,7 @@ func (ldm *LayerDownloadManager) makeDownloadFunc(descriptor DownloadDescriptor, downloadReader io.ReadCloser size int64 err error - retries int + attempt int = 1 ) defer descriptor.Close() @@ -287,16 +287,16 @@ func (ldm *LayerDownloadManager) makeDownloadFunc(descriptor DownloadDescriptor, default: } - retries++ - if _, isDNR := err.(DoNotRetry); isDNR || retries > ldm.maxDownloadAttempts { - logrus.Errorf("Download failed after %d attempts: %v", retries, err) + if _, isDNR := err.(DoNotRetry); isDNR || attempt >= ldm.maxDownloadAttempts { + logrus.Errorf("Download failed after %d attempts: %v", attempt, err) d.err = err return } - logrus.Infof("Download failed, retrying (%d/%d): %v", retries, ldm.maxDownloadAttempts, err) - delay := retries * 5 + logrus.Infof("Download failed, retrying (%d/%d): %v", attempt, ldm.maxDownloadAttempts, err) + delay := attempt * 5 ticker := time.NewTicker(ldm.waitDuration) + attempt++ selectLoop: for { diff --git a/distribution/xfer/download_test.go b/distribution/xfer/download_test.go index 3b4a6a9df9..f45598517d 100644 --- a/distribution/xfer/download_test.go +++ b/distribution/xfer/download_test.go @@ -216,7 +216,7 @@ func (d *mockDownloadDescriptor) Download(ctx context.Context, progressOutput pr if d.retries < d.simulateRetries { d.retries++ - return nil, 0, fmt.Errorf("simulating download attempt %d/%d", d.retries, d.simulateRetries) + return nil, 0, fmt.Errorf("simulating download attempt failure %d/%d", d.retries, d.simulateRetries) } return d.mockTarStream(), 0, nil @@ -367,28 +367,29 @@ func TestMaxDownloadAttempts(t *testing.T) { }{ { name: "max-attempts=5, succeed at 2nd attempt", - simulateRetries: 2, + simulateRetries: 1, maxDownloadAttempts: 5, }, { name: "max-attempts=5, succeed at 5th attempt", + simulateRetries: 4, + maxDownloadAttempts: 5, + }, + { + name: "max-attempts=5, fail at 5th attempt", simulateRetries: 5, maxDownloadAttempts: 5, + expectedErr: "simulating download attempt failure 5/5", }, { - name: "max-attempts=5, fail at 6th attempt", - simulateRetries: 6, - maxDownloadAttempts: 5, - expectedErr: "simulating download attempt 5/6", - }, - { - name: "max-attempts=0, fail after 1 attempt", + name: "max-attempts=1, fail after 1 attempt", simulateRetries: 1, - maxDownloadAttempts: 0, - expectedErr: "simulating download attempt 1/1", + maxDownloadAttempts: 1, + expectedErr: "simulating download attempt failure 1/1", }, } for _, tc := range tests { + tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() layerStore := &mockLayerStore{make(map[layer.ChainID]*mockLayer)} diff --git a/integration/build/build_test.go b/integration/build/build_test.go index 5da3a294a1..589ebc0a77 100644 --- a/integration/build/build_test.go +++ b/integration/build/build_test.go @@ -90,6 +90,7 @@ func TestBuildWithRemoveAndForceRemove(t *testing.T) { client := testEnv.APIClient() ctx := context.Background() for _, c := range cases { + c := c t.Run(c.name, func(t *testing.T) { t.Parallel() dockerfile := []byte(c.dockerfile)