Skip to content

Commit bb66ebd

Browse files
thaJeztahthaJeztah
committed
distribution: xfer.LayerDownloadManager.Download(): remove "os" argument
This argument was added for LCOW support, but it was only used to verify if the passed platform (OS) matched the host. Given that all uses of this function (except for one) passed runtime.GOOS, we may as well move the check to that location. We should do more cleaning up after this, and perform such validations early, instead of passing platform around in too many places where it's only used for similar validations. This is a first step in that direction. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent 0b0a995 commit bb66ebd

File tree

5 files changed

+14
-22
lines changed

5 files changed

+14
-22
lines changed

‎builder/builder-next/adapters/containerimage/pull.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"io"
88
"path"
9-
"runtime"
109
"sync"
1110
"time"
1211

@@ -563,7 +562,7 @@ func (p *puller) Snapshot(ctx context.Context, g session.Group) (cache.Immutable
563562
}()
564563

565564
r := image.NewRootFS()
566-
rootFS, release, err := p.is.DownloadManager.Download(ctx, *r, runtime.GOOS, layers, pkgprogress.ChanOutput(pchan))
565+
rootFS, release, err := p.is.DownloadManager.Download(ctx, *r, layers, pkgprogress.ChanOutput(pchan))
567566
stopProgress()
568567
if err != nil {
569568
return nil, err

‎builder/builder-next/worker/worker.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"io"
77
nethttp "net/http"
8-
"runtime"
98
"strings"
109
"time"
1110

@@ -356,7 +355,7 @@ func (w *Worker) FromRemote(ctx context.Context, remote *solver.Remote) (cache.I
356355
}()
357356

358357
r := image.NewRootFS()
359-
rootFS, release, err := w.DownloadManager.Download(ctx, *r, runtime.GOOS, layers, &discardProgress{})
358+
rootFS, release, err := w.DownloadManager.Download(ctx, *r, layers, &discardProgress{})
360359
if err != nil {
361360
return nil, err
362361
}

‎distribution/pull_v2.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ func (p *v2Puller) pullSchema1(ctx context.Context, ref reference.Reference, unv
547547
descriptors = append(descriptors, layerDescriptor)
548548
}
549549

550-
resultRootFS, release, err := p.config.DownloadManager.Download(ctx, *rootFS, runtime.GOOS, descriptors, p.config.ProgressOutput)
550+
resultRootFS, release, err := p.config.DownloadManager.Download(ctx, *rootFS, descriptors, p.config.ProgressOutput)
551551
if err != nil {
552552
return "", "", err
553553
}
@@ -665,14 +665,20 @@ func (p *v2Puller) pullSchema2Layers(ctx context.Context, target distribution.De
665665
}
666666
}
667667

668+
// Assume that the operating system is the host OS if blank, and validate it
669+
// to ensure we don't cause a panic by an invalid index into the layerstores.
670+
if layerStoreOS != "" && !system.IsOSSupported(layerStoreOS) {
671+
return "", system.ErrNotSupportedOperatingSystem
672+
}
673+
668674
if p.config.DownloadManager != nil {
669675
go func() {
670676
var (
671677
err error
672678
rootFS image.RootFS
673679
)
674680
downloadRootFS := *image.NewRootFS()
675-
rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, layerStoreOS, descriptors, p.config.ProgressOutput)
681+
rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput)
676682
if err != nil {
677683
// Intentionally do not cancel the config download here
678684
// as the error from config download (if there is one)

‎distribution/xfer/download.go‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"errors"
66
"fmt"
77
"io"
8-
"runtime"
98
"time"
109

1110
"github.com/docker/distribution"
@@ -14,7 +13,6 @@ import (
1413
"github.com/docker/docker/pkg/archive"
1514
"github.com/docker/docker/pkg/ioutils"
1615
"github.com/docker/docker/pkg/progress"
17-
"github.com/docker/docker/pkg/system"
1816
"github.com/sirupsen/logrus"
1917
)
2018

@@ -106,7 +104,7 @@ type DownloadDescriptorWithRegistered interface {
106104
// Download method is called to get the layer tar data. Layers are then
107105
// registered in the appropriate order. The caller must call the returned
108106
// release function once it is done with the returned RootFS object.
109-
func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS image.RootFS, os string, layers []DownloadDescriptor, progressOutput progress.Output) (image.RootFS, func(), error) {
107+
func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS image.RootFS, layers []DownloadDescriptor, progressOutput progress.Output) (image.RootFS, func(), error) {
110108
var (
111109
topLayer layer.Layer
112110
topDownload *downloadTransfer
@@ -116,16 +114,6 @@ func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS ima
116114
downloadsByKey = make(map[string]*downloadTransfer)
117115
)
118116

119-
// Assume that the operating system is the host OS if blank, and validate it
120-
// to ensure we don't cause a panic by an invalid index into the layerstores.
121-
// TODO remove now that LCOW is no longer a thing
122-
if os == "" {
123-
os = runtime.GOOS
124-
}
125-
if !system.IsOSSupported(os) {
126-
return image.RootFS{}, nil, system.ErrNotSupportedOperatingSystem
127-
}
128-
129117
rootFS := initialRootFS
130118
for _, descriptor := range layers {
131119
key := descriptor.Key()

‎distribution/xfer/download_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func TestSuccessfulDownload(t *testing.T) {
293293
}
294294
firstDescriptor.diffID = l.DiffID()
295295

296-
rootFS, releaseFunc, err := ldm.Download(context.Background(), *image.NewRootFS(), runtime.GOOS, descriptors, progress.ChanOutput(progressChan))
296+
rootFS, releaseFunc, err := ldm.Download(context.Background(), *image.NewRootFS(), descriptors, progress.ChanOutput(progressChan))
297297
if err != nil {
298298
t.Fatalf("download error: %v", err)
299299
}
@@ -348,7 +348,7 @@ func TestCancelledDownload(t *testing.T) {
348348
}()
349349

350350
descriptors := downloadDescriptors(nil)
351-
_, _, err := ldm.Download(ctx, *image.NewRootFS(), runtime.GOOS, descriptors, progress.ChanOutput(progressChan))
351+
_, _, err := ldm.Download(ctx, *image.NewRootFS(), descriptors, progress.ChanOutput(progressChan))
352352
if err != context.Canceled {
353353
t.Fatal("expected download to be cancelled")
354354
}
@@ -412,7 +412,7 @@ func TestMaxDownloadAttempts(t *testing.T) {
412412
descriptors := downloadDescriptors(&currentDownloads)
413413
descriptors[4].(*mockDownloadDescriptor).simulateRetries = tc.simulateRetries
414414

415-
_, _, err := ldm.Download(context.Background(), *image.NewRootFS(), runtime.GOOS, descriptors, progress.ChanOutput(progressChan))
415+
_, _, err := ldm.Download(context.Background(), *image.NewRootFS(), descriptors, progress.ChanOutput(progressChan))
416416
if tc.expectedErr == "" {
417417
assert.NilError(t, err)
418418
} else {

0 commit comments

Comments
 (0)