diff --git a/pkg/cri/store/image/image.go b/pkg/cri/store/image/image.go index 7eaadbb33..27cd8bb2b 100644 --- a/pkg/cri/store/image/image.go +++ b/pkg/cri/store/image/image.go @@ -210,8 +210,8 @@ func (s *store) add(img Image) error { s.images[img.ID] = img return nil } - // Or else, merge the references. - i.References = util.MergeStringSlices(i.References, img.References) + // Or else, merge and sort the references. + i.References = sortReferences(util.MergeStringSlices(i.References, img.References)) s.images[img.ID] = i return nil } diff --git a/pkg/cri/store/image/image_test.go b/pkg/cri/store/image/image_test.go index 880ad0dc3..08bd2665b 100644 --- a/pkg/cri/store/image/image_test.go +++ b/pkg/cri/store/image/image_test.go @@ -32,24 +32,24 @@ func TestInternalStore(t *testing.T) { { ID: "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", ChainID: "test-chain-id-1", - References: []string{"ref-1"}, + References: []string{"containerd.io/ref-1"}, Size: 10, }, { ID: "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", ChainID: "test-chain-id-2abcd", - References: []string{"ref-2abcd"}, + References: []string{"containerd.io/ref-2abcd"}, Size: 20, }, { ID: "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - References: []string{"ref-4a333"}, + References: []string{"containerd.io/ref-4a333"}, ChainID: "test-chain-id-4a333", Size: 30, }, { ID: "sha256:4123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - References: []string{"ref-4abcd"}, + References: []string{"containerd.io/ref-4abcd"}, ChainID: "test-chain-id-4abcd", Size: 40, }, @@ -143,7 +143,7 @@ func TestImageStore(t *testing.T) { image := Image{ ID: id, ChainID: "test-chain-id-1", - References: []string{"ref-1"}, + References: []string{"containerd.io/ref-1"}, Size: 10, } assert := assertlib.New(t) @@ -159,33 +159,33 @@ func TestImageStore(t *testing.T) { expected []Image }{ "nothing should happen if a non-exist ref disappear": { - ref: "ref-2", + ref: "containerd.io/ref-2", image: nil, expected: []Image{image}, }, "new ref for an existing image": { - ref: "ref-2", + ref: "containerd.io/ref-2", image: &Image{ ID: id, ChainID: "test-chain-id-1", - References: []string{"ref-2"}, + References: []string{"containerd.io/ref-2"}, Size: 10, }, expected: []Image{ { ID: id, ChainID: "test-chain-id-1", - References: []string{"ref-1", "ref-2"}, + References: []string{"containerd.io/ref-1", "containerd.io/ref-2"}, Size: 10, }, }, }, "new ref for a new image": { - ref: "ref-2", + ref: "containerd.io/ref-2", image: &Image{ ID: newID, ChainID: "test-chain-id-2", - References: []string{"ref-2"}, + References: []string{"containerd.io/ref-2"}, Size: 20, }, expected: []Image{ @@ -193,30 +193,30 @@ func TestImageStore(t *testing.T) { { ID: newID, ChainID: "test-chain-id-2", - References: []string{"ref-2"}, + References: []string{"containerd.io/ref-2"}, Size: 20, }, }, }, "existing ref point to a new image": { - ref: "ref-1", + ref: "containerd.io/ref-1", image: &Image{ ID: newID, ChainID: "test-chain-id-2", - References: []string{"ref-1"}, + References: []string{"containerd.io/ref-1"}, Size: 20, }, expected: []Image{ { ID: newID, ChainID: "test-chain-id-2", - References: []string{"ref-1"}, + References: []string{"containerd.io/ref-1"}, Size: 20, }, }, }, "existing ref disappear": { - ref: "ref-1", + ref: "containerd.io/ref-1", image: nil, expected: []Image{}, }, diff --git a/pkg/cri/store/image/sort.go b/pkg/cri/store/image/sort.go new file mode 100644 index 000000000..403d7d220 --- /dev/null +++ b/pkg/cri/store/image/sort.go @@ -0,0 +1,75 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package image + +import ( + "sort" + + "github.com/containerd/containerd/reference/docker" +) + +// sortReferences sorts references by refRank then string comparison +func sortReferences(references []string) []string { + var prefs []docker.Reference + var bad []string + + for _, ref := range references { + pref, err := docker.ParseAnyReference(ref) + if err != nil { + bad = append(bad, ref) + } else { + prefs = append(prefs, pref) + } + } + sort.Slice(prefs, func(a, b int) bool { + ar := refRank(prefs[a]) + br := refRank(prefs[b]) + if ar == br { + return prefs[a].String() < prefs[b].String() + } + return ar < br + }) + sort.Strings(bad) + var refs []string + for _, pref := range prefs { + refs = append(refs, pref.String()) + } + return append(refs, bad...) +} + +// refRank ranks precedence for reference type, preferring higher information references +// 1. Name + Tag + Digest +// 2. Name + Tag +// 3. Name + Digest +// 4. Name +// 5. Digest +// 6. Parse error +func refRank(ref docker.Reference) uint8 { + if _, ok := ref.(docker.Named); ok { + if _, ok = ref.(docker.Tagged); ok { + if _, ok = ref.(docker.Digested); ok { + return 1 + } + return 2 + } + if _, ok = ref.(docker.Digested); ok { + return 3 + } + return 4 + } + return 5 +} diff --git a/pkg/cri/store/image/sort_test.go b/pkg/cri/store/image/sort_test.go new file mode 100644 index 000000000..65f0d9194 --- /dev/null +++ b/pkg/cri/store/image/sort_test.go @@ -0,0 +1,84 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package image + +import ( + "io" + "io/ioutil" + "math/rand" + "testing" + + "github.com/opencontainers/go-digest" +) + +func TestReferenceSorting(t *testing.T) { + digested := func(seed int64) string { + b, err := ioutil.ReadAll(io.LimitReader(rand.New(rand.NewSource(seed)), 64)) + if err != nil { + panic(err) + } + return digest.FromBytes(b).String() + } + // Add z. prefix to string sort after "sha256:" + r1 := func(name, tag string, seed int64) string { + return "z.containerd.io/" + name + ":" + tag + "@" + digested(seed) + } + r2 := func(name, tag string) string { + return "z.containerd.io/" + name + ":" + tag + } + r3 := func(name string, seed int64) string { + return "z.containerd.io/" + name + "@" + digested(seed) + } + + for i, tc := range []struct { + unsorted []string + expected []string + }{ + { + unsorted: []string{r2("name", "latest"), r3("name", 1), r1("name", "latest", 1)}, + expected: []string{r1("name", "latest", 1), r2("name", "latest"), r3("name", 1)}, + }, + { + unsorted: []string{"can't parse this:latest", r3("name", 1), r2("name", "latest")}, + expected: []string{r2("name", "latest"), r3("name", 1), "can't parse this:latest"}, + }, + { + unsorted: []string{digested(1), r3("name", 1), r2("name", "latest")}, + expected: []string{r2("name", "latest"), r3("name", 1), digested(1)}, + }, + { + unsorted: []string{r2("name", "tag2"), r2("name", "tag3"), r2("name", "tag1")}, + expected: []string{r2("name", "tag1"), r2("name", "tag2"), r2("name", "tag3")}, + }, + { + unsorted: []string{r2("name-2", "tag"), r2("name-3", "tag"), r2("name-1", "tag")}, + expected: []string{r2("name-1", "tag"), r2("name-2", "tag"), r2("name-3", "tag")}, + }, + } { + sorted := sortReferences(tc.unsorted) + if len(sorted) != len(tc.expected) { + t.Errorf("[%d]: Mismatched sized, got %d, expected %d", i, len(sorted), len(tc.expected)) + continue + } + for j := range sorted { + if sorted[j] != tc.expected[j] { + t.Errorf("[%d]: Wrong value at %d, got %q, expected %q", i, j, sorted[j], tc.expected[j]) + break + } + } + } +}