From ff9be61ad4fb1641dd95688b4c92d858a8cea654 Mon Sep 17 00:00:00 2001 From: Jirka Chadima Date: Wed, 1 Jan 2020 13:36:45 +0100 Subject: [PATCH 1/7] Adds the possibility to choose the image by the variant field in a manifest Signed-off-by: Jirka Chadima --- manifest/docker_schema2_list.go | 14 +++++++++++++- manifest/list_test.go | 6 +++--- manifest/oci_index.go | 14 +++++++++++++- types/types.go | 2 ++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/manifest/docker_schema2_list.go b/manifest/docker_schema2_list.go index 453976c487..77f74cf203 100644 --- a/manifest/docker_schema2_list.go +++ b/manifest/docker_schema2_list.go @@ -101,11 +101,23 @@ func (list *Schema2List) ChooseInstance(ctx *types.SystemContext) (digest.Digest wantedOS = ctx.OSChoice } + var fallback digest.Digest + for _, d := range list.Manifests { if d.Platform.Architecture == wantedArch && d.Platform.OS == wantedOS { - return d.Digest, nil + // TODO It should be possible to somehow use runtime.GOARM to construct a default VariantChoice + if ctx != nil && (ctx.VariantChoice == "" || d.Platform.Variant == ctx.VariantChoice) { + return d.Digest, nil + } + if fallback == "" { + fallback = d.Digest + } } } + if fallback != "" { + return fallback, nil + } + return "", fmt.Errorf("no image found in manifest list for architecture %s, OS %s", wantedArch, wantedOS) } diff --git a/manifest/list_test.go b/manifest/list_test.go index 58bcb60b77..9ce17155dd 100644 --- a/manifest/list_test.go +++ b/manifest/list_test.go @@ -75,9 +75,7 @@ func TestChooseInstance(t *testing.T) { matchedInstances: map[string]digest.Digest{ "amd64": "sha256:030fcb92e1487b18c974784dcc110a93147c9fc402188370fbfd17efabffc6af", "s390x": "sha256:e5aa1b0a24620228b75382997a0977f609b3ca3a95533dafdef84c74cc8df642", - // There are several "arm" images with different variants; - // the current code returns the first match. NOTE: This is NOT an API promise. - "arm": "sha256:9142d97ef280a7953cf1a85716de49a24cc1dd62776352afad67e635331ff77a", + "arm": "sha256:b5dbad4bdb4444d919294afe49a095c23e86782f98cdf0aa286198ddb814b50b", }, unmatchedInstances: []string{ "unmatched", @@ -104,6 +102,7 @@ func TestChooseInstance(t *testing.T) { digest, err := list.ChooseInstance(&types.SystemContext{ ArchitectureChoice: arch, OSChoice: "linux", + VariantChoice: "v6", }) require.NoError(t, err, arch) assert.Equal(t, expected, digest) @@ -113,6 +112,7 @@ func TestChooseInstance(t *testing.T) { _, err := list.ChooseInstance(&types.SystemContext{ ArchitectureChoice: arch, OSChoice: "linux", + VariantChoice: "v6", }) assert.Error(t, err) } diff --git a/manifest/oci_index.go b/manifest/oci_index.go index 816503ce5e..abf4829618 100644 --- a/manifest/oci_index.go +++ b/manifest/oci_index.go @@ -84,11 +84,23 @@ func (index *OCI1Index) ChooseInstance(ctx *types.SystemContext) (digest.Digest, wantedOS = ctx.OSChoice } + var fallback digest.Digest + for _, d := range index.Manifests { if d.Platform != nil && d.Platform.Architecture == wantedArch && d.Platform.OS == wantedOS { - return d.Digest, nil + // TODO It should be possible to somehow use runtime.GOARM to construct a default VariantChoice + if ctx != nil && (ctx.VariantChoice == "" || d.Platform.Variant == ctx.VariantChoice) { + return d.Digest, nil + } + if fallback == "" { + fallback = d.Digest + } } } + if fallback != "" { + return fallback, nil + } + for _, d := range index.Manifests { if d.Platform == nil { return d.Digest, nil diff --git a/types/types.go b/types/types.go index ba249ca25d..ed26a25f23 100644 --- a/types/types.go +++ b/types/types.go @@ -510,6 +510,8 @@ type SystemContext struct { ArchitectureChoice string // If not "", overrides the use of platform.GOOS when choosing an image or verifying OS match. OSChoice string + // If not "", overrides the use of detected ARM platform variant when choosing an image or verifying variant match. + VariantChoice string // If not "", overrides the system's default directory containing a blob info cache. BlobInfoCacheDir string // Additional tags when creating or copying a docker-archive. From 89ddfced316a5390c26f711ecb9982f03be553c5 Mon Sep 17 00:00:00 2001 From: Jirka Chadima Date: Mon, 6 Jan 2020 13:36:21 +0100 Subject: [PATCH 2/7] Introduces ARM Variant detection from /proc/cpuinfo Largely based on https://github.com/moby/moby/blob/bc846d2e8fe5538220e0c31e9d0e8446f6fbc022/distribution/cpuinfo_unix.go https://github.com/containerd/containerd/blob/726dcaea50883e51b2ec6db13caff0e7936b711d/platforms/cpuinfo.go Signed-off-by: Jirka Chadima --- copy/copy.go | 22 +++--- manifest/docker_schema2_list.go | 29 ++------ manifest/list_test.go | 10 +-- manifest/oci_index.go | 29 +++----- manifest/platform_matcher.go | 120 ++++++++++++++++++++++++++++++++ 5 files changed, 149 insertions(+), 61 deletions(-) create mode 100644 manifest/platform_matcher.go diff --git a/copy/copy.go b/copy/copy.go index 29660b6b25..6a0d1bc174 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -8,7 +8,6 @@ import ( "io/ioutil" "os" "reflect" - "runtime" "strings" "sync" "time" @@ -703,22 +702,17 @@ func checkImageDestinationForCurrentRuntime(ctx context.Context, sys *types.Syst if err != nil { return errors.Wrapf(err, "Error parsing image configuration") } - - wantedOS := runtime.GOOS - if sys != nil && sys.OSChoice != "" { - wantedOS = sys.OSChoice - } - if wantedOS != c.OS { - return fmt.Errorf("Image operating system mismatch: image uses %q, expecting %q", c.OS, wantedOS) + wantedPlatform, err := manifest.WantedPlatform(sys) + if err != nil { + return errors.Wrapf(err, "error getting platform information %#v", sys) } - - wantedArch := runtime.GOARCH - if sys != nil && sys.ArchitectureChoice != "" { - wantedArch = sys.ArchitectureChoice + if wantedPlatform.OS != c.OS { + return fmt.Errorf("Image operating system mismatch: image uses %q, expecting %q", c.OS, wantedPlatform.OS) } - if wantedArch != c.Architecture { - return fmt.Errorf("Image architecture mismatch: image uses %q, expecting %q", c.Architecture, wantedArch) + if wantedPlatform.Architecture != c.Architecture { + return fmt.Errorf("Image architecture mismatch: image uses %q, expecting %q", c.Architecture, wantedPlatform.Architecture) } + // TODO also check for variant } return nil } diff --git a/manifest/docker_schema2_list.go b/manifest/docker_schema2_list.go index 77f74cf203..77a1f7b9cf 100644 --- a/manifest/docker_schema2_list.go +++ b/manifest/docker_schema2_list.go @@ -3,7 +3,6 @@ package manifest import ( "encoding/json" "fmt" - "runtime" "github.com/containers/image/v5/types" "github.com/opencontainers/go-digest" @@ -92,33 +91,19 @@ func (list *Schema2List) UpdateInstances(updates []ListUpdate) error { // ChooseInstance parses blob as a schema2 manifest list, and returns the digest // of the image which is appropriate for the current environment. func (list *Schema2List) ChooseInstance(ctx *types.SystemContext) (digest.Digest, error) { - wantedArch := runtime.GOARCH - if ctx != nil && ctx.ArchitectureChoice != "" { - wantedArch = ctx.ArchitectureChoice - } - wantedOS := runtime.GOOS - if ctx != nil && ctx.OSChoice != "" { - wantedOS = ctx.OSChoice + wantedPlatform, err := WantedPlatform(ctx) + if err != nil { + return "", errors.Wrapf(err, "error getting platform information %#v", ctx) } - var fallback digest.Digest - for _, d := range list.Manifests { - if d.Platform.Architecture == wantedArch && d.Platform.OS == wantedOS { - // TODO It should be possible to somehow use runtime.GOARM to construct a default VariantChoice - if ctx != nil && (ctx.VariantChoice == "" || d.Platform.Variant == ctx.VariantChoice) { - return d.Digest, nil - } - if fallback == "" { - fallback = d.Digest - } + // TODO some variants might work on different demanded variants (https://github.com/containerd/containerd/blob/master/platforms/compare.go#L29) + if d.Platform.Architecture == wantedPlatform.Architecture && d.Platform.OS == wantedPlatform.OS && (wantedPlatform.Variant == "" || d.Platform.Variant == wantedPlatform.Variant) { + return d.Digest, nil } } - if fallback != "" { - return fallback, nil - } - return "", fmt.Errorf("no image found in manifest list for architecture %s, OS %s", wantedArch, wantedOS) + return "", fmt.Errorf("no image found in image index for architecture %s, OS %s, Variant %s", wantedPlatform.Architecture, wantedPlatform.OS, wantedPlatform.Variant) } // Serialize returns the list in a blob format. diff --git a/manifest/list_test.go b/manifest/list_test.go index 9ce17155dd..bc7ddfba55 100644 --- a/manifest/list_test.go +++ b/manifest/list_test.go @@ -99,11 +99,14 @@ func TestChooseInstance(t *testing.T) { require.NoError(t, err) // Match found for arch, expected := range manifestList.matchedInstances { - digest, err := list.ChooseInstance(&types.SystemContext{ + ctx := &types.SystemContext{ ArchitectureChoice: arch, OSChoice: "linux", - VariantChoice: "v6", - }) + } + if arch == "arm" { + ctx.VariantChoice = "v6" + } + digest, err := list.ChooseInstance(ctx) require.NoError(t, err, arch) assert.Equal(t, expected, digest) } @@ -112,7 +115,6 @@ func TestChooseInstance(t *testing.T) { _, err := list.ChooseInstance(&types.SystemContext{ ArchitectureChoice: arch, OSChoice: "linux", - VariantChoice: "v6", }) assert.Error(t, err) } diff --git a/manifest/oci_index.go b/manifest/oci_index.go index abf4829618..ef74a47f96 100644 --- a/manifest/oci_index.go +++ b/manifest/oci_index.go @@ -75,38 +75,25 @@ func (index *OCI1Index) UpdateInstances(updates []ListUpdate) error { // ChooseInstance parses blob as an oci v1 manifest index, and returns the digest // of the image which is appropriate for the current environment. func (index *OCI1Index) ChooseInstance(ctx *types.SystemContext) (digest.Digest, error) { - wantedArch := runtime.GOARCH - if ctx != nil && ctx.ArchitectureChoice != "" { - wantedArch = ctx.ArchitectureChoice - } - wantedOS := runtime.GOOS - if ctx != nil && ctx.OSChoice != "" { - wantedOS = ctx.OSChoice + wantedPlatform, err := WantedPlatform(ctx) + if err != nil { + return "", errors.Wrapf(err, "error getting platform information %#v", ctx) } - var fallback digest.Digest - for _, d := range index.Manifests { - if d.Platform != nil && d.Platform.Architecture == wantedArch && d.Platform.OS == wantedOS { - // TODO It should be possible to somehow use runtime.GOARM to construct a default VariantChoice - if ctx != nil && (ctx.VariantChoice == "" || d.Platform.Variant == ctx.VariantChoice) { - return d.Digest, nil - } - if fallback == "" { - fallback = d.Digest - } + // TODO some variants might work on different demanded variants (https://github.com/containerd/containerd/blob/master/platforms/compare.go#L29) + if d.Platform.Architecture == wantedPlatform.Architecture && d.Platform.OS == wantedPlatform.OS && (wantedPlatform.Variant == "" || d.Platform.Variant == wantedPlatform.Variant) { + return d.Digest, nil } } - if fallback != "" { - return fallback, nil - } for _, d := range index.Manifests { if d.Platform == nil { return d.Digest, nil } } - return "", fmt.Errorf("no image found in image index for architecture %s, OS %s", wantedArch, wantedOS) + + return "", fmt.Errorf("no image found in image index for architecture %s, OS %s, Variant %s", wantedPlatform.Architecture, wantedPlatform.OS, wantedPlatform.Variant) } // Serialize returns the index in a blob format. diff --git a/manifest/platform_matcher.go b/manifest/platform_matcher.go new file mode 100644 index 0000000000..206f608a8e --- /dev/null +++ b/manifest/platform_matcher.go @@ -0,0 +1,120 @@ +package manifest + +// Largely based on +// https://github.com/moby/moby/blob/bc846d2e8fe5538220e0c31e9d0e8446f6fbc022/distribution/cpuinfo_unix.go +// https://github.com/containerd/containerd/blob/726dcaea50883e51b2ec6db13caff0e7936b711d/platforms/cpuinfo.go + +import ( + "bufio" + "fmt" + "os" + "runtime" + "strings" + + "github.com/containers/image/v5/types" + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" +) + +// For Linux, the kernel has already detected the ABI, ISA and Features. +// So we don't need to access the ARM registers to detect platform information +// by ourselves. We can just parse these information from /proc/cpuinfo +func getCPUInfo(pattern string) (info string, err error) { + if runtime.GOOS != "linux" { + return "", fmt.Errorf("getCPUInfo for OS %s not implemented", runtime.GOOS) + } + + cpuinfo, err := os.Open("/proc/cpuinfo") + if err != nil { + return "", err + } + defer cpuinfo.Close() + + // Start to Parse the Cpuinfo line by line. For SMP SoC, we parse + // the first core is enough. + scanner := bufio.NewScanner(cpuinfo) + for scanner.Scan() { + newline := scanner.Text() + list := strings.Split(newline, ":") + + if len(list) > 1 && strings.EqualFold(strings.TrimSpace(list[0]), pattern) { + return strings.TrimSpace(list[1]), nil + } + } + + // Check whether the scanner encountered errors + err = scanner.Err() + if err != nil { + return "", err + } + + return "", fmt.Errorf("getCPUInfo for pattern: %s not found", pattern) +} + +func getCPUVariant() string { + if runtime.GOOS == "windows" { + // Windows only supports v7 for ARM32 and v8 for ARM64 and so we can use + // runtime.GOARCH to determine the variants + var variant string + switch runtime.GOARCH { + case "arm64": + variant = "v8" + case "arm": + variant = "v7" + default: + variant = "unknown" + } + + return variant + } + + variant, err := getCPUInfo("Cpu architecture") + if err != nil { + return "" + } + // TODO handle RPi Zero mismatch (https://github.com/moby/moby/pull/36121#issuecomment-398328286) + + switch strings.ToLower(variant) { + case "8", "aarch64": + variant = "v8" + case "7", "7m", "?(12)", "?(13)", "?(14)", "?(15)", "?(16)", "?(17)": + variant = "v7" + case "6", "6tej": + variant = "v6" + case "5", "5t", "5te", "5tej": + variant = "v5" + case "4", "4t": + variant = "v4" + case "3": + variant = "v3" + default: + variant = "unknown" + } + + return variant +} + +func WantedPlatform(ctx *types.SystemContext) (imgspecv1.Platform, error) { + wantedArch := runtime.GOARCH + if ctx != nil && ctx.ArchitectureChoice != "" { + wantedArch = ctx.ArchitectureChoice + } + wantedOS := runtime.GOOS + if ctx != nil && ctx.OSChoice != "" { + wantedOS = ctx.OSChoice + } + + wantedPlatform := imgspecv1.Platform{ + OS: wantedOS, + Architecture: wantedArch, + } + + if ctx != nil && ctx.VariantChoice != "" { + if wantedArch == "arm" || wantedArch == "arm64" { + wantedPlatform.Variant = ctx.VariantChoice + } else { + wantedPlatform.Variant = getCPUVariant() + // TODO handle Variant == 'unknown' + } + } + return wantedPlatform, nil +} From 78151d366bcf0993cfe8b9c0a025a3e9e1161950 Mon Sep 17 00:00:00 2001 From: Jirka Chadima Date: Tue, 7 Jan 2020 12:27:06 +0100 Subject: [PATCH 3/7] Allows matching of compatible OS Variants when selecting the appropriate image Signed-off-by: Jirka Chadima --- copy/copy.go | 3 +- image/fixtures/schema2list.json | 10 ------ manifest/docker_schema2_list.go | 15 ++++----- manifest/list_test.go | 3 +- manifest/oci_index.go | 22 +++++++----- manifest/platform_matcher.go | 60 +++++++++++++++++++++++++++------ 6 files changed, 75 insertions(+), 38 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index 6a0d1bc174..dd4b1d2d39 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -702,10 +702,11 @@ func checkImageDestinationForCurrentRuntime(ctx context.Context, sys *types.Syst if err != nil { return errors.Wrapf(err, "Error parsing image configuration") } - wantedPlatform, err := manifest.WantedPlatform(sys) + wantedPlatforms, err := manifest.WantedPlatforms(sys) if err != nil { return errors.Wrapf(err, "error getting platform information %#v", sys) } + wantedPlatform := wantedPlatforms[0] // TODO fixme if wantedPlatform.OS != c.OS { return fmt.Errorf("Image operating system mismatch: image uses %q, expecting %q", c.OS, wantedPlatform.OS) } diff --git a/image/fixtures/schema2list.json b/image/fixtures/schema2list.json index 844215b29b..398b746cbd 100644 --- a/image/fixtures/schema2list.json +++ b/image/fixtures/schema2list.json @@ -31,16 +31,6 @@ "variant": "v6" } }, - { - "mediaType": "application/vnd.docker.distribution.manifest.v2+json", - "size": 527, - "digest": "sha256:a8fe0549cac196f439de3bf2b57af14f7cd4e59915ccd524428f588628a4ef31", - "platform": { - "architecture": "arm", - "os": "linux", - "variant": "v7" - } - }, { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "size": 527, diff --git a/manifest/docker_schema2_list.go b/manifest/docker_schema2_list.go index 77a1f7b9cf..f3c0d540a1 100644 --- a/manifest/docker_schema2_list.go +++ b/manifest/docker_schema2_list.go @@ -91,19 +91,18 @@ func (list *Schema2List) UpdateInstances(updates []ListUpdate) error { // ChooseInstance parses blob as a schema2 manifest list, and returns the digest // of the image which is appropriate for the current environment. func (list *Schema2List) ChooseInstance(ctx *types.SystemContext) (digest.Digest, error) { - wantedPlatform, err := WantedPlatform(ctx) + wantedPlatforms, err := WantedPlatforms(ctx) if err != nil { return "", errors.Wrapf(err, "error getting platform information %#v", ctx) } - - for _, d := range list.Manifests { - // TODO some variants might work on different demanded variants (https://github.com/containerd/containerd/blob/master/platforms/compare.go#L29) - if d.Platform.Architecture == wantedPlatform.Architecture && d.Platform.OS == wantedPlatform.OS && (wantedPlatform.Variant == "" || d.Platform.Variant == wantedPlatform.Variant) { - return d.Digest, nil + for _, wantedPlatform := range wantedPlatforms { + for _, d := range list.Manifests { + if MatchesPlatform(d.Platform, wantedPlatform) { + return d.Digest, nil + } } } - - return "", fmt.Errorf("no image found in image index for architecture %s, OS %s, Variant %s", wantedPlatform.Architecture, wantedPlatform.OS, wantedPlatform.Variant) + return "", fmt.Errorf("no image found in image index for architecture %s, OS %s, Variant %s", wantedPlatforms[0].Architecture, wantedPlatforms[0].OS, wantedPlatforms[0].Variant) } // Serialize returns the list in a blob format. diff --git a/manifest/list_test.go b/manifest/list_test.go index bc7ddfba55..93866b1093 100644 --- a/manifest/list_test.go +++ b/manifest/list_test.go @@ -76,6 +76,7 @@ func TestChooseInstance(t *testing.T) { "amd64": "sha256:030fcb92e1487b18c974784dcc110a93147c9fc402188370fbfd17efabffc6af", "s390x": "sha256:e5aa1b0a24620228b75382997a0977f609b3ca3a95533dafdef84c74cc8df642", "arm": "sha256:b5dbad4bdb4444d919294afe49a095c23e86782f98cdf0aa286198ddb814b50b", + "arm64": "sha256:dc472a59fb006797aa2a6bfb54cc9c57959bb0a6d11fadaa608df8c16dea39cf", }, unmatchedInstances: []string{ "unmatched", @@ -104,7 +105,7 @@ func TestChooseInstance(t *testing.T) { OSChoice: "linux", } if arch == "arm" { - ctx.VariantChoice = "v6" + ctx.VariantChoice = "v7" } digest, err := list.ChooseInstance(ctx) require.NoError(t, err, arch) diff --git a/manifest/oci_index.go b/manifest/oci_index.go index ef74a47f96..b86db0cd36 100644 --- a/manifest/oci_index.go +++ b/manifest/oci_index.go @@ -75,15 +75,22 @@ func (index *OCI1Index) UpdateInstances(updates []ListUpdate) error { // ChooseInstance parses blob as an oci v1 manifest index, and returns the digest // of the image which is appropriate for the current environment. func (index *OCI1Index) ChooseInstance(ctx *types.SystemContext) (digest.Digest, error) { - wantedPlatform, err := WantedPlatform(ctx) + wantedPlatforms, err := WantedPlatforms(ctx) if err != nil { return "", errors.Wrapf(err, "error getting platform information %#v", ctx) } - - for _, d := range index.Manifests { - // TODO some variants might work on different demanded variants (https://github.com/containerd/containerd/blob/master/platforms/compare.go#L29) - if d.Platform.Architecture == wantedPlatform.Architecture && d.Platform.OS == wantedPlatform.OS && (wantedPlatform.Variant == "" || d.Platform.Variant == wantedPlatform.Variant) { - return d.Digest, nil + for _, wantedPlatform := range wantedPlatforms { + for _, d := range index.Manifests { + imagePlatform := Schema2PlatformSpec{ + Architecture: d.Platform.Architecture, + OS: d.Platform.OS, + OSVersion: d.Platform.OSVersion, + OSFeatures: dupStringSlice(d.Platform.OSFeatures), + Variant: d.Platform.Variant, + } + if MatchesPlatform(imagePlatform, wantedPlatform) { + return d.Digest, nil + } } } @@ -92,8 +99,7 @@ func (index *OCI1Index) ChooseInstance(ctx *types.SystemContext) (digest.Digest, return d.Digest, nil } } - - return "", fmt.Errorf("no image found in image index for architecture %s, OS %s, Variant %s", wantedPlatform.Architecture, wantedPlatform.OS, wantedPlatform.Variant) + return "", fmt.Errorf("no image found in image index for architecture %s, OS %s, Variant %s", wantedPlatforms[0].Architecture, wantedPlatforms[0].OS, wantedPlatforms[0].Variant) } // Serialize returns the index in a blob format. diff --git a/manifest/platform_matcher.go b/manifest/platform_matcher.go index 206f608a8e..1bda43cb58 100644 --- a/manifest/platform_matcher.go +++ b/manifest/platform_matcher.go @@ -93,7 +93,12 @@ func getCPUVariant() string { return variant } -func WantedPlatform(ctx *types.SystemContext) (imgspecv1.Platform, error) { +var compatibility = map[string][]string{ + "arm": []string{"v7", "v6", "v5"}, + "arm64": []string{"v8"}, +} + +func WantedPlatforms(ctx *types.SystemContext) ([]imgspecv1.Platform, error) { wantedArch := runtime.GOARCH if ctx != nil && ctx.ArchitectureChoice != "" { wantedArch = ctx.ArchitectureChoice @@ -103,18 +108,53 @@ func WantedPlatform(ctx *types.SystemContext) (imgspecv1.Platform, error) { wantedOS = ctx.OSChoice } - wantedPlatform := imgspecv1.Platform{ - OS: wantedOS, - Architecture: wantedArch, - } + var wantedPlatforms []imgspecv1.Platform - if ctx != nil && ctx.VariantChoice != "" { - if wantedArch == "arm" || wantedArch == "arm64" { - wantedPlatform.Variant = ctx.VariantChoice + wantedVariant := "" + if wantedArch == "arm" || wantedArch == "arm64" { + if ctx != nil && ctx.VariantChoice != "" { + wantedVariant = ctx.VariantChoice } else { - wantedPlatform.Variant = getCPUVariant() // TODO handle Variant == 'unknown' + wantedVariant = getCPUVariant() + } + } + + if wantedVariant != "" && compatibility[wantedArch] != nil { + wantedPlatforms = make([]imgspecv1.Platform, 0, len(compatibility[wantedArch])) + for _, v := range compatibility[wantedArch] { + if wantedVariant >= v { + wantedPlatforms = append(wantedPlatforms, imgspecv1.Platform{ + OS: wantedOS, + Architecture: wantedArch, + Variant: v, + }) + } + } + } else { + wantedPlatforms = []imgspecv1.Platform{ + imgspecv1.Platform{ + OS: wantedOS, + Architecture: wantedArch, + Variant: wantedVariant, + }, } } - return wantedPlatform, nil + + return wantedPlatforms, nil +} + +func MatchesPlatform(image Schema2PlatformSpec, wanted imgspecv1.Platform) bool { + if image.Architecture != wanted.Architecture { + return false + } + if image.OS != wanted.OS { + return false + } + + if wanted.Variant == "" || image.Variant == wanted.Variant { + return true + } + + return false } From 6a3e032a63ba6caf76f6cb839ce3db06c71a9a3c Mon Sep 17 00:00:00 2001 From: Jirka Chadima Date: Thu, 9 Jan 2020 10:58:00 +0100 Subject: [PATCH 4/7] Makes platform_matcher internal Signed-off-by: Jirka Chadima --- copy/copy.go | 3 ++- .../pkg/platform}/platform_matcher.go | 2 +- manifest/docker_schema2_list.go | 12 ++++++++++-- manifest/oci_index.go | 7 ++++--- 4 files changed, 17 insertions(+), 7 deletions(-) rename {manifest => internal/pkg/platform}/platform_matcher.go (97%) diff --git a/copy/copy.go b/copy/copy.go index dd4b1d2d39..dfde9e3466 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -14,6 +14,7 @@ import ( "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/image" + platform "github.com/containers/image/v5/internal/pkg/platform" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/pkg/blobinfocache" "github.com/containers/image/v5/pkg/compression" @@ -702,7 +703,7 @@ func checkImageDestinationForCurrentRuntime(ctx context.Context, sys *types.Syst if err != nil { return errors.Wrapf(err, "Error parsing image configuration") } - wantedPlatforms, err := manifest.WantedPlatforms(sys) + wantedPlatforms, err := platform.WantedPlatforms(sys) if err != nil { return errors.Wrapf(err, "error getting platform information %#v", sys) } diff --git a/manifest/platform_matcher.go b/internal/pkg/platform/platform_matcher.go similarity index 97% rename from manifest/platform_matcher.go rename to internal/pkg/platform/platform_matcher.go index 1bda43cb58..adbc9b1c17 100644 --- a/manifest/platform_matcher.go +++ b/internal/pkg/platform/platform_matcher.go @@ -144,7 +144,7 @@ func WantedPlatforms(ctx *types.SystemContext) ([]imgspecv1.Platform, error) { return wantedPlatforms, nil } -func MatchesPlatform(image Schema2PlatformSpec, wanted imgspecv1.Platform) bool { +func MatchesPlatform(image imgspecv1.Platform, wanted imgspecv1.Platform) bool { if image.Architecture != wanted.Architecture { return false } diff --git a/manifest/docker_schema2_list.go b/manifest/docker_schema2_list.go index f3c0d540a1..178633ba0b 100644 --- a/manifest/docker_schema2_list.go +++ b/manifest/docker_schema2_list.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" + platform "github.com/containers/image/v5/internal/pkg/platform" "github.com/containers/image/v5/types" "github.com/opencontainers/go-digest" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" @@ -91,13 +92,20 @@ func (list *Schema2List) UpdateInstances(updates []ListUpdate) error { // ChooseInstance parses blob as a schema2 manifest list, and returns the digest // of the image which is appropriate for the current environment. func (list *Schema2List) ChooseInstance(ctx *types.SystemContext) (digest.Digest, error) { - wantedPlatforms, err := WantedPlatforms(ctx) + wantedPlatforms, err := platform.WantedPlatforms(ctx) if err != nil { return "", errors.Wrapf(err, "error getting platform information %#v", ctx) } for _, wantedPlatform := range wantedPlatforms { for _, d := range list.Manifests { - if MatchesPlatform(d.Platform, wantedPlatform) { + imagePlatform := imgspecv1.Platform{ + Architecture: d.Platform.Architecture, + OS: d.Platform.OS, + OSVersion: d.Platform.OSVersion, + OSFeatures: dupStringSlice(d.Platform.OSFeatures), + Variant: d.Platform.Variant, + } + if platform.MatchesPlatform(imagePlatform, wantedPlatform) { return d.Digest, nil } } diff --git a/manifest/oci_index.go b/manifest/oci_index.go index b86db0cd36..7ebac7818b 100644 --- a/manifest/oci_index.go +++ b/manifest/oci_index.go @@ -5,6 +5,7 @@ import ( "fmt" "runtime" + platform "github.com/containers/image/v5/internal/pkg/platform" "github.com/containers/image/v5/types" "github.com/opencontainers/go-digest" imgspec "github.com/opencontainers/image-spec/specs-go" @@ -75,20 +76,20 @@ func (index *OCI1Index) UpdateInstances(updates []ListUpdate) error { // ChooseInstance parses blob as an oci v1 manifest index, and returns the digest // of the image which is appropriate for the current environment. func (index *OCI1Index) ChooseInstance(ctx *types.SystemContext) (digest.Digest, error) { - wantedPlatforms, err := WantedPlatforms(ctx) + wantedPlatforms, err := platform.WantedPlatforms(ctx) if err != nil { return "", errors.Wrapf(err, "error getting platform information %#v", ctx) } for _, wantedPlatform := range wantedPlatforms { for _, d := range index.Manifests { - imagePlatform := Schema2PlatformSpec{ + imagePlatform := imgspecv1.Platform{ Architecture: d.Platform.Architecture, OS: d.Platform.OS, OSVersion: d.Platform.OSVersion, OSFeatures: dupStringSlice(d.Platform.OSFeatures), Variant: d.Platform.Variant, } - if MatchesPlatform(imagePlatform, wantedPlatform) { + if platform.MatchesPlatform(imagePlatform, wantedPlatform) { return d.Digest, nil } } From 345c9c5d75ea09507feee9108c3c5d826d03aca6 Mon Sep 17 00:00:00 2001 From: Jirka Chadima Date: Thu, 9 Jan 2020 14:00:58 +0100 Subject: [PATCH 5/7] Improves generality of platform matcher, adds a few tests Signed-off-by: Jirka Chadima --- internal/pkg/platform/platform_matcher.go | 88 ++++++++++++------- .../pkg/platform/platform_matcher_test.go | 46 ++++++++++ 2 files changed, 102 insertions(+), 32 deletions(-) create mode 100644 internal/pkg/platform/platform_matcher_test.go diff --git a/internal/pkg/platform/platform_matcher.go b/internal/pkg/platform/platform_matcher.go index adbc9b1c17..55925cc908 100644 --- a/internal/pkg/platform/platform_matcher.go +++ b/internal/pkg/platform/platform_matcher.go @@ -1,4 +1,4 @@ -package manifest +package platform // Largely based on // https://github.com/moby/moby/blob/bc846d2e8fe5538220e0c31e9d0e8446f6fbc022/distribution/cpuinfo_unix.go @@ -50,23 +50,23 @@ func getCPUInfo(pattern string) (info string, err error) { return "", fmt.Errorf("getCPUInfo for pattern: %s not found", pattern) } -func getCPUVariant() string { - if runtime.GOOS == "windows" { - // Windows only supports v7 for ARM32 and v8 for ARM64 and so we can use - // runtime.GOARCH to determine the variants - var variant string - switch runtime.GOARCH { - case "arm64": - variant = "v8" - case "arm": - variant = "v7" - default: - variant = "unknown" - } - - return variant +func getCPUVariantWindows() string { + // Windows only supports v7 for ARM32 and v8 for ARM64 and so we can use + // runtime.GOARCH to determine the variants + var variant string + switch runtime.GOARCH { + case "arm64": + variant = "v8" + case "arm": + variant = "v7" + default: + variant = "" } + return variant +} + +func getCPUVariantArm() string { variant, err := getCPUInfo("Cpu architecture") if err != nil { return "" @@ -87,17 +87,30 @@ func getCPUVariant() string { case "3": variant = "v3" default: - variant = "unknown" + variant = "" } return variant } +func getCPUVariant(os string, arch string) string { + if os == "windows" { + return getCPUVariantWindows() + } + if arch == "arm" || arch == "arm64" { + return getCPUVariantArm() + } + return "" +} + var compatibility = map[string][]string{ - "arm": []string{"v7", "v6", "v5"}, - "arm64": []string{"v8"}, + "arm": {"v7", "v6", "v5"}, + "arm64": {"v8"}, } +// Returns all compatible platforms with the platform specifics possibly overriden by user, +// the most compatible platform is first. +// If some option (arch, os, variant) is not present, a value from current platform is detected. func WantedPlatforms(ctx *types.SystemContext) ([]imgspecv1.Platform, error) { wantedArch := runtime.GOARCH if ctx != nil && ctx.ArchitectureChoice != "" { @@ -108,22 +121,33 @@ func WantedPlatforms(ctx *types.SystemContext) ([]imgspecv1.Platform, error) { wantedOS = ctx.OSChoice } - var wantedPlatforms []imgspecv1.Platform - - wantedVariant := "" - if wantedArch == "arm" || wantedArch == "arm64" { - if ctx != nil && ctx.VariantChoice != "" { - wantedVariant = ctx.VariantChoice - } else { - // TODO handle Variant == 'unknown' - wantedVariant = getCPUVariant() - } + wantedVariant := getCPUVariant(runtime.GOOS, runtime.GOARCH) + if ctx != nil && ctx.VariantChoice != "" { + wantedVariant = ctx.VariantChoice } + var wantedPlatforms []imgspecv1.Platform if wantedVariant != "" && compatibility[wantedArch] != nil { wantedPlatforms = make([]imgspecv1.Platform, 0, len(compatibility[wantedArch])) - for _, v := range compatibility[wantedArch] { - if wantedVariant >= v { + wantedIndex := -1 + for i, v := range compatibility[wantedArch] { + if wantedVariant == v { + wantedIndex = i + break + } + } + // user wants a variant which we know nothing about - not even compatibility + if wantedIndex == -1 { + wantedPlatforms = []imgspecv1.Platform{ + { + OS: wantedOS, + Architecture: wantedArch, + Variant: wantedVariant, + }, + } + } else { + for i := wantedIndex; i < len(compatibility[wantedArch]); i++ { + v := compatibility[wantedArch][i] wantedPlatforms = append(wantedPlatforms, imgspecv1.Platform{ OS: wantedOS, Architecture: wantedArch, @@ -133,7 +157,7 @@ func WantedPlatforms(ctx *types.SystemContext) ([]imgspecv1.Platform, error) { } } else { wantedPlatforms = []imgspecv1.Platform{ - imgspecv1.Platform{ + { OS: wantedOS, Architecture: wantedArch, Variant: wantedVariant, diff --git a/internal/pkg/platform/platform_matcher_test.go b/internal/pkg/platform/platform_matcher_test.go new file mode 100644 index 0000000000..658a9ba56b --- /dev/null +++ b/internal/pkg/platform/platform_matcher_test.go @@ -0,0 +1,46 @@ +package platform + +import ( + "testing" + + "github.com/containers/image/v5/types" + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" +) + +func TestWantedPlatformsCompatibility(t *testing.T) { + ctx := &types.SystemContext{ + ArchitectureChoice: "arm", + OSChoice: "linux", + VariantChoice: "v6", + } + platforms, err := WantedPlatforms(ctx) + assert.Nil(t, err) + assert.Equal(t, len(platforms), 2) + assert.Equal(t, platforms[0], imgspecv1.Platform{ + OS: ctx.OSChoice, + Architecture: ctx.ArchitectureChoice, + Variant: "v6", + }) + assert.Equal(t, platforms[1], imgspecv1.Platform{ + OS: ctx.OSChoice, + Architecture: ctx.ArchitectureChoice, + Variant: "v5", + }) +} + +func TestWantedPlatformsCustom(t *testing.T) { + ctx := &types.SystemContext{ + ArchitectureChoice: "armel", + OSChoice: "freeBSD", + VariantChoice: "custom", + } + platforms, err := WantedPlatforms(ctx) + assert.Nil(t, err) + assert.Equal(t, len(platforms), 1) + assert.Equal(t, platforms[0], imgspecv1.Platform{ + OS: ctx.OSChoice, + Architecture: ctx.ArchitectureChoice, + Variant: ctx.VariantChoice, + }) +} From 6389bbc5f9c24a4a0879e6ffcae5373926fcf360 Mon Sep 17 00:00:00 2001 From: Jirka Chadima Date: Thu, 9 Jan 2020 14:01:08 +0100 Subject: [PATCH 6/7] Makes copy command ready to check Variant eventually Signed-off-by: Jirka Chadima --- copy/copy.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/copy/copy.go b/copy/copy.go index dfde9e3466..7b3f36aeb3 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -705,16 +705,21 @@ func checkImageDestinationForCurrentRuntime(ctx context.Context, sys *types.Syst } wantedPlatforms, err := platform.WantedPlatforms(sys) if err != nil { - return errors.Wrapf(err, "error getting platform information %#v", sys) + return errors.Wrapf(err, "error getting current platform information %#v", sys) } - wantedPlatform := wantedPlatforms[0] // TODO fixme - if wantedPlatform.OS != c.OS { - return fmt.Errorf("Image operating system mismatch: image uses %q, expecting %q", c.OS, wantedPlatform.OS) - } - if wantedPlatform.Architecture != c.Architecture { - return fmt.Errorf("Image architecture mismatch: image uses %q, expecting %q", c.Architecture, wantedPlatform.Architecture) + for _, wantedPlatform := range wantedPlatforms { + if wantedPlatform.OS != c.OS { + return fmt.Errorf("Image operating system mismatch: image uses %q, expecting %q", c.OS, wantedPlatform.OS) + } + if wantedPlatform.Architecture != c.Architecture { + return fmt.Errorf("Image architecture mismatch: image uses %q, expecting %q", c.Architecture, wantedPlatform.Architecture) + } + /* + // TODO Waiting for https://github.com/opencontainers/image-spec/pull/777 + if wantedPlatform.Variant != "" && c.Variant != "" && wantedPlatform.Variant != c.Variant { + return fmt.Errorf("Image variant mismatch: image uses %q, expecting %q", c.Variant, wantedPlatform.Variant) + }*/ } - // TODO also check for variant } return nil } From fc304bbd7639689b03267cda9de39987b7491cdb Mon Sep 17 00:00:00 2001 From: Jirka Chadima Date: Thu, 9 Jan 2020 14:01:34 +0100 Subject: [PATCH 7/7] Improves error messages when matching image to a platform Signed-off-by: Jirka Chadima --- manifest/docker_schema2_list.go | 2 +- manifest/oci_index.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/manifest/docker_schema2_list.go b/manifest/docker_schema2_list.go index 178633ba0b..59612f64b7 100644 --- a/manifest/docker_schema2_list.go +++ b/manifest/docker_schema2_list.go @@ -110,7 +110,7 @@ func (list *Schema2List) ChooseInstance(ctx *types.SystemContext) (digest.Digest } } } - return "", fmt.Errorf("no image found in image index for architecture %s, OS %s, Variant %s", wantedPlatforms[0].Architecture, wantedPlatforms[0].OS, wantedPlatforms[0].Variant) + return "", fmt.Errorf("no image found in manifest list for architecture %s, variant %s, OS %s", wantedPlatforms[0].Architecture, wantedPlatforms[0].Variant, wantedPlatforms[0].OS) } // Serialize returns the list in a blob format. diff --git a/manifest/oci_index.go b/manifest/oci_index.go index 7ebac7818b..932be51218 100644 --- a/manifest/oci_index.go +++ b/manifest/oci_index.go @@ -100,7 +100,7 @@ func (index *OCI1Index) ChooseInstance(ctx *types.SystemContext) (digest.Digest, return d.Digest, nil } } - return "", fmt.Errorf("no image found in image index for architecture %s, OS %s, Variant %s", wantedPlatforms[0].Architecture, wantedPlatforms[0].OS, wantedPlatforms[0].Variant) + return "", fmt.Errorf("no image found in manifest list for architecture %s, variant %s, OS %s", wantedPlatforms[0].Architecture, wantedPlatforms[0].Variant, wantedPlatforms[0].OS) } // Serialize returns the index in a blob format.