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>
This commit is contained in:
Sebastiaan van Stijn 2022-01-21 13:34:37 +01:00
parent 0b0a995d9d
commit bb66ebd621
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C
5 changed files with 14 additions and 22 deletions

View File

@ -6,7 +6,6 @@ import (
"fmt"
"io"
"path"
"runtime"
"sync"
"time"
@ -563,7 +562,7 @@ func (p *puller) Snapshot(ctx context.Context, g session.Group) (cache.Immutable
}()
r := image.NewRootFS()
rootFS, release, err := p.is.DownloadManager.Download(ctx, *r, runtime.GOOS, layers, pkgprogress.ChanOutput(pchan))
rootFS, release, err := p.is.DownloadManager.Download(ctx, *r, layers, pkgprogress.ChanOutput(pchan))
stopProgress()
if err != nil {
return nil, err

View File

@ -5,7 +5,6 @@ import (
"fmt"
"io"
nethttp "net/http"
"runtime"
"strings"
"time"
@ -356,7 +355,7 @@ func (w *Worker) FromRemote(ctx context.Context, remote *solver.Remote) (cache.I
}()
r := image.NewRootFS()
rootFS, release, err := w.DownloadManager.Download(ctx, *r, runtime.GOOS, layers, &discardProgress{})
rootFS, release, err := w.DownloadManager.Download(ctx, *r, layers, &discardProgress{})
if err != nil {
return nil, err
}

View File

@ -547,7 +547,7 @@ func (p *v2Puller) pullSchema1(ctx context.Context, ref reference.Reference, unv
descriptors = append(descriptors, layerDescriptor)
}
resultRootFS, release, err := p.config.DownloadManager.Download(ctx, *rootFS, runtime.GOOS, descriptors, p.config.ProgressOutput)
resultRootFS, release, err := p.config.DownloadManager.Download(ctx, *rootFS, descriptors, p.config.ProgressOutput)
if err != nil {
return "", "", err
}
@ -665,6 +665,12 @@ func (p *v2Puller) pullSchema2Layers(ctx context.Context, target distribution.De
}
}
// Assume that the operating system is the host OS if blank, and validate it
// to ensure we don't cause a panic by an invalid index into the layerstores.
if layerStoreOS != "" && !system.IsOSSupported(layerStoreOS) {
return "", system.ErrNotSupportedOperatingSystem
}
if p.config.DownloadManager != nil {
go func() {
var (
@ -672,7 +678,7 @@ func (p *v2Puller) pullSchema2Layers(ctx context.Context, target distribution.De
rootFS image.RootFS
)
downloadRootFS := *image.NewRootFS()
rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, layerStoreOS, descriptors, p.config.ProgressOutput)
rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput)
if err != nil {
// Intentionally do not cancel the config download here
// as the error from config download (if there is one)

View File

@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"io"
"runtime"
"time"
"github.com/docker/distribution"
@ -14,7 +13,6 @@ import (
"github.com/docker/docker/pkg/archive"
"github.com/docker/docker/pkg/ioutils"
"github.com/docker/docker/pkg/progress"
"github.com/docker/docker/pkg/system"
"github.com/sirupsen/logrus"
)
@ -106,7 +104,7 @@ type DownloadDescriptorWithRegistered interface {
// Download method is called to get the layer tar data. Layers are then
// registered in the appropriate order. The caller must call the returned
// release function once it is done with the returned RootFS object.
func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS image.RootFS, os string, layers []DownloadDescriptor, progressOutput progress.Output) (image.RootFS, func(), error) {
func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS image.RootFS, layers []DownloadDescriptor, progressOutput progress.Output) (image.RootFS, func(), error) {
var (
topLayer layer.Layer
topDownload *downloadTransfer
@ -116,16 +114,6 @@ func (ldm *LayerDownloadManager) Download(ctx context.Context, initialRootFS ima
downloadsByKey = make(map[string]*downloadTransfer)
)
// Assume that the operating system is the host OS if blank, and validate it
// to ensure we don't cause a panic by an invalid index into the layerstores.
// TODO remove now that LCOW is no longer a thing
if os == "" {
os = runtime.GOOS
}
if !system.IsOSSupported(os) {
return image.RootFS{}, nil, system.ErrNotSupportedOperatingSystem
}
rootFS := initialRootFS
for _, descriptor := range layers {
key := descriptor.Key()

View File

@ -293,7 +293,7 @@ func TestSuccessfulDownload(t *testing.T) {
}
firstDescriptor.diffID = l.DiffID()
rootFS, releaseFunc, err := ldm.Download(context.Background(), *image.NewRootFS(), runtime.GOOS, descriptors, progress.ChanOutput(progressChan))
rootFS, releaseFunc, err := ldm.Download(context.Background(), *image.NewRootFS(), descriptors, progress.ChanOutput(progressChan))
if err != nil {
t.Fatalf("download error: %v", err)
}
@ -348,7 +348,7 @@ func TestCancelledDownload(t *testing.T) {
}()
descriptors := downloadDescriptors(nil)
_, _, err := ldm.Download(ctx, *image.NewRootFS(), runtime.GOOS, descriptors, progress.ChanOutput(progressChan))
_, _, err := ldm.Download(ctx, *image.NewRootFS(), descriptors, progress.ChanOutput(progressChan))
if err != context.Canceled {
t.Fatal("expected download to be cancelled")
}
@ -412,7 +412,7 @@ func TestMaxDownloadAttempts(t *testing.T) {
descriptors := downloadDescriptors(&currentDownloads)
descriptors[4].(*mockDownloadDescriptor).simulateRetries = tc.simulateRetries
_, _, err := ldm.Download(context.Background(), *image.NewRootFS(), runtime.GOOS, descriptors, progress.ChanOutput(progressChan))
_, _, err := ldm.Download(context.Background(), *image.NewRootFS(), descriptors, progress.ChanOutput(progressChan))
if tc.expectedErr == "" {
assert.NilError(t, err)
} else {