Skip to content

Commit 26c0574

Browse files
henry118henry118
committed
bug fix: make sure cri image is pinned when it is pulled outside cri
Signed-off-by: Henry Wang <[email protected]> (cherry picked from commit 1eaf0c1)
1 parent e52570c commit 26c0574

3 files changed

Lines changed: 65 additions & 8 deletions

File tree

‎integration/containerd_image_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,35 @@ func TestContainerdSandboxImage(t *testing.T) {
237237
assert.True(t, pimg.Pinned)
238238
}
239239

240+
func TestContainerdSandboxImagePulledOutsideCRI(t *testing.T) {
241+
var pauseImage = images.Get(images.Pause)
242+
ctx := context.Background()
243+
244+
t.Log("make sure the pause image does not exist")
245+
imageService.RemoveImage(&runtime.ImageSpec{Image: pauseImage})
246+
247+
t.Log("pull pause image")
248+
_, err := containerdClient.Pull(ctx, pauseImage)
249+
assert.NoError(t, err)
250+
251+
t.Log("pause image should be seen by cri plugin")
252+
var pimg *runtime.Image
253+
require.NoError(t, Eventually(func() (bool, error) {
254+
pimg, err = imageService.ImageStatus(&runtime.ImageSpec{Image: pauseImage})
255+
return pimg != nil, err
256+
}, time.Second, 10*time.Second))
257+
258+
t.Log("verify pinned field is set for pause image")
259+
assert.True(t, pimg.Pinned)
260+
261+
t.Log("make sure the pause image exist")
262+
pauseImg, err := containerdClient.GetImage(ctx, pauseImage)
263+
require.NoError(t, err)
264+
265+
t.Log("ensure correct labels are set on pause image")
266+
assert.Equal(t, "pinned", pauseImg.Labels()["io.cri-containerd.pinned"])
267+
}
268+
240269
func TestContainerdImageWithDockerSchema1(t *testing.T) {
241270
if goruntime.GOOS == "windows" {
242271
t.Skip("Skipped on Windows because the test image is not a multi-platform one.")

‎pkg/cri/sbserver/image_pull.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,14 +279,28 @@ func (c *criService) createImageReference(ctx context.Context, name string, desc
279279
}
280280
// TODO(random-liu): Figure out which is the more performant sequence create then update or
281281
// update then create.
282-
oldImg, err := c.client.ImageService().Create(ctx, img)
282+
_, err := c.client.ImageService().Create(ctx, img)
283283
if err == nil || !errdefs.IsAlreadyExists(err) {
284284
return err
285285
}
286-
if oldImg.Target.Digest == img.Target.Digest && oldImg.Labels[crilabels.ImageLabelKey] == labels[crilabels.ImageLabelKey] {
286+
// Retrieve oldImg from image store here because Create routine returns an
287+
// empty image on ErrAlreadyExists
288+
oldImg, err := c.client.ImageService().Get(ctx, name)
289+
if err != nil {
290+
return err
291+
}
292+
fieldpaths := []string{"target"}
293+
if oldImg.Labels[crilabels.ImageLabelKey] != labels[crilabels.ImageLabelKey] {
294+
fieldpaths = append(fieldpaths, "labels."+crilabels.ImageLabelKey)
295+
}
296+
if oldImg.Labels[crilabels.PinnedImageLabelKey] != labels[crilabels.PinnedImageLabelKey] &&
297+
labels[crilabels.PinnedImageLabelKey] == crilabels.PinnedImageLabelValue {
298+
fieldpaths = append(fieldpaths, "labels."+crilabels.PinnedImageLabelKey)
299+
}
300+
if oldImg.Target.Digest == img.Target.Digest && len(fieldpaths) < 2 {
287301
return nil
288302
}
289-
_, err = c.client.ImageService().Update(ctx, img, "target", "labels."+crilabels.ImageLabelKey)
303+
_, err = c.client.ImageService().Update(ctx, img, fieldpaths...)
290304
return err
291305
}
292306

@@ -324,7 +338,7 @@ func (c *criService) updateImage(ctx context.Context, r string) error {
324338
return fmt.Errorf("get image id: %w", err)
325339
}
326340
id := configDesc.Digest.String()
327-
labels := c.getLabels(ctx, id)
341+
labels := c.getLabels(ctx, r)
328342
if err := c.createImageReference(ctx, id, img.Target(), labels); err != nil {
329343
return fmt.Errorf("create image id reference %q: %w", id, err)
330344
}

‎pkg/cri/server/image_pull.go‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,14 +281,28 @@ func (c *criService) createImageReference(ctx context.Context, name string, desc
281281
}
282282
// TODO(random-liu): Figure out which is the more performant sequence create then update or
283283
// update then create.
284-
oldImg, err := c.client.ImageService().Create(ctx, img)
284+
_, err := c.client.ImageService().Create(ctx, img)
285285
if err == nil || !errdefs.IsAlreadyExists(err) {
286286
return err
287287
}
288-
if oldImg.Target.Digest == img.Target.Digest && oldImg.Labels[crilabels.ImageLabelKey] == labels[crilabels.ImageLabelKey] {
288+
// Retrieve oldImg from image store here because Create routine returns an
289+
// empty image on ErrAlreadyExists
290+
oldImg, err := c.client.ImageService().Get(ctx, name)
291+
if err != nil {
292+
return err
293+
}
294+
fieldpaths := []string{"target"}
295+
if oldImg.Labels[crilabels.ImageLabelKey] != labels[crilabels.ImageLabelKey] {
296+
fieldpaths = append(fieldpaths, "labels."+crilabels.ImageLabelKey)
297+
}
298+
if oldImg.Labels[crilabels.PinnedImageLabelKey] != labels[crilabels.PinnedImageLabelKey] &&
299+
labels[crilabels.PinnedImageLabelKey] == crilabels.PinnedImageLabelValue {
300+
fieldpaths = append(fieldpaths, "labels."+crilabels.PinnedImageLabelKey)
301+
}
302+
if oldImg.Target.Digest == img.Target.Digest && len(fieldpaths) < 2 {
289303
return nil
290304
}
291-
_, err = c.client.ImageService().Update(ctx, img, "target", "labels."+crilabels.ImageLabelKey)
305+
_, err = c.client.ImageService().Update(ctx, img, fieldpaths...)
292306
return err
293307
}
294308

@@ -326,7 +340,7 @@ func (c *criService) updateImage(ctx context.Context, r string) error {
326340
return fmt.Errorf("get image id: %w", err)
327341
}
328342
id := configDesc.Digest.String()
329-
labels := c.getLabels(ctx, id)
343+
labels := c.getLabels(ctx, r)
330344
if err := c.createImageReference(ctx, id, img.Target(), labels); err != nil {
331345
return fmt.Errorf("create image id reference %q: %w", id, err)
332346
}

0 commit comments

Comments
 (0)