Browse code

Add resolve all images and unit test

Add single resolve function to get a consistent list of images matching
the same digest.

Signed-off-by: Derek McGowan <derek@mcg.dev>

Derek McGowan authored on 2023/11/17 14:33:11
Showing 6 changed files
... ...
@@ -29,6 +29,8 @@ import (
29 29
 
30 30
 var truncatedID = regexp.MustCompile(`^(sha256:)?([a-f0-9]{4,64})$`)
31 31
 
32
+var errInconsistentData error = errors.New("consistency error: data changed during operation, retry")
33
+
32 34
 // GetImage returns an image corresponding to the image referred to by refOrID.
33 35
 func (i *ImageService) GetImage(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (*image.Image, error) {
34 36
 	desc, err := i.resolveImage(ctx, refOrID)
... ...
@@ -382,3 +384,150 @@ func (i *ImageService) getImageLabelByDigest(ctx context.Context, target digest.
382 382
 
383 383
 	return value, nil
384 384
 }
385
+
386
+func convertError(err error) error {
387
+	// TODO: Convert containerd error to Docker error
388
+	return err
389
+}
390
+
391
+// resolveAllReferences resolves the reference name or ID to an image and returns all the images with
392
+// the same target.
393
+//
394
+// Returns:
395
+//
396
+// 1: *(github.com/containerd/containerd/images).Image
397
+//
398
+//	An image match from the image store with the provided refOrID
399
+//
400
+// 2: [](github.com/containerd/containerd/images).Image
401
+//
402
+//	List of all images with the same target that matches the refOrID. If the first argument is
403
+//	non-nil, the image list will all have the same target as the matched image. If the first
404
+//	argument is nil but the list is non-empty, this value is a list of all the images with a
405
+//	target that matches the digest provided in the refOrID, but none are an image name match
406
+//	to refOrID.
407
+//
408
+// 3: error
409
+//
410
+//	An error looking up refOrID or no images found with matching name or target. Note that the first
411
+//	argument may be nil with a nil error if the second argument is non-empty.
412
+func (i *ImageService) resolveAllReferences(ctx context.Context, refOrID string) (*containerdimages.Image, []containerdimages.Image, error) {
413
+	parsed, err := reference.ParseAnyReference(refOrID)
414
+	if err != nil {
415
+		return nil, nil, errdefs.InvalidParameter(err)
416
+	}
417
+	var dgst digest.Digest
418
+	var img *containerdimages.Image
419
+
420
+	if truncatedID.MatchString(refOrID) {
421
+		if d, ok := parsed.(reference.Digested); ok {
422
+			if cimg, err := i.images.Get(ctx, d.String()); err == nil {
423
+				img = &cimg
424
+				dgst = d.Digest()
425
+				if cimg.Target.Digest != dgst {
426
+					// Ambiguous image reference, use reference name
427
+					log.G(ctx).WithField("image", refOrID).WithField("target", cimg.Target.Digest).Warn("digest reference points to image with a different digest")
428
+					dgst = cimg.Target.Digest
429
+				}
430
+			} else if !cerrdefs.IsNotFound(err) {
431
+				return nil, nil, convertError(err)
432
+			} else {
433
+				dgst = d.Digest()
434
+			}
435
+		} else {
436
+			idWithoutAlgo := strings.TrimPrefix(refOrID, "sha256:")
437
+			name := reference.TagNameOnly(parsed.(reference.Named)).String()
438
+			filters := []string{
439
+				fmt.Sprintf("name==%q", name), // Or it could just look like one.
440
+				"target.digest~=" + strconv.Quote(fmt.Sprintf(`^sha256:%s[0-9a-fA-F]{%d}$`, regexp.QuoteMeta(idWithoutAlgo), 64-len(idWithoutAlgo))),
441
+			}
442
+			imgs, err := i.images.List(ctx, filters...)
443
+			if err != nil {
444
+				return nil, nil, convertError(err)
445
+			}
446
+
447
+			if len(imgs) == 0 {
448
+				return nil, nil, images.ErrImageDoesNotExist{Ref: parsed}
449
+			}
450
+
451
+			for _, limg := range imgs {
452
+				if limg.Name == name {
453
+					copyImg := limg
454
+					img = &copyImg
455
+				}
456
+				if dgst != "" {
457
+					if limg.Target.Digest != dgst {
458
+						return nil, nil, errdefs.NotFound(errors.New("ambiguous reference"))
459
+					}
460
+				} else {
461
+					dgst = limg.Target.Digest
462
+				}
463
+			}
464
+
465
+			// Return immediately if target digest matches already included
466
+			if img == nil || len(imgs) > 1 {
467
+				return img, imgs, nil
468
+			}
469
+		}
470
+	} else {
471
+		named, ok := parsed.(reference.Named)
472
+		if !ok {
473
+			return nil, nil, errdefs.InvalidParameter(errors.New("invalid name reference"))
474
+		}
475
+
476
+		digested, ok := parsed.(reference.Digested)
477
+		if ok {
478
+			dgst = digested.Digest()
479
+		}
480
+
481
+		name := reference.TagNameOnly(named).String()
482
+
483
+		cimg, err := i.images.Get(ctx, name)
484
+		if err != nil {
485
+			if !cerrdefs.IsNotFound(err) {
486
+				return nil, nil, convertError(err)
487
+			}
488
+			return nil, nil, images.ErrImageDoesNotExist{Ref: parsed}
489
+		} else {
490
+			img = &cimg
491
+			if dgst != "" && img.Target.Digest != dgst {
492
+				// Ambiguous image reference, use reference name
493
+				log.G(ctx).WithField("image", name).WithField("target", cimg.Target.Digest).Warn("digest reference points to image with a different digest")
494
+			}
495
+			dgst = img.Target.Digest
496
+		}
497
+
498
+	}
499
+
500
+	// Lookup up all associated images and check for consistency with first reference
501
+	// Ideally operations dependent on multiple values will rely on the garbage collector,
502
+	// this logic will just check for consistency and throw an error
503
+	imgs, err := i.images.List(ctx, "target.digest=="+dgst.String())
504
+	if err != nil {
505
+		return nil, nil, errors.Wrap(err, "failed to lookup digest")
506
+	}
507
+	if len(imgs) == 0 {
508
+		if img == nil {
509
+			return nil, nil, images.ErrImageDoesNotExist{Ref: parsed}
510
+		}
511
+		err = errInconsistentData
512
+	} else if img != nil {
513
+		// Check to ensure the original img is in the list still
514
+		err = errInconsistentData
515
+		for _, rimg := range imgs {
516
+			if rimg.Name == img.Name {
517
+				err = nil
518
+				break
519
+			}
520
+		}
521
+	}
522
+	if errors.Is(err, errInconsistentData) {
523
+		if retries, ok := ctx.Value(errInconsistentData).(int); !ok || retries < 3 {
524
+			log.G(ctx).WithFields(log.Fields{"retry": retries, "ref": refOrID}).Info("image changed during lookup, retrying")
525
+			return i.resolveAllReferences(context.WithValue(ctx, errInconsistentData, retries+1), refOrID)
526
+		}
527
+		return nil, nil, err
528
+	}
529
+
530
+	return img, imgs, nil
531
+}
385 532
new file mode 100644
... ...
@@ -0,0 +1,287 @@
0
+package containerd
1
+
2
+import (
3
+	"context"
4
+	"io"
5
+	"math/rand"
6
+	"path/filepath"
7
+	"testing"
8
+
9
+	"github.com/containerd/containerd/images"
10
+	"github.com/containerd/containerd/metadata"
11
+	"github.com/containerd/containerd/namespaces"
12
+	"github.com/containerd/log/logtest"
13
+	"github.com/distribution/reference"
14
+	dockerimages "github.com/docker/docker/daemon/images"
15
+	"github.com/opencontainers/go-digest"
16
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
17
+
18
+	"go.etcd.io/bbolt"
19
+
20
+	"gotest.tools/v3/assert"
21
+	is "gotest.tools/v3/assert/cmp"
22
+)
23
+
24
+func TestLookup(t *testing.T) {
25
+	ctx := namespaces.WithNamespace(context.TODO(), "testing")
26
+	ctx = logtest.WithT(ctx, t)
27
+	mdb := newTestDB(ctx, t)
28
+	service := &ImageService{
29
+		images: metadata.NewImageStore(mdb),
30
+	}
31
+
32
+	ubuntuLatest := images.Image{
33
+		Name:   "docker.io/library/ubuntu:latest",
34
+		Target: desc(10),
35
+	}
36
+	ubuntuLatestWithDigest := images.Image{
37
+		Name:   "docker.io/library/ubuntu:latest@" + digestFor(10).String(),
38
+		Target: desc(10),
39
+	}
40
+	ubuntuLatestWithOldDigest := images.Image{
41
+		Name:   "docker.io/library/ubuntu:latest@" + digestFor(11).String(),
42
+		Target: desc(11),
43
+	}
44
+	ambiguousShortName := images.Image{
45
+		Name:   "docker.io/library/abcdef:latest",
46
+		Target: desc(12),
47
+	}
48
+	ambiguousShortNameWithDigest := images.Image{
49
+		Name:   "docker.io/library/abcdef:latest@" + digestFor(12).String(),
50
+		Target: desc(12),
51
+	}
52
+	shortNameIsHashAlgorithm := images.Image{
53
+		Name:   "docker.io/library/sha256:defcab",
54
+		Target: desc(13),
55
+	}
56
+
57
+	testImages := []images.Image{
58
+		ubuntuLatest,
59
+		ubuntuLatestWithDigest,
60
+		ubuntuLatestWithOldDigest,
61
+		ambiguousShortName,
62
+		ambiguousShortNameWithDigest,
63
+		shortNameIsHashAlgorithm,
64
+		{
65
+			Name:   "docker.io/test/volatile:retried",
66
+			Target: desc(14),
67
+		},
68
+		{
69
+			Name:   "docker.io/test/volatile:inconsistent",
70
+			Target: desc(15),
71
+		},
72
+	}
73
+	for _, img := range testImages {
74
+		if _, err := service.images.Create(ctx, img); err != nil {
75
+			t.Fatalf("failed to create image %q: %v", img.Name, err)
76
+		}
77
+	}
78
+
79
+	for _, tc := range []struct {
80
+		lookup string
81
+		img    *images.Image
82
+		all    []images.Image
83
+		err    error
84
+	}{
85
+		{
86
+			// Get ubuntu images with default "latest" tag
87
+			lookup: "ubuntu",
88
+			img:    &ubuntuLatest,
89
+			all:    []images.Image{ubuntuLatest, ubuntuLatestWithDigest},
90
+		},
91
+		{
92
+			// Get all images by image id
93
+			lookup: ubuntuLatest.Target.Digest.String(),
94
+			img:    nil,
95
+			all:    []images.Image{ubuntuLatest, ubuntuLatestWithDigest},
96
+		},
97
+		{
98
+			// Fail to lookup reference with no tag, reference has both tag and digest
99
+			lookup: "ubuntu@" + ubuntuLatestWithOldDigest.Target.Digest.String(),
100
+			err:    dockerimages.ErrImageDoesNotExist{Ref: nameDigest("ubuntu", ubuntuLatestWithOldDigest.Target.Digest)},
101
+		},
102
+		{
103
+			// Get all image with both tag and digest
104
+			lookup: "ubuntu:latest@" + ubuntuLatestWithOldDigest.Target.Digest.String(),
105
+			img:    &ubuntuLatestWithOldDigest,
106
+			all:    []images.Image{ubuntuLatestWithOldDigest},
107
+		},
108
+		{
109
+			// Get abcdef image which also matches short image id
110
+			lookup: "abcdef",
111
+			img:    &ambiguousShortName,
112
+			all:    []images.Image{ambiguousShortName, ambiguousShortNameWithDigest},
113
+		},
114
+		{
115
+			// Fail to lookup image named "sha256" with tag that doesn't exist
116
+			lookup: "sha256:abcdef",
117
+			err:    dockerimages.ErrImageDoesNotExist{Ref: nameTag("sha256", "abcdef")},
118
+		},
119
+		{
120
+			// Lookup with shortened image id
121
+			lookup: ambiguousShortName.Target.Digest.Encoded()[:8],
122
+			img:    nil,
123
+			all:    []images.Image{ambiguousShortName, ambiguousShortNameWithDigest},
124
+		},
125
+		{
126
+			// Lookup an actual image named "sha256" in the default namespace
127
+			lookup: "sha256:defcab",
128
+			img:    &shortNameIsHashAlgorithm,
129
+			all:    []images.Image{shortNameIsHashAlgorithm},
130
+		},
131
+	} {
132
+		tc := tc
133
+		t.Run(tc.lookup, func(t *testing.T) {
134
+			t.Parallel()
135
+			img, all, err := service.resolveAllReferences(ctx, tc.lookup)
136
+			if tc.err == nil {
137
+				assert.NilError(t, err)
138
+			} else {
139
+				assert.Error(t, err, tc.err.Error())
140
+			}
141
+			if tc.img == nil {
142
+				assert.Assert(t, is.Nil(img))
143
+			} else {
144
+				assert.Assert(t, img != nil)
145
+				assert.Check(t, is.Equal(img.Name, tc.img.Name))
146
+				assert.Check(t, is.Equal(img.Target.Digest, tc.img.Target.Digest))
147
+			}
148
+
149
+			assert.Assert(t, is.Len(tc.all, len(all)))
150
+
151
+			// Order should match
152
+			for i := range all {
153
+				assert.Check(t, is.Equal(all[i].Name, tc.all[i].Name), "image[%d]", i)
154
+				assert.Check(t, is.Equal(all[i].Target.Digest, tc.all[i].Target.Digest), "image[%d]", i)
155
+			}
156
+		})
157
+	}
158
+
159
+	t.Run("fail-inconsistency", func(t *testing.T) {
160
+		service := &ImageService{
161
+			images: &mutateOnGetImageStore{
162
+				Store: service.images,
163
+				getMutations: []images.Image{
164
+					{
165
+						Name:   "docker.io/test/volatile:inconsistent",
166
+						Target: desc(18),
167
+					},
168
+					{
169
+						Name:   "docker.io/test/volatile:inconsistent",
170
+						Target: desc(19),
171
+					},
172
+					{
173
+						Name:   "docker.io/test/volatile:inconsistent",
174
+						Target: desc(20),
175
+					},
176
+					{
177
+						Name:   "docker.io/test/volatile:inconsistent",
178
+						Target: desc(21),
179
+					},
180
+					{
181
+						Name:   "docker.io/test/volatile:inconsistent",
182
+						Target: desc(22),
183
+					},
184
+				},
185
+				t: t,
186
+			},
187
+		}
188
+
189
+		_, _, err := service.resolveAllReferences(ctx, "test/volatile:inconsistent")
190
+		assert.ErrorIs(t, err, errInconsistentData)
191
+	})
192
+
193
+	t.Run("retry-inconsistency", func(t *testing.T) {
194
+		service := &ImageService{
195
+			images: &mutateOnGetImageStore{
196
+				Store: service.images,
197
+				getMutations: []images.Image{
198
+					{
199
+						Name:   "docker.io/test/volatile:retried",
200
+						Target: desc(16),
201
+					},
202
+					{
203
+						Name:   "docker.io/test/volatile:retried",
204
+						Target: desc(17),
205
+					},
206
+				},
207
+				t: t,
208
+			},
209
+		}
210
+
211
+		img, all, err := service.resolveAllReferences(ctx, "test/volatile:retried")
212
+		assert.NilError(t, err)
213
+
214
+		assert.Assert(t, img != nil)
215
+		assert.Check(t, is.Equal(img.Name, "docker.io/test/volatile:retried"))
216
+		assert.Check(t, is.Equal(img.Target.Digest, digestFor(17)))
217
+		assert.Assert(t, is.Len(all, 1))
218
+		assert.Check(t, is.Equal(all[0].Name, "docker.io/test/volatile:retried"))
219
+		assert.Check(t, is.Equal(all[0].Target.Digest, digestFor(17)))
220
+	})
221
+}
222
+
223
+type mutateOnGetImageStore struct {
224
+	images.Store
225
+	getMutations []images.Image
226
+	t            *testing.T
227
+}
228
+
229
+func (m *mutateOnGetImageStore) Get(ctx context.Context, name string) (images.Image, error) {
230
+	img, err := m.Store.Get(ctx, name)
231
+	if len(m.getMutations) > 0 {
232
+		m.Store.Update(ctx, m.getMutations[0])
233
+		m.getMutations = m.getMutations[1:]
234
+		m.t.Logf("Get %s", name)
235
+	}
236
+	return img, err
237
+}
238
+
239
+func nameDigest(name string, dgst digest.Digest) reference.Reference {
240
+	named, _ := reference.WithName(name)
241
+	digested, _ := reference.WithDigest(named, dgst)
242
+	return digested
243
+}
244
+
245
+func nameTag(name, tag string) reference.Reference {
246
+	named, _ := reference.WithName(name)
247
+	tagged, _ := reference.WithTag(named, tag)
248
+	return tagged
249
+}
250
+
251
+func desc(size int64) ocispec.Descriptor {
252
+	return ocispec.Descriptor{
253
+		Digest:    digestFor(size),
254
+		Size:      size,
255
+		MediaType: ocispec.MediaTypeImageIndex,
256
+	}
257
+
258
+}
259
+
260
+func digestFor(i int64) digest.Digest {
261
+	r := rand.New(rand.NewSource(i))
262
+	dgstr := digest.SHA256.Digester()
263
+	_, err := io.Copy(dgstr.Hash(), io.LimitReader(r, i))
264
+	if err != nil {
265
+		panic(err)
266
+	}
267
+	return dgstr.Digest()
268
+}
269
+
270
+func newTestDB(ctx context.Context, t *testing.T) *metadata.DB {
271
+	t.Helper()
272
+
273
+	p := filepath.Join(t.TempDir(), "metadata")
274
+	bdb, err := bbolt.Open(p, 0600, &bbolt.Options{})
275
+	if err != nil {
276
+		t.Fatal(err)
277
+	}
278
+	t.Cleanup(func() { bdb.Close() })
279
+
280
+	mdb := metadata.NewDB(bdb, nil, nil)
281
+	if err := mdb.Init(ctx); err != nil {
282
+		t.Fatal(err)
283
+	}
284
+
285
+	return mdb
286
+}
... ...
@@ -7,6 +7,7 @@ import (
7 7
 
8 8
 	"github.com/containerd/containerd"
9 9
 	cerrdefs "github.com/containerd/containerd/errdefs"
10
+	"github.com/containerd/containerd/images"
10 11
 	"github.com/containerd/containerd/plugin"
11 12
 	"github.com/containerd/containerd/remotes/docker"
12 13
 	"github.com/containerd/containerd/snapshots"
... ...
@@ -14,7 +15,7 @@ import (
14 14
 	"github.com/distribution/reference"
15 15
 	"github.com/docker/docker/container"
16 16
 	daemonevents "github.com/docker/docker/daemon/events"
17
-	"github.com/docker/docker/daemon/images"
17
+	daemonimages "github.com/docker/docker/daemon/images"
18 18
 	"github.com/docker/docker/daemon/snapshotter"
19 19
 	"github.com/docker/docker/errdefs"
20 20
 	"github.com/docker/docker/image"
... ...
@@ -27,6 +28,7 @@ import (
27 27
 // ImageService implements daemon.ImageService
28 28
 type ImageService struct {
29 29
 	client          *containerd.Client
30
+	images          images.Store
30 31
 	containers      container.Store
31 32
 	snapshotter     string
32 33
 	registryHosts   docker.RegistryHosts
... ...
@@ -59,6 +61,7 @@ type ImageServiceConfig struct {
59 59
 func NewService(config ImageServiceConfig) *ImageService {
60 60
 	return &ImageService{
61 61
 		client:          config.Client,
62
+		images:          config.Client.ImageService(),
62 63
 		containers:      config.Containers,
63 64
 		snapshotter:     config.Snapshotter,
64 65
 		registryHosts:   config.RegistryHosts,
... ...
@@ -70,8 +73,8 @@ func NewService(config ImageServiceConfig) *ImageService {
70 70
 }
71 71
 
72 72
 // DistributionServices return services controlling daemon image storage.
73
-func (i *ImageService) DistributionServices() images.DistributionServices {
74
-	return images.DistributionServices{}
73
+func (i *ImageService) DistributionServices() daemonimages.DistributionServices {
74
+	return daemonimages.DistributionServices{}
75 75
 }
76 76
 
77 77
 // CountImages returns the number of images stored by ImageService
78 78
new file mode 100644
... ...
@@ -0,0 +1,56 @@
0
+/*
1
+   Copyright The containerd Authors.
2
+
3
+   Licensed under the Apache License, Version 2.0 (the "License");
4
+   you may not use this file except in compliance with the License.
5
+   You may obtain a copy of the License at
6
+
7
+       http://www.apache.org/licenses/LICENSE-2.0
8
+
9
+   Unless required by applicable law or agreed to in writing, software
10
+   distributed under the License is distributed on an "AS IS" BASIS,
11
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12
+   See the License for the specific language governing permissions and
13
+   limitations under the License.
14
+*/
15
+
16
+package logtest
17
+
18
+import (
19
+	"context"
20
+	"fmt"
21
+	"io"
22
+	"path/filepath"
23
+	"runtime"
24
+	"testing"
25
+
26
+	"github.com/containerd/log"
27
+	"github.com/sirupsen/logrus"
28
+)
29
+
30
+// WithT adds a logging hook for the given test
31
+// Changes debug level to debug, clears output, and
32
+// outputs all log messages as test logs.
33
+func WithT(ctx context.Context, t testing.TB) context.Context {
34
+	// Create a new logger to avoid adding hooks from multiple tests
35
+	l := logrus.New()
36
+
37
+	// Increase debug level for tests
38
+	l.SetLevel(logrus.DebugLevel)
39
+	l.SetOutput(io.Discard)
40
+	l.SetReportCaller(true)
41
+
42
+	// Add testing hook
43
+	l.AddHook(&testHook{
44
+		t: t,
45
+		fmt: &logrus.TextFormatter{
46
+			DisableColors:   true,
47
+			TimestampFormat: log.RFC3339NanoFixed,
48
+			CallerPrettyfier: func(frame *runtime.Frame) (string, string) {
49
+				return filepath.Base(frame.Function), fmt.Sprintf("%s:%d", frame.File, frame.Line)
50
+			},
51
+		},
52
+	})
53
+
54
+	return log.WithLogger(ctx, logrus.NewEntry(l).WithField("testcase", t.Name()))
55
+}
0 56
new file mode 100644
... ...
@@ -0,0 +1,50 @@
0
+/*
1
+   Copyright The containerd Authors.
2
+
3
+   Licensed under the Apache License, Version 2.0 (the "License");
4
+   you may not use this file except in compliance with the License.
5
+   You may obtain a copy of the License at
6
+
7
+       http://www.apache.org/licenses/LICENSE-2.0
8
+
9
+   Unless required by applicable law or agreed to in writing, software
10
+   distributed under the License is distributed on an "AS IS" BASIS,
11
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12
+   See the License for the specific language governing permissions and
13
+   limitations under the License.
14
+*/
15
+
16
+package logtest
17
+
18
+import (
19
+	"bytes"
20
+	"sync"
21
+	"testing"
22
+
23
+	"github.com/sirupsen/logrus"
24
+)
25
+
26
+type testHook struct {
27
+	t   testing.TB
28
+	fmt logrus.Formatter
29
+	mu  sync.Mutex
30
+}
31
+
32
+func (*testHook) Levels() []logrus.Level {
33
+	return logrus.AllLevels
34
+}
35
+
36
+func (h *testHook) Fire(e *logrus.Entry) error {
37
+	s, err := h.fmt.Format(e)
38
+	if err != nil {
39
+		return err
40
+	}
41
+
42
+	// Because the logger could be called from multiple goroutines,
43
+	// but t.Log() is not designed for.
44
+	h.mu.Lock()
45
+	defer h.mu.Unlock()
46
+	h.t.Log(string(bytes.TrimRight(s, "\n")))
47
+
48
+	return nil
49
+}
... ...
@@ -359,6 +359,7 @@ github.com/containerd/go-runc
359 359
 # github.com/containerd/log v0.1.0
360 360
 ## explicit; go 1.20
361 361
 github.com/containerd/log
362
+github.com/containerd/log/logtest
362 363
 # github.com/containerd/nydus-snapshotter v0.8.2
363 364
 ## explicit; go 1.19
364 365
 github.com/containerd/nydus-snapshotter/pkg/converter