diff --git a/distribution/config.go b/distribution/config.go index ab1ab6c003..9e652ee970 100644 --- a/distribution/config.go +++ b/distribution/config.go @@ -142,9 +142,12 @@ func (s *imageConfigStore) RootFSFromConfig(c []byte) (*image.RootFS, error) { return nil, err } - // fail immediately on windows + // fail immediately on Windows when downloading a non-Windows image + // and vice versa if runtime.GOOS == "windows" && unmarshalledConfig.OS == "linux" { return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS) + } else if runtime.GOOS != "windows" && unmarshalledConfig.OS == "windows" { + return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS) } return unmarshalledConfig.RootFS, nil diff --git a/distribution/pull_v2.go b/distribution/pull_v2.go index e707e7d3b6..70faa8e367 100644 --- a/distribution/pull_v2.go +++ b/distribution/pull_v2.go @@ -533,15 +533,18 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s } configChan := make(chan []byte, 1) - errChan := make(chan error, 1) + configErrChan := make(chan error, 1) + layerErrChan := make(chan error, 1) + downloadsDone := make(chan struct{}) var cancel func() ctx, cancel = context.WithCancel(ctx) + defer cancel() // Pull the image config go func() { configJSON, err := p.pullSchema2Config(ctx, target.Digest) if err != nil { - errChan <- ImageConfigPullError{Err: err} + configErrChan <- ImageConfigPullError{Err: err} cancel() return } @@ -552,6 +555,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s configJSON []byte // raw serialized image config downloadedRootFS *image.RootFS // rootFS from registered layers configRootFS *image.RootFS // rootFS from configuration + release func() // release resources from rootFS download ) // https://github.com/docker/docker/issues/24766 - Err on the side of caution, @@ -563,7 +567,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s // check to block Windows images being pulled on Linux is implemented, it // may be necessary to perform the same type of serialisation. if runtime.GOOS == "windows" { - configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, errChan) + configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan) if err != nil { return "", "", err } @@ -574,41 +578,52 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s } if p.config.DownloadManager != nil { - downloadRootFS := *image.NewRootFS() - rootFS, release, err := p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput) - if err != nil { - if configJSON != nil { - // Already received the config - return "", "", err + go func() { + var ( + err error + rootFS image.RootFS + ) + downloadRootFS := *image.NewRootFS() + 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) + // is more interesting than the layer download error + layerErrChan <- err + return } - select { - case err = <-errChan: - return "", "", err - default: - cancel() - select { - case <-configChan: - case <-errChan: - } - return "", "", err - } - } - if release != nil { - defer release() - } - downloadedRootFS = &rootFS + downloadedRootFS = &rootFS + close(downloadsDone) + }() + } else { + // We have nothing to download + close(downloadsDone) } if configJSON == nil { - configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, errChan) + configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan) + if err == nil && configRootFS == nil { + err = errRootFSInvalid + } if err != nil { + cancel() + select { + case <-downloadsDone: + case <-layerErrChan: + } return "", "", err } + } - if configRootFS == nil { - return "", "", errRootFSInvalid - } + select { + case <-downloadsDone: + case err = <-layerErrChan: + return "", "", err + } + + if release != nil { + defer release() } if downloadedRootFS != nil { diff --git a/integration-cli/docker_cli_pull_test.go b/integration-cli/docker_cli_pull_test.go index 40da4c1feb..0b1be6cd97 100644 --- a/integration-cli/docker_cli_pull_test.go +++ b/integration-cli/docker_cli_pull_test.go @@ -272,3 +272,10 @@ func (s *DockerSuite) TestPullLinuxImageFailsOnWindows(c *check.C) { _, _, err := dockerCmdWithError("pull", "ubuntu") c.Assert(err.Error(), checker.Contains, "cannot be used on this platform") } + +// Regression test for https://github.com/docker/docker/issues/28892 +func (s *DockerSuite) TestPullWindowsImageFailsOnLinux(c *check.C) { + testRequires(c, DaemonIsLinux, Network) + _, _, err := dockerCmdWithError("pull", "microsoft/nanoserver") + c.Assert(err.Error(), checker.Contains, "cannot be used on this platform") +}