Repository: containerd/containerd
File: internal/cri/server/images/image_pull.go (Go)

Code before:
// snapshotter and returns its ID if so.
// Returns the image ID on success, or an error if a pull is needed.
func (c *CRIImageService) getImageIDIfUnpacked(ctx context.Context, ref, snapshotter string) (string, error) {
	unpacked, err := c.IsImageUnpackedForSnapshotter(ctx, ref, snapshotter)
	if err != nil {
		return "", err
	}

	if !unpacked {
		return "", fmt.Errorf("image exists but not unpacked for snapshotter %q: %w", snapshotter, errdefs.ErrFailedPrecondition)
	}

	// Image is ready - get its ID
	existingImage, err := c.client.GetImage(ctx, ref)
	if err != nil {
		return "", err
	}

	configDesc, err := existingImage.Config(ctx)
	if err != nil {
		return "", err
	}

	imageID := configDesc.Digest.String()

	// Ensure image has proper labels and update image store
	if err := c.ensureImageLabels(ctx, existingImage, ref); err != nil {
		log.G(ctx).WithError(err).Warnf("Failed to ensure image labels for %s", ref)
	}
	if err := c.imageStore.Update(ctx, ref); err != nil {
		log.G(ctx).WithError(err).Warnf("Failed to update image store for %s", ref)
	}

	return imageID, nil
}

// pullWithStaleSnapshotRetry executes a pull operation and retries if it fails
// due to stale snapshots that can be cleaned up. It retries up to the number of
// layers in the image to handle cases where multiple layer snapshots are stale.
func (c *CRIImageService) pullWithStaleSnapshotRetry(
	ctx context.Context,
	ref string,
	snapshotter string,
	doPull func() (containerd.Image, error),
) (containerd.Image, error) {
	var image containerd.Image
	var err error

	// First attempt
	image, err = doPull()
	if err == nil {

Review comment (suggestion):
suggest moving the ensure labels step and the subsequent update all the way back in the caller in image pull


Code after:
			return "", "", fmt.Errorf("invalid decoded auth: %q", decoded)
		}
		return user, strings.Trim(passwd, "\x00"), nil
	}
	// TODO(random-liu): Support RegistryToken.
	// An empty auth config is valid for anonymous registry
	return "", "", nil
}

// createOrUpdateImageReference creates or updates image reference inside containerd image store.
// Note that because create and update are not finished in one transaction, there could be race. E.g.
// the image reference is deleted by someone else after create returns already exists, but before update
// happens.
func (c *CRIImageService) createOrUpdateImageReference(ctx context.Context, name string, desc imagespec.Descriptor, labels map[string]string) error {
	img := containerdimages.Image{
		Name:   name,
		Target: desc,
		// Add a label to indicate that the image is managed by the cri plugin.
		Labels: labels,
	}
	// TODO(random-liu): Figure out which is the more performant sequence create then update or
	// update then create.
	// TODO: Call CRIImageService directly
	_, err := c.images.Create(ctx, img)
	if err == nil {
		return nil
	} else if !errdefs.IsAlreadyExists(err) {
		return err
	}
	// Retrieve oldImg from image store here because Create routine returns an
	// empty image on ErrAlreadyExists
	oldImg, err := c.images.Get(ctx, name)
	if err != nil {
		return err
	}
	fieldpaths := []string{"target"}
	if oldImg.Labels[crilabels.ImageLabelKey] != labels[crilabels.ImageLabelKey] {
		fieldpaths = append(fieldpaths, "labels."+crilabels.ImageLabelKey)
	}
	if oldImg.Labels[crilabels.PinnedImageLabelKey] != labels[crilabels.PinnedImageLabelKey] &&
		labels[crilabels.PinnedImageLabelKey] == crilabels.PinnedImageLabelValue {
		fieldpaths = append(fieldpaths, "labels."+crilabels.PinnedImageLabelKey)
	}
	if oldImg.Target.Digest == img.Target.Digest && len(fieldpaths) < 2 {
		return nil
	}
	_, err = c.images.Update(ctx, img, fieldpaths...)
	return err
}

// getLabels get image labels to be added on CRI image