From 21e53b4643f2c6d07d6f7e2ddbd04ff7037012cc Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 3 Oct 2017 16:58:07 -0700 Subject: [PATCH 1/2] Add support for Windows version filtering on pull Update logic to choose manifest from manifest list to check for os version on Windows. Separate the logic for windows and unix to keep unix logic the same. Signed-off-by: Derek McGowan (cherry picked from commit 38aef56e1fcb8ea318df98c89cf002267b88a136) Signed-off-by: John Stephens --- components/engine/distribution/pull_v2.go | 25 +++----- .../engine/distribution/pull_v2_unix.go | 16 +++++ .../engine/distribution/pull_v2_windows.go | 60 +++++++++++++++++++ 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/components/engine/distribution/pull_v2.go b/components/engine/distribution/pull_v2.go index 39bf782495..d1d5f5c8b8 100644 --- a/components/engine/distribution/pull_v2.go +++ b/components/engine/distribution/pull_v2.go @@ -708,29 +708,20 @@ func (p *v2Puller) pullManifestList(ctx context.Context, ref reference.Named, mf } logrus.Debugf("%s resolved to a manifestList object with %d entries; looking for a os/arch match", ref, len(mfstList.Manifests)) - var manifestDigest digest.Digest - // TODO @jhowardmsft LCOW Support: Need to remove the hard coding in LCOW mode. - lookingForOS := runtime.GOOS - if system.LCOWSupported() { - lookingForOS = "linux" - } - for _, manifestDescriptor := range mfstList.Manifests { - // TODO(aaronl): The manifest list spec supports optional - // "features" and "variant" fields. These are not yet used. - // Once they are, their values should be interpreted here. - if manifestDescriptor.Platform.Architecture == runtime.GOARCH && manifestDescriptor.Platform.OS == lookingForOS { - manifestDigest = manifestDescriptor.Digest - logrus.Debugf("found match for %s/%s with media type %s, digest %s", runtime.GOOS, runtime.GOARCH, manifestDescriptor.MediaType, manifestDigest.String()) - break - } - } - if manifestDigest == "" { + manifestMatches := filterManifests(mfstList.Manifests) + + if len(manifestMatches) == 0 { errMsg := fmt.Sprintf("no matching manifest for %s/%s in the manifest list entries", runtime.GOOS, runtime.GOARCH) logrus.Debugf(errMsg) return "", "", errors.New(errMsg) } + if len(manifestMatches) > 1 { + logrus.Debugf("found multiple matches in manifest list, choosing best match %s", manifestMatches[0].Digest.String()) + } + manifestDigest := manifestMatches[0].Digest + manSvc, err := p.repo.Manifests(ctx) if err != nil { return "", "", err diff --git a/components/engine/distribution/pull_v2_unix.go b/components/engine/distribution/pull_v2_unix.go index 45a7a0c150..60d8605fb8 100644 --- a/components/engine/distribution/pull_v2_unix.go +++ b/components/engine/distribution/pull_v2_unix.go @@ -3,11 +3,27 @@ package distribution import ( + "runtime" + "github.com/docker/distribution" "github.com/docker/distribution/context" + "github.com/docker/distribution/manifest/manifestlist" + "github.com/sirupsen/logrus" ) func (ld *v2LayerDescriptor) open(ctx context.Context) (distribution.ReadSeekCloser, error) { blobs := ld.repo.Blobs(ctx) return blobs.Open(ctx, ld.digest) } + +func filterManifests(manifests []manifestlist.ManifestDescriptor) []manifestlist.ManifestDescriptor { + var matches []manifestlist.ManifestDescriptor + for _, manifestDescriptor := range manifests { + if manifestDescriptor.Platform.Architecture == runtime.GOARCH && manifestDescriptor.Platform.OS == runtime.GOOS { + matches = append(matches, manifestDescriptor) + + logrus.Debugf("found match for %s/%s with media type %s, digest %s", runtime.GOOS, runtime.GOARCH, manifestDescriptor.MediaType, manifestDescriptor.Digest.String()) + } + } + return matches +} diff --git a/components/engine/distribution/pull_v2_windows.go b/components/engine/distribution/pull_v2_windows.go index e10070d53a..e94109484d 100644 --- a/components/engine/distribution/pull_v2_windows.go +++ b/components/engine/distribution/pull_v2_windows.go @@ -3,13 +3,19 @@ package distribution import ( + "fmt" "net/http" "os" + "runtime" + "sort" + "strings" "github.com/docker/distribution" "github.com/docker/distribution/context" + "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/schema2" "github.com/docker/distribution/registry/client/transport" + "github.com/docker/docker/pkg/system" "github.com/sirupsen/logrus" ) @@ -55,3 +61,57 @@ func (ld *v2LayerDescriptor) open(ctx context.Context) (distribution.ReadSeekClo } return rsc, err } + +func filterManifests(manifests []manifestlist.ManifestDescriptor) []manifestlist.ManifestDescriptor { + version := system.GetOSVersion() + + // TODO @jhowardmsft LCOW Support: Need to remove the hard coding in LCOW mode. + lookingForOS := runtime.GOOS + osVersion := fmt.Sprintf("%d.%d.%d", version.MajorVersion, version.MinorVersion, version.Build) + if system.LCOWSupported() { + lookingForOS = "linux" + osVersion = "" + } + + var matches []manifestlist.ManifestDescriptor + for _, manifestDescriptor := range manifests { + if manifestDescriptor.Platform.Architecture == runtime.GOARCH && manifestDescriptor.Platform.OS == lookingForOS { + if !versionMatch(manifestDescriptor.Platform.OSVersion, osVersion) { + continue + } + matches = append(matches, manifestDescriptor) + + logrus.Debugf("found match for %s/%s with media type %s, digest %s", runtime.GOOS, runtime.GOARCH, manifestDescriptor.MediaType, manifestDescriptor.Digest.String()) + } + } + sort.Stable(manifestsByVersion(matches)) + return matches +} + +func versionMatch(actual, expected string) bool { + // Check whether actual and expected are equivalent, or whether + // expected is a version prefix of actual. + return actual == "" || expected == "" || actual == expected || strings.HasPrefix(actual, expected+".") +} + +type manifestsByVersion []manifestlist.ManifestDescriptor + +func (mbv manifestsByVersion) Less(i, j int) bool { + if mbv[i].Platform.OSVersion == "" { + return false + } + if mbv[j].Platform.OSVersion == "" { + return true + } + // TODO: Split version by parts and compare + // TODO: Prefer versions which have a greater version number + return false +} + +func (mbv manifestsByVersion) Len() int { + return len(mbv) +} + +func (mbv manifestsByVersion) Swap(i, j int) { + mbv[i], mbv[j] = mbv[j], mbv[i] +} From 9083769a6e55984a2742f7d13dd01f4630bdc92b Mon Sep 17 00:00:00 2001 From: John Stephens Date: Fri, 6 Oct 2017 22:19:06 -0700 Subject: [PATCH 2/2] Stop filtering Windows manifest lists by version Signed-off-by: John Stephens (cherry picked from commit 8ed8f4a71d7e1a936fa077b4348b7375c81746a6) Conflicts: components/engine/distribution/pull_v2_windows.go Signed-off-by: John Stephens --- .../engine/distribution/pull_v2_windows.go | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/components/engine/distribution/pull_v2_windows.go b/components/engine/distribution/pull_v2_windows.go index e94109484d..76b6f270d1 100644 --- a/components/engine/distribution/pull_v2_windows.go +++ b/components/engine/distribution/pull_v2_windows.go @@ -75,43 +75,40 @@ func filterManifests(manifests []manifestlist.ManifestDescriptor) []manifestlist var matches []manifestlist.ManifestDescriptor for _, manifestDescriptor := range manifests { + // TODO: Consider filtering out greater versions, including only greater UBR if manifestDescriptor.Platform.Architecture == runtime.GOARCH && manifestDescriptor.Platform.OS == lookingForOS { - if !versionMatch(manifestDescriptor.Platform.OSVersion, osVersion) { - continue - } matches = append(matches, manifestDescriptor) logrus.Debugf("found match for %s/%s with media type %s, digest %s", runtime.GOOS, runtime.GOARCH, manifestDescriptor.MediaType, manifestDescriptor.Digest.String()) } } - sort.Stable(manifestsByVersion(matches)) + if lookingForOS == "windows" { + sort.Stable(manifestsByVersion{osVersion, matches}) + } return matches } func versionMatch(actual, expected string) bool { - // Check whether actual and expected are equivalent, or whether - // expected is a version prefix of actual. - return actual == "" || expected == "" || actual == expected || strings.HasPrefix(actual, expected+".") + // Check whether the version matches up to the build, ignoring UBR + return strings.HasPrefix(actual, expected+".") } -type manifestsByVersion []manifestlist.ManifestDescriptor +type manifestsByVersion struct { + version string + list []manifestlist.ManifestDescriptor +} func (mbv manifestsByVersion) Less(i, j int) bool { - if mbv[i].Platform.OSVersion == "" { - return false - } - if mbv[j].Platform.OSVersion == "" { - return true - } // TODO: Split version by parts and compare // TODO: Prefer versions which have a greater version number - return false + // Move compatible versions to the top, with no other ordering changes + return versionMatch(mbv.list[i].Platform.OSVersion, mbv.version) && !versionMatch(mbv.list[j].Platform.OSVersion, mbv.version) } func (mbv manifestsByVersion) Len() int { - return len(mbv) + return len(mbv.list) } func (mbv manifestsByVersion) Swap(i, j int) { - mbv[i], mbv[j] = mbv[j], mbv[i] + mbv.list[i], mbv.list[j] = mbv.list[j], mbv.list[i] }