Add more comments to imgutil pull

Signed-off-by: Jin Dong <[email protected]>
containerd · Feb 8, 2023 · b584d31 · b584d31
1 parent 84e4cad
commit b584d31
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 18 deletions.
diff --git a/pkg/cmd/image/pull.go b/pkg/cmd/image/pull.go
@@ -35,6 +35,7 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// Pull pulls an image specified by `rawRef`.
 func Pull(ctx context.Context, client *containerd.Client, rawRef string, options types.ImagePullOptions) error {
 	ocispecPlatforms, err := platformutil.NewOCISpecPlatformSlice(options.AllPlatforms, options.Platform)
 	if err != nil {
@@ -54,8 +55,8 @@ func Pull(ctx context.Context, client *containerd.Client, rawRef string, options
 	return nil
 }
 
+// EnsureImage pulls an image either from ipfs or from registry.
 func EnsureImage(ctx context.Context, client *containerd.Client, rawRef string, ocispecPlatforms []v1.Platform, pull string, unpack *bool, quiet bool, options types.ImagePullOptions) (*imgutil.EnsuredImage, error) {
-
 	var ensured *imgutil.EnsuredImage
 
 	if scheme, ref, err := referenceutil.ParseIPFSRefWithScheme(rawRef); err == nil {

diff --git a/pkg/imgutil/imgutil.go b/pkg/imgutil/imgutil.go
@@ -42,6 +42,7 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// EnsuredImage contains the image existed in containerd and its metadata.
 type EnsuredImage struct {
 	Ref         string
 	Image       containerd.Image
@@ -53,7 +54,7 @@ type EnsuredImage struct {
 // PullMode is either one of "always", "missing", "never"
 type PullMode = string
 
-// GetExistingImage returns the specified image if exists in containerd. May return errdefs.NotFound() if not exists.
+// GetExistingImage returns the specified image if exists in containerd. Return errdefs.NotFound() if not exists.
 func GetExistingImage(ctx context.Context, client *containerd.Client, snapshotter, rawRef string, platform ocispec.Platform) (*EnsuredImage, error) {
 	var res *EnsuredImage
 	imagewalker := &imagewalker.ImageWalker{
@@ -109,18 +110,18 @@ func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr
 	default:
 		return nil, fmt.Errorf("unexpected pull mode: %q", mode)
 	}
+
+	// if not `always` pull and given one platform and image found locally, return existing image directly.
 	if mode != "always" && len(ocispecPlatforms) == 1 {
-		res, err := GetExistingImage(ctx, client, snapshotter, rawRef, ocispecPlatforms[0])
-		if err == nil {
+		if res, err := GetExistingImage(ctx, client, snapshotter, rawRef, ocispecPlatforms[0]); err == nil {
 			return res, nil
-		}
-		if !errdefs.IsNotFound(err) {
+		} else if !errdefs.IsNotFound(err) {
 			return nil, err
 		}
 	}
 
 	if mode == "never" {
-		return nil, fmt.Errorf("image %q is not available", rawRef)
+		return nil, fmt.Errorf("image not available: %q", rawRef)
 	}
 
 	named, err := refdocker.ParseDockerRef(rawRef)
@@ -164,6 +165,7 @@ func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr
 	return img, nil
 }
 
+// ResolveDigest resolves `rawRef` and returns its descriptor digest.
 func ResolveDigest(ctx context.Context, rawRef string, insecure bool, hostsDirs []string) (string, error) {
 	named, err := refdocker.ParseDockerRef(rawRef)
 	if err != nil {
@@ -209,14 +211,13 @@ func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io
 		config.ProgressOutput = stderr
 	}
 
-	var unpackB bool
+	// unpack(B) if given 1 platform unless specified by `unpack`
+	unpackB := len(ocispecPlatforms) == 1
 	if unpack != nil {
 		unpackB = *unpack
 		if unpackB && len(ocispecPlatforms) != 1 {
 			return nil, fmt.Errorf("unpacking requires a single platform to be specified (e.g., --platform=amd64)")
 		}
-	} else {
-		unpackB = len(ocispecPlatforms) == 1
 	}
 
 	snOpt := getSnapshotterOpts(snapshotter)
@@ -233,6 +234,7 @@ func PullImage(ctx context.Context, client *containerd.Client, stdout, stderr io
 	} else {
 		logrus.Debugf("The image will not be unpacked. Platforms=%v.", ocispecPlatforms)
 	}
+
 	containerdImage, err = pull.Pull(ctx, client, ref, config)
 	if err != nil {
 		return nil, err
@@ -274,8 +276,7 @@ func getImageConfig(ctx context.Context, image containerd.Image) (*ocispec.Image
 	}
 }
 
-// ReadIndex returns the index .
-// ReadIndex returns nil for non-indexed image.
+// ReadIndex returns image index, or nil for non-indexed image.
 func ReadIndex(ctx context.Context, img containerd.Image) (*ocispec.Index, *ocispec.Descriptor, error) {
 	desc := img.Target()
 	if !images.IsIndexType(desc.MediaType) {
@@ -293,8 +294,7 @@ func ReadIndex(ctx context.Context, img containerd.Image) (*ocispec.Index, *ocis
 	return &idx, &desc, nil
 }
 
-// ReadManifest returns the manifest for img.platform.
-// ReadManifest returns nil if no manifest was found.
+// ReadManifest returns the manifest for img.platform, or nil if no manifest was found.
 func ReadManifest(ctx context.Context, img containerd.Image) (*ocispec.Manifest, *ocispec.Descriptor, error) {
 	cs := img.ContentStore()
 	targetDesc := img.Target()
@@ -356,6 +356,7 @@ func ReadImageConfig(ctx context.Context, img containerd.Image) (ocispec.Image,
 	return config, configDesc, nil
 }
 
+// ParseRepoTag parses raw `imgName` to repository and tag.
 func ParseRepoTag(imgName string) (string, string) {
 	logrus.Debugf("raw image name=%q", imgName)
 

diff --git a/pkg/ipfs/image.go b/pkg/ipfs/image.go
@@ -54,12 +54,11 @@ func EnsureImage(ctx context.Context, client *containerd.Client, stdout, stderr
 		return nil, fmt.Errorf("unexpected scheme: %q", scheme)
 	}
 
+	// if not `always` pull and given one platform and image found locally, return existing image directly.
 	if mode != "always" && len(ocispecPlatforms) == 1 {
-		res, err := imgutil.GetExistingImage(ctx, client, snapshotter, ref, ocispecPlatforms[0])
-		if err == nil {
+		if res, err := imgutil.GetExistingImage(ctx, client, snapshotter, ref, ocispecPlatforms[0]); err == nil {
 			return res, nil
-		}
-		if !errdefs.IsNotFound(err) {
+		} else if !errdefs.IsNotFound(err) {
 			return nil, err
 		}
 	}