Merge pull request #375 from yanxuean/image-trunc

support get image by truncindex

pkg/server/container_status_test.go

@@ -30,6 +30,7 @@ import (
 
 func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Status,
 	*imagestore.Image, *runtime.ContainerStatus) {
+	imageID := "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
 	testID := "test-id"
 	config := &runtime.ContainerConfig{
 		Metadata: &runtime.ContainerMetadata{
@@ -53,7 +54,7 @@ func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Sta
 		Name:      "test-long-name",
 		SandboxID: "test-sandbox-id",
 		Config:    config,
-		ImageRef:  "test-image-id",
+		ImageRef:  imageID,
 		LogPath:   "test-log-path",
 	}
 	status := &containerstore.Status{
@@ -62,7 +63,7 @@ func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Sta
 		StartedAt: startedAt,
 	}
 	image := &imagestore.Image{
-		ID:          "test-image-id",
+		ID:          imageID,
 		RepoTags:    []string{"test-image-repo-tag"},
 		RepoDigests: []string{"test-image-repo-digest"},
 	}

pkg/server/helpers.go

@@ -28,7 +28,6 @@ import (
 	"github.com/containerd/cgroups"
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/content"
-	"github.com/containerd/containerd/errdefs"
 	"github.com/docker/distribution/reference"
 	imagedigest "github.com/opencontainers/go-digest"
 	"github.com/opencontainers/image-spec/identity"
@@ -205,34 +204,41 @@ func getRepoDigestAndTag(namedRef reference.Named, digest imagedigest.Digest, sc
 
 // localResolve resolves image reference locally and returns corresponding image metadata. It returns
 // nil without error if the reference doesn't exist.
-func (c *criContainerdService) localResolve(ctx context.Context, ref string) (*imagestore.Image, error) {
+func (c *criContainerdService) localResolve(ctx context.Context, refOrID string) (*imagestore.Image, error) {
-	_, err := imagedigest.Parse(ref)
+	getImageID := func(refOrId string) string {
-	if err != nil {
+		if _, err := imagedigest.Parse(refOrID); err == nil {
-		// ref is not image id, try to resolve it locally.
+			return refOrID
-		normalized, err := util.NormalizeImageRef(ref)
-		if err != nil {
-			return nil, fmt.Errorf("invalid image reference %q: %v", ref, err)
 		}
-		image, err := c.client.GetImage(ctx, normalized.String())
+
-		if err != nil {
+		return func(ref string) string {
-			if errdefs.IsNotFound(err) {
+			// ref is not image id, try to resolve it locally.
-				return nil, nil
+			normalized, err := util.NormalizeImageRef(ref)
+			if err != nil {
+				return ""
 			}
-			return nil, fmt.Errorf("failed to get image from containerd: %v", err)
+			image, err := c.client.GetImage(ctx, normalized.String())
-		}
+			if err != nil {
-		desc, err := image.Config(ctx)
+				return ""
-		if err != nil {
+			}
-			return nil, fmt.Errorf("failed to get image config descriptor: %v", err)
+			desc, err := image.Config(ctx)
-		}
+			if err != nil {
-		ref = desc.Digest.String()
+				return ""
+			}
+			return desc.Digest.String()
+		}(refOrID)
+	}
+
+	imageID := getImageID(refOrID)
+	if imageID == "" {
+		// Try to treat ref as imageID
+		imageID = refOrID
 	}
-	imageID := ref
 	image, err := c.imageStore.Get(imageID)
 	if err != nil {
 		if err == store.ErrNotExist {
 			return nil, nil
 		}
-		return nil, fmt.Errorf("failed to get image %q metadata: %v", imageID, err)
+		return nil, fmt.Errorf("failed to get image %q : %v", imageID, err)
 	}
 	return &image, nil
 }

pkg/server/image_list_test.go

@@ -32,7 +32,7 @@ func TestListImages(t *testing.T) {
 	c := newTestCRIContainerdService()
 	imagesInStore := []imagestore.Image{
 		{
-			ID:          "test-id-1",
+			ID:          "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			ChainID:     "test-chainid-1",
 			RepoTags:    []string{"tag-a-1", "tag-b-1"},
 			RepoDigests: []string{"digest-a-1", "digest-b-1"},
@@ -42,7 +42,7 @@ func TestListImages(t *testing.T) {
 			},
 		},
 		{
-			ID:          "test-id-2",
+			ID:          "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			ChainID:     "test-chainid-2",
 			RepoTags:    []string{"tag-a-2", "tag-b-2"},
 			RepoDigests: []string{"digest-a-2", "digest-b-2"},
@@ -52,7 +52,7 @@ func TestListImages(t *testing.T) {
 			},
 		},
 		{
-			ID:          "test-id-3",
+			ID:          "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			ChainID:     "test-chainid-3",
 			RepoTags:    []string{"tag-a-3", "tag-b-3"},
 			RepoDigests: []string{"digest-a-3", "digest-b-3"},
@@ -64,21 +64,21 @@ func TestListImages(t *testing.T) {
 	}
 	expect := []*runtime.Image{
 		{
-			Id:          "test-id-1",
+			Id:          "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			RepoTags:    []string{"tag-a-1", "tag-b-1"},
 			RepoDigests: []string{"digest-a-1", "digest-b-1"},
 			Size_:       uint64(1000),
 			Username:    "root",
 		},
 		{
-			Id:          "test-id-2",
+			Id:          "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			RepoTags:    []string{"tag-a-2", "tag-b-2"},
 			RepoDigests: []string{"digest-a-2", "digest-b-2"},
 			Size_:       uint64(2000),
 			Uid:         &runtime.Int64Value{Value: 1234},
 		},
 		{
-			Id:          "test-id-3",
+			Id:          "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			RepoTags:    []string{"tag-a-3", "tag-b-3"},
 			RepoDigests: []string{"digest-a-3", "digest-b-3"},
 			Size_:       uint64(3000),

pkg/store/image/image.go

@@ -20,7 +20,8 @@ import (
 	"sync"
 
 	"github.com/containerd/containerd"
-	"github.com/docker/docker/pkg/truncindex"
+	"github.com/docker/distribution/digestset"
+	godigest "github.com/opencontainers/go-digest"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 
 	"github.com/kubernetes-incubator/cri-containerd/pkg/store"
@@ -47,16 +48,16 @@ type Image struct {
 
 // Store stores all images.
 type Store struct {
-	lock    sync.RWMutex
+	lock      sync.RWMutex
-	images  map[string]Image
+	images    map[string]Image
-	idIndex *truncindex.TruncIndex
+	digestSet *digestset.Set
 }
 
 // NewStore creates an image store.
 func NewStore() *Store {
 	return &Store{
-		images:  make(map[string]Image),
+		images:    make(map[string]Image),
-		idIndex: truncindex.NewTruncIndex([]string{}),
+		digestSet: digestset.NewSet(),
 	}
 }
 
@@ -64,11 +65,11 @@ func NewStore() *Store {
 func (s *Store) Add(img Image) error {
 	s.lock.Lock()
 	defer s.lock.Unlock()
-	if _, err := s.idIndex.Get(img.ID); err != nil {
+	if _, err := s.digestSet.Lookup(img.ID); err != nil {
-		if err != truncindex.ErrNotExist {
+		if err != digestset.ErrDigestNotFound {
 			return err
 		}
-		if err := s.idIndex.Add(img.ID); err != nil {
+		if err := s.digestSet.Add(godigest.Digest(img.ID)); err != nil {
 			return err
 		}
 	}
@@ -91,14 +92,14 @@ func (s *Store) Add(img Image) error {
 func (s *Store) Get(id string) (Image, error) {
 	s.lock.RLock()
 	defer s.lock.RUnlock()
-	id, err := s.idIndex.Get(id)
+	digest, err := s.digestSet.Lookup(id)
 	if err != nil {
-		if err == truncindex.ErrNotExist {
+		if err == digestset.ErrDigestNotFound {
 			err = store.ErrNotExist
 		}
 		return Image{}, err
 	}
-	if i, ok := s.images[id]; ok {
+	if i, ok := s.images[digest.String()]; ok {
 		return i, nil
 	}
 	return Image{}, store.ErrNotExist
@@ -119,14 +120,14 @@ func (s *Store) List() []Image {
 func (s *Store) Delete(id string) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
-	id, err := s.idIndex.Get(id)
+	digest, err := s.digestSet.Lookup(id)
 	if err != nil {
 		// Note: The idIndex.Delete and delete doesn't handle truncated index.
 		// So we need to return if there are error.
 		return
 	}
-	s.idIndex.Delete(id) // nolint: errcheck
+	s.digestSet.Remove(digest) // nolint: errcheck
-	delete(s.images, id)
+	delete(s.images, digest.String())
 }
 
 // mergeStringSlices merges 2 string slices into one and remove duplicated elements.

pkg/store/image/image_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package image
 
 import (
+	"strings"
 	"testing"
 
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -26,33 +27,33 @@ import (
 )
 
 func TestImageStore(t *testing.T) {
-	images := map[string]Image{
+	images := []Image{
-		"1": {
+		{
-			ID:          "1",
+			ID:          "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			ChainID:     "test-chain-id-1",
 			RepoTags:    []string{"tag-1"},
 			RepoDigests: []string{"digest-1"},
 			Size:        10,
 			Config:      &imagespec.ImageConfig{},
 		},
-		"2abcd": {
+		{
-			ID:          "2abcd",
+			ID:          "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			ChainID:     "test-chain-id-2abcd",
 			RepoTags:    []string{"tag-2abcd"},
 			RepoDigests: []string{"digest-2abcd"},
 			Size:        20,
 			Config:      &imagespec.ImageConfig{},
 		},
-		"4a333": {
+		{
-			ID:          "4a333",
+			ID:          "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			RepoTags:    []string{"tag-4a333"},
 			RepoDigests: []string{"digest-4a333"},
 			ChainID:     "test-chain-id-4a333",
 			Size:        30,
 			Config:      &imagespec.ImageConfig{},
 		},
-		"4abcd": {
+		{
-			ID:          "4abcd",
+			ID:          "sha256:4123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			RepoTags:    []string{"tag-4abcd"},
 			RepoDigests: []string{"digest-4abcd"},
 			ChainID:     "test-chain-id-4abcd",
@@ -61,6 +62,7 @@ func TestImageStore(t *testing.T) {
 		},
 	}
 	assert := assertlib.New(t)
+	genTruncIndex := func(normalName string) string { return normalName[:(len(normalName)+1)/2] }
 
 	s := NewStore()
 
@@ -71,11 +73,26 @@ func TestImageStore(t *testing.T) {
 	}
 
 	t.Logf("should be able to get image")
-	genTruncIndex := func(normalName string) string { return normalName[:(len(normalName)+1)/2] }
+	for _, v := range images {
-	for id, img := range images {
+		truncID := genTruncIndex(v.ID)
-		got, err := s.Get(genTruncIndex(id))
+		got, err := s.Get(truncID)
-		assert.NoError(err)
+		assert.NoError(err, "truncID:%s, fullID:%s", truncID, v.ID)
-		assert.Equal(img, got)
+		assert.Equal(v, got)
+	}
+
+	t.Logf("should be able to get image by truncated imageId without algorithm")
+	for _, v := range images {
+		truncID := genTruncIndex(v.ID[strings.Index(v.ID, ":")+1:])
+		got, err := s.Get(truncID)
+		assert.NoError(err, "truncID:%s, fullID:%s", truncID, v.ID)
+		assert.Equal(v, got)
+	}
+
+	t.Logf("should not be able to get image by ambiguous prefix")
+	ambiguousPrefixs := []string{"sha256", "sha256:"}
+	for _, v := range ambiguousPrefixs {
+		_, err := s.Get(v)
+		assert.NotEqual(nil, err)
 	}
 
 	t.Logf("should be able to list images")
@@ -83,8 +100,8 @@ func TestImageStore(t *testing.T) {
 	assert.Len(imgs, len(images))
 
 	imageNum := len(images)
-	for testID, v := range images {
+	for _, v := range images {
-		truncID := genTruncIndex(testID)
+		truncID := genTruncIndex(v.ID)
 		oldRepoTag := v.RepoTags[0]
 		oldRepoDigest := v.RepoDigests[0]
 		newRepoTag := oldRepoTag + "new"