Signed-off-by: Darren Stahl <darst@microsoft.com>
| ... | ... |
@@ -142,9 +142,12 @@ func (s *imageConfigStore) RootFSFromConfig(c []byte) (*image.RootFS, error) {
|
| 142 | 142 |
return nil, err |
| 143 | 143 |
} |
| 144 | 144 |
|
| 145 |
- // fail immediately on windows |
|
| 145 |
+ // fail immediately on Windows when downloading a non-Windows image |
|
| 146 |
+ // and vice versa |
|
| 146 | 147 |
if runtime.GOOS == "windows" && unmarshalledConfig.OS == "linux" {
|
| 147 | 148 |
return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
|
| 149 |
+ } else if runtime.GOOS != "windows" && unmarshalledConfig.OS == "windows" {
|
|
| 150 |
+ return nil, fmt.Errorf("image operating system %q cannot be used on this platform", unmarshalledConfig.OS)
|
|
| 148 | 151 |
} |
| 149 | 152 |
|
| 150 | 153 |
return unmarshalledConfig.RootFS, nil |
| ... | ... |
@@ -533,15 +533,18 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s |
| 533 | 533 |
} |
| 534 | 534 |
|
| 535 | 535 |
configChan := make(chan []byte, 1) |
| 536 |
- errChan := make(chan error, 1) |
|
| 536 |
+ configErrChan := make(chan error, 1) |
|
| 537 |
+ layerErrChan := make(chan error, 1) |
|
| 538 |
+ downloadsDone := make(chan struct{})
|
|
| 537 | 539 |
var cancel func() |
| 538 | 540 |
ctx, cancel = context.WithCancel(ctx) |
| 541 |
+ defer cancel() |
|
| 539 | 542 |
|
| 540 | 543 |
// Pull the image config |
| 541 | 544 |
go func() {
|
| 542 | 545 |
configJSON, err := p.pullSchema2Config(ctx, target.Digest) |
| 543 | 546 |
if err != nil {
|
| 544 |
- errChan <- ImageConfigPullError{Err: err}
|
|
| 547 |
+ configErrChan <- ImageConfigPullError{Err: err}
|
|
| 545 | 548 |
cancel() |
| 546 | 549 |
return |
| 547 | 550 |
} |
| ... | ... |
@@ -552,6 +555,7 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s |
| 552 | 552 |
configJSON []byte // raw serialized image config |
| 553 | 553 |
downloadedRootFS *image.RootFS // rootFS from registered layers |
| 554 | 554 |
configRootFS *image.RootFS // rootFS from configuration |
| 555 |
+ release func() // release resources from rootFS download |
|
| 555 | 556 |
) |
| 556 | 557 |
|
| 557 | 558 |
// 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 |
| 563 | 563 |
// check to block Windows images being pulled on Linux is implemented, it |
| 564 | 564 |
// may be necessary to perform the same type of serialisation. |
| 565 | 565 |
if runtime.GOOS == "windows" {
|
| 566 |
- configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, errChan) |
|
| 566 |
+ configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan) |
|
| 567 | 567 |
if err != nil {
|
| 568 | 568 |
return "", "", err |
| 569 | 569 |
} |
| ... | ... |
@@ -574,41 +578,52 @@ func (p *v2Puller) pullSchema2(ctx context.Context, ref reference.Named, mfst *s |
| 574 | 574 |
} |
| 575 | 575 |
|
| 576 | 576 |
if p.config.DownloadManager != nil {
|
| 577 |
- downloadRootFS := *image.NewRootFS() |
|
| 578 |
- rootFS, release, err := p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput) |
|
| 579 |
- if err != nil {
|
|
| 580 |
- if configJSON != nil {
|
|
| 581 |
- // Already received the config |
|
| 582 |
- return "", "", err |
|
| 583 |
- } |
|
| 584 |
- select {
|
|
| 585 |
- case err = <-errChan: |
|
| 586 |
- return "", "", err |
|
| 587 |
- default: |
|
| 588 |
- cancel() |
|
| 589 |
- select {
|
|
| 590 |
- case <-configChan: |
|
| 591 |
- case <-errChan: |
|
| 592 |
- } |
|
| 593 |
- return "", "", err |
|
| 577 |
+ go func() {
|
|
| 578 |
+ var ( |
|
| 579 |
+ err error |
|
| 580 |
+ rootFS image.RootFS |
|
| 581 |
+ ) |
|
| 582 |
+ downloadRootFS := *image.NewRootFS() |
|
| 583 |
+ rootFS, release, err = p.config.DownloadManager.Download(ctx, downloadRootFS, descriptors, p.config.ProgressOutput) |
|
| 584 |
+ if err != nil {
|
|
| 585 |
+ // Intentionally do not cancel the config download here |
|
| 586 |
+ // as the error from config download (if there is one) |
|
| 587 |
+ // is more interesting than the layer download error |
|
| 588 |
+ layerErrChan <- err |
|
| 589 |
+ return |
|
| 594 | 590 |
} |
| 595 |
- } |
|
| 596 |
- if release != nil {
|
|
| 597 |
- defer release() |
|
| 598 |
- } |
|
| 599 | 591 |
|
| 600 |
- downloadedRootFS = &rootFS |
|
| 592 |
+ downloadedRootFS = &rootFS |
|
| 593 |
+ close(downloadsDone) |
|
| 594 |
+ }() |
|
| 595 |
+ } else {
|
|
| 596 |
+ // We have nothing to download |
|
| 597 |
+ close(downloadsDone) |
|
| 601 | 598 |
} |
| 602 | 599 |
|
| 603 | 600 |
if configJSON == nil {
|
| 604 |
- configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, errChan) |
|
| 601 |
+ configJSON, configRootFS, err = receiveConfig(p.config.ImageStore, configChan, configErrChan) |
|
| 602 |
+ if err == nil && configRootFS == nil {
|
|
| 603 |
+ err = errRootFSInvalid |
|
| 604 |
+ } |
|
| 605 | 605 |
if err != nil {
|
| 606 |
+ cancel() |
|
| 607 |
+ select {
|
|
| 608 |
+ case <-downloadsDone: |
|
| 609 |
+ case <-layerErrChan: |
|
| 610 |
+ } |
|
| 606 | 611 |
return "", "", err |
| 607 | 612 |
} |
| 613 |
+ } |
|
| 608 | 614 |
|
| 609 |
- if configRootFS == nil {
|
|
| 610 |
- return "", "", errRootFSInvalid |
|
| 611 |
- } |
|
| 615 |
+ select {
|
|
| 616 |
+ case <-downloadsDone: |
|
| 617 |
+ case err = <-layerErrChan: |
|
| 618 |
+ return "", "", err |
|
| 619 |
+ } |
|
| 620 |
+ |
|
| 621 |
+ if release != nil {
|
|
| 622 |
+ defer release() |
|
| 612 | 623 |
} |
| 613 | 624 |
|
| 614 | 625 |
if downloadedRootFS != nil {
|
| ... | ... |
@@ -272,3 +272,10 @@ func (s *DockerSuite) TestPullLinuxImageFailsOnWindows(c *check.C) {
|
| 272 | 272 |
_, _, err := dockerCmdWithError("pull", "ubuntu")
|
| 273 | 273 |
c.Assert(err.Error(), checker.Contains, "cannot be used on this platform") |
| 274 | 274 |
} |
| 275 |
+ |
|
| 276 |
+// Regression test for https://github.com/docker/docker/issues/28892 |
|
| 277 |
+func (s *DockerSuite) TestPullWindowsImageFailsOnLinux(c *check.C) {
|
|
| 278 |
+ testRequires(c, DaemonIsLinux, Network) |
|
| 279 |
+ _, _, err := dockerCmdWithError("pull", "microsoft/nanoserver")
|
|
| 280 |
+ c.Assert(err.Error(), checker.Contains, "cannot be used on this platform") |
|
| 281 |
+} |