isula-build/patch/0045-fix-images-command-when-only-give-repository.patch

From 4e71f4409e53eadea0aa39383fba3e249072a932 Mon Sep 17 00:00:00 2001
From: meilier <xingweizheng@huawei.com>
Date: Tue, 2 Feb 2021 00:46:23 +0800
Subject: [PATCH 07/10] fix images command when only give repository

---
 daemon/images.go      | 145 +++++++++++++++++++++-------------
 daemon/images_test.go | 178 ++++++++++++++++++++++++++++++++++++++++++
 image/image.go        |   9 ++-
 3 files changed, 277 insertions(+), 55 deletions(-)
 create mode 100644 daemon/images_test.go

diff --git a/daemon/images.go b/daemon/images.go
index 5560d18c..e61817cc 100644
--- a/daemon/images.go
+++ b/daemon/images.go
@@ -15,9 +15,11 @@ package daemon
 
 import (
 	"context"
+	"fmt"
 	"sort"
 	"strings"
 
+	"github.com/containers/storage"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 
@@ -29,79 +31,114 @@ import (
 )
 
 const (
-	none              = "<none>"
-	decimalPrefixBase = 1000
+	none                    = "<none>"
+	decimalPrefixBase       = 1000
+	minImageFieldLenWithTag = 2
 )
 
+type listOptions struct {
+	localStore *store.Store
+	logEntry   *logrus.Entry
+	imageName  string
+}
+
+func (b *Backend) getListOptions(req *pb.ListRequest) listOptions {
+	return listOptions{
+		localStore: b.daemon.localStore,
+		logEntry:   logrus.WithFields(logrus.Fields{"ImageName": req.GetImageName()}),
+		imageName:  req.GetImageName(),
+	}
+}
+
 // List lists all images
 func (b *Backend) List(ctx context.Context, req *pb.ListRequest) (*pb.ListResponse, error) {
-	logEntry := logrus.WithFields(logrus.Fields{"ImageName": req.GetImageName()})
-	logEntry.Info("ListRequest received")
-
-	var reqRepository, reqTag string
-	const minImageFieldLenWithTag = 2
-	if req.ImageName != "" {
-		imageName := req.ImageName
-		_, img, err := image.FindImage(b.daemon.localStore, imageName)
-		if err != nil {
-			logEntry.Error(err)
-			return nil, errors.Wrapf(err, "find local image %v error", imageName)
-		}
+	logrus.WithFields(logrus.Fields{
+		"ImageName": req.GetImageName(),
+	}).Info("ListRequest received")
 
-		parts := strings.Split(imageName, ":")
-		if len(parts) >= minImageFieldLenWithTag {
-			reqRepository, reqTag = strings.Join(parts[0:len(parts)-1], ":"), parts[len(parts)-1]
-		}
+	opts := b.getListOptions(req)
 
-		imageInfo := &pb.ListResponse_ImageInfo{
-			Repository: reqRepository,
-			Tag:        reqTag,
-			Id:         img.ID,
-			Created:    img.Created.Format(constant.LayoutTime),
-			Size_:      getImageSize(b.daemon.localStore, img.ID),
-		}
+	slashLastIndex := strings.LastIndex(opts.imageName, "/")
+	colonLastIndex := strings.LastIndex(opts.imageName, ":")
+	if opts.imageName != "" && strings.Contains(opts.imageName, ":") && colonLastIndex > slashLastIndex {
+		return listOneImage(opts)
+	}
+	return listImages(opts)
+}
 
-		return &pb.ListResponse{Images: []*pb.ListResponse_ImageInfo{imageInfo}}, nil
+func listOneImage(opts listOptions) (*pb.ListResponse, error) {
+	_, image, err := image.FindImage(opts.localStore, opts.imageName)
+	if err != nil {
+		opts.logEntry.Error(err)
+		return nil, errors.Wrapf(err, "find local image %v error", opts.imageName)
 	}
 
-	images, err := b.daemon.localStore.Images()
+	result := make([]*pb.ListResponse_ImageInfo, 0, len(image.Names))
+	appendImageToResult(&result, image, opts.localStore)
+
+	for _, info := range result {
+		if opts.imageName == fmt.Sprintf("%s:%s", info.Repository, info.Tag) {
+			result = []*pb.ListResponse_ImageInfo{info}
+		}
+	}
+
+	return &pb.ListResponse{Images: result}, nil
+}
+
+func listImages(opts listOptions) (*pb.ListResponse, error) {
+	images, err := opts.localStore.Images()
 	if err != nil {
-		logEntry.Error(err)
+		opts.logEntry.Error(err)
 		return &pb.ListResponse{}, errors.Wrap(err, "failed list images from local storage")
 	}
+
 	sort.Slice(images, func(i, j int) bool {
 		return images[i].Created.After(images[j].Created)
 	})
 	result := make([]*pb.ListResponse_ImageInfo, 0, len(images))
-	for _, image := range images {
-		names := image.Names
-		if len(names) == 0 {
-			names = []string{none}
+	for i := range images {
+		appendImageToResult(&result, &images[i], opts.localStore)
+	}
+
+	if opts.imageName == "" {
+		return &pb.ListResponse{Images: result}, nil
+	}
+
+	sameRepositoryResult := make([]*pb.ListResponse_ImageInfo, 0, len(images))
+	for _, info := range result {
+		if opts.imageName == info.Repository || strings.HasPrefix(info.Id, opts.imageName) {
+			sameRepositoryResult = append(sameRepositoryResult, info)
+		}
+	}
+
+	if len(sameRepositoryResult) == 0 {
+		return &pb.ListResponse{}, errors.Errorf("failed to list images with repository %q in local storage", opts.imageName)
+	}
+	return &pb.ListResponse{Images: sameRepositoryResult}, nil
+}
+
+func appendImageToResult(result *[]*pb.ListResponse_ImageInfo, image *storage.Image, store *store.Store) {
+	names := image.Names
+	if len(names) == 0 {
+		names = []string{none}
+	}
+
+	for _, name := range names {
+		repository, tag := name, none
+		parts := strings.Split(name, ":")
+		if len(parts) >= minImageFieldLenWithTag {
+			repository, tag = strings.Join(parts[0:len(parts)-1], ":"), parts[len(parts)-1]
 		}
-		for _, name := range names {
-			repository, tag := name, none
-			parts := strings.Split(name, ":")
-			if len(parts) >= minImageFieldLenWithTag {
-				repository, tag = strings.Join(parts[0:len(parts)-1], ":"), parts[len(parts)-1]
-			}
-			if reqRepository != "" && reqRepository != repository {
-				continue
-			}
-			if reqTag != "" && reqTag != tag {
-				continue
-			}
-
-			imageInfo := &pb.ListResponse_ImageInfo{
-				Repository: repository,
-				Tag:        tag,
-				Id:         image.ID,
-				Created:    image.Created.Format(constant.LayoutTime),
-				Size_:      getImageSize(b.daemon.localStore, image.ID),
-			}
-			result = append(result, imageInfo)
+
+		imageInfo := &pb.ListResponse_ImageInfo{
+			Repository: repository,
+			Tag:        tag,
+			Id:         image.ID,
+			Created:    image.Created.Format(constant.LayoutTime),
+			Size_:      getImageSize(store, image.ID),
 		}
+		*result = append(*result, imageInfo)
 	}
-	return &pb.ListResponse{Images: result}, nil
 }
 
 func getImageSize(store *store.Store, id string) string {
diff --git a/daemon/images_test.go b/daemon/images_test.go
new file mode 100644
index 00000000..a970ce0b
--- /dev/null
+++ b/daemon/images_test.go
@@ -0,0 +1,178 @@
+// Copyright (c) Huawei Technologies Co., Ltd. 2020. All rights reserved.
+// isula-build licensed under the Mulan PSL v2.
+// You can use this software according to the terms and conditions of the Mulan PSL v2.
+// You may obtain a copy of Mulan PSL v2 at:
+//     http://license.coscl.org.cn/MulanPSL2
+// THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND, EITHER EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR
+// PURPOSE.
+// See the Mulan PSL v2 for more details.
+// Author: Weizheng Xing
+// Create: 2021-02-03
+// Description: This file tests List interface
+
+package daemon
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/bndr/gotabulate"
+	"github.com/containers/storage"
+	"github.com/containers/storage/pkg/stringid"
+	"gotest.tools/v3/assert"
+
+	constant "isula.org/isula-build"
+	pb "isula.org/isula-build/api/services"
+)
+
+func TestList(t *testing.T) {
+	d := prepare(t)
+	defer tmpClean(d)
+
+	options := &storage.ImageOptions{}
+	img, err := d.Daemon.localStore.CreateImage(stringid.GenerateRandomID(), []string{"image:test1"}, "", "", options)
+	if err != nil {
+		t.Fatalf("create image with error: %v", err)
+	}
+	_, err = d.Daemon.localStore.CreateImage(stringid.GenerateRandomID(), []string{"image:test2"}, "", "", options)
+	if err != nil {
+		t.Fatalf("create image with error: %v", err)
+	}
+	_, err = d.Daemon.localStore.CreateImage(stringid.GenerateRandomID(), []string{"egami:test"}, "", "", options)
+	if err != nil {
+		t.Fatalf("create image with error: %v", err)
+	}
+	// image with no name and tag
+	_, err = d.Daemon.localStore.CreateImage(stringid.GenerateRandomID(), []string{}, "", "", options)
+	if err != nil {
+		t.Fatalf("create image with error: %v", err)
+	}
+	d.Daemon.localStore.SetNames(img.ID, append(img.Names, "image:test1-backup"))
+	// image who's repo contains port
+	_, err = d.Daemon.localStore.CreateImage(stringid.GenerateRandomID(), []string{"hub.example.com:8080/image:test"}, "", "", options)
+	if err != nil {
+		t.Fatalf("create image with error: %v", err)
+	}
+
+	testcases := []struct {
+		name      string
+		req       *pb.ListRequest
+		wantErr   bool
+		errString string
+	}{
+		{
+			name: "normal case list specific image with repository[:tag]",
+			req: &pb.ListRequest{
+				ImageName: "image:test1",
+			},
+			wantErr: false,
+		},
+		{
+			name: "normal case list specific image with image id",
+			req: &pb.ListRequest{
+				ImageName: img.ID,
+			},
+			wantErr: false,
+		},
+		{
+			name: "normal case list all images",
+			req: &pb.ListRequest{
+				ImageName: "",
+			},
+			wantErr: false,
+		},
+		{
+			name: "normal case list all images with repository",
+			req: &pb.ListRequest{
+				ImageName: "image",
+			},
+			wantErr: false,
+		},
+		{
+			name: "abnormal case no image found in local store",
+			req: &pb.ListRequest{
+				ImageName: "coffee:costa",
+			},
+			wantErr:   true,
+			errString: "failed to parse image",
+		},
+		{
+			name: "abnormal case no repository",
+			req: &pb.ListRequest{
+				ImageName: "coffee",
+			},
+			wantErr:   true,
+			errString: "failed to list images with repository",
+		},
+		{
+			name: "abnormal case ImageName only contains latest tag",
+			req: &pb.ListRequest{
+				ImageName: ":latest",
+			},
+			wantErr:   true,
+			errString: "invalid reference format",
+		},
+		{
+			name: "normal case ImageName contains port number and tag",
+			req: &pb.ListRequest{
+				ImageName: "hub.example.com:8080/image:test",
+			},
+			wantErr: false,
+		},
+		{
+			name: "normal case ImageName contains port number",
+			req: &pb.ListRequest{
+				ImageName: "hub.example.com:8080/image",
+			},
+			wantErr: false,
+		},
+		{
+			name: "abnormal case wrong ImageName",
+			req: &pb.ListRequest{
+				ImageName: "hub.example.com:8080/",
+			},
+			wantErr:   true,
+			errString: "failed to list images with repository",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.TODO()
+			resp, err := d.Daemon.backend.List(ctx, tc.req)
+
+			if tc.wantErr == true {
+				assert.ErrorContains(t, err, tc.errString)
+			}
+			if tc.wantErr == false {
+				assert.NilError(t, err)
+				formatAndPrint(resp.Images)
+			}
+		})
+	}
+}
+
+func formatAndPrint(images []*pb.ListResponse_ImageInfo) {
+	emptyStr := `-----------   ----   ---------   --------
+	REPOSITORY    TAG    IMAGE ID    CREATED
+	-----------   ----   ---------   --------`
+	lines := make([][]string, 0, len(images))
+	title := []string{"REPOSITORY", "TAG", "IMAGE ID", "CREATED", "SIZE"}
+	for _, image := range images {
+		if image == nil {
+			continue
+		}
+		line := []string{image.Repository, image.Tag, image.Id[:constant.DefaultIDLen], image.Created, image.Size_}
+		lines = append(lines, line)
+	}
+	if len(lines) == 0 {
+		fmt.Println(emptyStr)
+		return
+	}
+	tabulate := gotabulate.Create(lines)
+	tabulate.SetHeaders(title)
+	tabulate.SetAlign("left")
+	fmt.Print(tabulate.Render("simple"))
+}
diff --git a/image/image.go b/image/image.go
index bbbc7b94..36785bdf 100644
--- a/image/image.go
+++ b/image/image.go
@@ -590,12 +590,19 @@ func ResolveName(name string, sc *types.SystemContext, store *store.Store) ([]st
 }
 
 func tryResolveNameInStore(name string, store *store.Store) string {
+	defaultTag := "latest"
+
 	logrus.Infof("Try to find image: %s in local storage", name)
 	img, err := store.Image(name)
+	if err == nil {
+		return img.ID
+	}
+
+	logrus.Infof("Try to find image: %s:%s in local storage", name, defaultTag)
+	img, err = store.Image(fmt.Sprintf("%s:%s", name, defaultTag))
 	if err != nil {
 		return ""
 	}
-
 	return img.ID
 }
 
-- 
2.27.0