From 953d67d25025a3c6a01bcb9674f0d91b489be2be Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Thu, 26 Jul 2018 08:05:44 +0000 Subject: [PATCH] Create image reference cache. Signed-off-by: Lantao Liu --- pkg/server/container_create.go | 8 +- pkg/server/container_status.go | 9 +- pkg/server/container_status_test.go | 21 +-- pkg/server/events.go | 66 +++++--- pkg/server/events_test.go | 4 +- pkg/server/helpers.go | 104 ++++--------- pkg/server/helpers_test.go | 57 +++++++ pkg/server/image_list.go | 18 --- pkg/server/image_list_test.go | 54 ++++--- pkg/server/image_load.go | 31 +--- pkg/server/image_pull.go | 34 ++--- pkg/server/image_remove.go | 70 +++------ pkg/server/image_status.go | 24 +-- pkg/server/image_status_test.go | 19 ++- pkg/server/restart.go | 70 +-------- pkg/server/service.go | 4 +- pkg/server/service_test.go | 2 +- pkg/store/image/fake_image.go | 34 +++++ pkg/store/image/image.go | 214 +++++++++++++++++++------- pkg/store/image/image_test.go | 226 ++++++++++++++++++++-------- pkg/util/strings.go | 16 ++ pkg/util/strings_test.go | 11 ++ 22 files changed, 640 insertions(+), 456 deletions(-) create mode 100644 pkg/store/image/fake_image.go diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index e76b1c460..ef9832b0f 100644 --- a/pkg/server/container_create.go +++ b/pkg/server/container_create.go @@ -115,13 +115,9 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta // Prepare container image snapshot. For container, the image should have // been pulled before creating the container, so do not ensure the image. - imageRef := config.GetImage().GetImage() - image, err := c.localResolve(ctx, imageRef) + image, err := c.localResolve(config.GetImage().GetImage()) if err != nil { - return nil, errors.Wrapf(err, "failed to resolve image %q", imageRef) - } - if image == nil { - return nil, errors.Errorf("image %q not found", imageRef) + return nil, errors.Wrapf(err, "failed to resolve image %q", config.GetImage().GetImage()) } // Run container using the same runtime with sandbox. diff --git a/pkg/server/container_status.go b/pkg/server/container_status.go index 51d356d03..ffbe2381a 100644 --- a/pkg/server/container_status.go +++ b/pkg/server/container_status.go @@ -46,14 +46,15 @@ func (c *criService) ContainerStatus(ctx context.Context, r *runtime.ContainerSt if err != nil { return nil, errors.Wrapf(err, "failed to get image %q", imageRef) } - if len(image.RepoTags) > 0 { + repoTags, repoDigests := parseImageReferences(image.References) + if len(repoTags) > 0 { // Based on current behavior of dockershim, this field should be // image tag. - spec = &runtime.ImageSpec{Image: image.RepoTags[0]} + spec = &runtime.ImageSpec{Image: repoTags[0]} } - if len(image.RepoDigests) > 0 { + if len(repoDigests) > 0 { // Based on the CRI definition, this field will be consumed by user. - imageRef = image.RepoDigests[0] + imageRef = repoDigests[0] } status := toCRIContainerStatus(container, spec, imageRef) info, err := toCRIContainerInfo(ctx, container, r.GetVerbose()) diff --git a/pkg/server/container_status_test.go b/pkg/server/container_status_test.go index 78f8d2d54..5a9ba6c24 100644 --- a/pkg/server/container_status_test.go +++ b/pkg/server/container_status_test.go @@ -63,9 +63,11 @@ func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Sta StartedAt: startedAt, } image := &imagestore.Image{ - ID: imageID, - RepoTags: []string{"test-image-repo-tag"}, - RepoDigests: []string{"test-image-repo-digest"}, + ID: imageID, + References: []string{ + "gcr.io/library/busybox:latest", + "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + }, } expected := &runtime.ContainerStatus{ Id: testID, @@ -73,8 +75,8 @@ func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Sta State: runtime.ContainerState_CONTAINER_RUNNING, CreatedAt: createdAt, StartedAt: startedAt, - Image: &runtime.ImageSpec{Image: "test-image-repo-tag"}, - ImageRef: "test-image-repo-digest", + Image: &runtime.ImageSpec{Image: "gcr.io/library/busybox:latest"}, + ImageRef: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", Reason: completeExitReason, Labels: config.GetLabels(), Annotations: config.GetAnnotations(), @@ -120,7 +122,7 @@ func TestToCRIContainerStatus(t *testing.T) { expectedReason: errorExitReason, }, } { - metadata, status, image, expected := getContainerStatusTestData() + metadata, status, _, expected := getContainerStatusTestData() // Update status with test case. status.FinishedAt = test.finishedAt status.ExitCode = test.exitCode @@ -138,8 +140,8 @@ func TestToCRIContainerStatus(t *testing.T) { expected.ExitCode = test.exitCode expected.Message = test.message containerStatus := toCRIContainerStatus(container, - &runtime.ImageSpec{Image: image.RepoTags[0]}, - image.RepoDigests[0]) + expected.Image, + expected.ImageRef) assert.Equal(t, expected, containerStatus, desc) } } @@ -207,7 +209,8 @@ func TestContainerStatus(t *testing.T) { assert.NoError(t, c.containerStore.Add(container)) } if test.imageExist { - c.imageStore.Add(*image) + c.imageStore, err = imagestore.NewFakeStore([]imagestore.Image{*image}) + assert.NoError(t, err) } resp, err := c.ContainerStatus(context.Background(), &runtime.ContainerStatusRequest{ContainerId: container.ID}) if test.expectErr { diff --git a/pkg/server/events.go b/pkg/server/events.go index bfb95e6d6..c3ab1dfd5 100644 --- a/pkg/server/events.go +++ b/pkg/server/events.go @@ -34,6 +34,7 @@ import ( ctrdutil "github.com/containerd/cri/pkg/containerd/util" "github.com/containerd/cri/pkg/store" containerstore "github.com/containerd/cri/pkg/store/container" + imagestore "github.com/containerd/cri/pkg/store/image" sandboxstore "github.com/containerd/cri/pkg/store/sandbox" ) @@ -49,6 +50,7 @@ const ( type eventMonitor struct { containerStore *containerstore.Store sandboxStore *sandboxstore.Store + imageStore *imagestore.Store ch <-chan *events.Envelope errCh <-chan error ctx context.Context @@ -76,12 +78,13 @@ type backOffQueue struct { // Create new event monitor. New event monitor will start subscribing containerd event. All events // happen after it should be monitored. -func newEventMonitor(c *containerstore.Store, s *sandboxstore.Store) *eventMonitor { +func newEventMonitor(c *containerstore.Store, s *sandboxstore.Store, i *imagestore.Store) *eventMonitor { // event subscribe doesn't need namespace. ctx, cancel := context.WithCancel(context.Background()) return &eventMonitor{ containerStore: c, sandboxStore: s, + imageStore: i, ctx: ctx, cancel: cancel, backOff: newBackOff(), @@ -93,12 +96,13 @@ func (em *eventMonitor) subscribe(subscriber events.Subscriber) { filters := []string{ `topic=="/tasks/exit"`, `topic=="/tasks/oom"`, + `topic~="/images/"`, } em.ch, em.errCh = subscriber.Subscribe(em.ctx, filters...) } func convertEvent(e *gogotypes.Any) (string, interface{}, error) { - containerID := "" + id := "" evt, err := typeurl.UnmarshalAny(e) if err != nil { return "", nil, errors.Wrap(err, "failed to unmarshalany") @@ -106,16 +110,22 @@ func convertEvent(e *gogotypes.Any) (string, interface{}, error) { switch evt.(type) { case *eventtypes.TaskExit: - containerID = evt.(*eventtypes.TaskExit).ContainerID + id = evt.(*eventtypes.TaskExit).ContainerID case *eventtypes.TaskOOM: - containerID = evt.(*eventtypes.TaskOOM).ContainerID + id = evt.(*eventtypes.TaskOOM).ContainerID + case *eventtypes.ImageCreate: + id = evt.(*eventtypes.ImageCreate).Name + case *eventtypes.ImageUpdate: + id = evt.(*eventtypes.ImageUpdate).Name + case *eventtypes.ImageDelete: + id = evt.(*eventtypes.ImageDelete).Name default: return "", nil, errors.New("unsupported event") } - return containerID, evt, nil + return id, evt, nil } -// start starts the event monitor which monitors and handles all container events. It returns +// start starts the event monitor which monitors and handles all subscribed events. It returns // an error channel for the caller to wait for stop errors from the event monitor. // start must be called after subscribe. func (em *eventMonitor) start() <-chan error { @@ -130,19 +140,19 @@ func (em *eventMonitor) start() <-chan error { select { case e := <-em.ch: logrus.Debugf("Received containerd event timestamp - %v, namespace - %q, topic - %q", e.Timestamp, e.Namespace, e.Topic) - cID, evt, err := convertEvent(e.Event) + id, evt, err := convertEvent(e.Event) if err != nil { logrus.WithError(err).Errorf("Failed to convert event %+v", e) break } - if em.backOff.isInBackOff(cID) { - logrus.Infof("Events for container %q is in backoff, enqueue event %+v", cID, evt) - em.backOff.enBackOff(cID, evt) + if em.backOff.isInBackOff(id) { + logrus.Infof("Events for %q is in backoff, enqueue event %+v", id, evt) + em.backOff.enBackOff(id, evt) break } if err := em.handleEvent(evt); err != nil { - logrus.WithError(err).Errorf("Failed to handle event %+v for container %s", evt, cID) - em.backOff.enBackOff(cID, evt) + logrus.WithError(err).Errorf("Failed to handle event %+v for %s", evt, id) + em.backOff.enBackOff(id, evt) } case err := <-em.errCh: // Close errCh in defer directly if there is no error. @@ -152,13 +162,13 @@ func (em *eventMonitor) start() <-chan error { } return case <-backOffCheckCh: - cIDs := em.backOff.getExpiredContainers() - for _, cID := range cIDs { - queue := em.backOff.deBackOff(cID) + ids := em.backOff.getExpiredIDs() + for _, id := range ids { + queue := em.backOff.deBackOff(id) for i, any := range queue.events { if err := em.handleEvent(any); err != nil { - logrus.WithError(err).Errorf("Failed to handle backOff event %+v for container %s", any, cID) - em.backOff.reBackOff(cID, queue.events[i:], queue.duration) + logrus.WithError(err).Errorf("Failed to handle backOff event %+v for %s", any, id) + em.backOff.reBackOff(id, queue.events[i:], queue.duration) break } } @@ -230,6 +240,18 @@ func (em *eventMonitor) handleEvent(any interface{}) error { if err != nil { return errors.Wrap(err, "failed to update container status for TaskOOM event") } + case *eventtypes.ImageCreate: + e := any.(*eventtypes.ImageCreate) + logrus.Infof("ImageCreate event %+v", e) + return em.imageStore.Update(ctx, e.Name) + case *eventtypes.ImageUpdate: + e := any.(*eventtypes.ImageUpdate) + logrus.Infof("ImageUpdate event %+v", e) + return em.imageStore.Update(ctx, e.Name) + case *eventtypes.ImageDelete: + e := any.(*eventtypes.ImageDelete) + logrus.Infof("ImageDelete event %+v", e) + return em.imageStore.Update(ctx, e.Name) } return nil @@ -331,14 +353,14 @@ func newBackOff() *backOff { } } -func (b *backOff) getExpiredContainers() []string { - var containers []string - for c, q := range b.queuePool { +func (b *backOff) getExpiredIDs() []string { + var ids []string + for id, q := range b.queuePool { if q.isExpire() { - containers = append(containers, c) + ids = append(ids, id) } } - return containers + return ids } func (b *backOff) isInBackOff(key string) bool { diff --git a/pkg/server/events_test.go b/pkg/server/events_test.go index baef19c6e..836dd2771 100644 --- a/pkg/server/events_test.go +++ b/pkg/server/events_test.go @@ -94,11 +94,11 @@ func TestBackOff(t *testing.T) { assert.Equal(t, actual.isInBackOff(notExistKey), false) t.Logf("No containers should be expired") - assert.Empty(t, actual.getExpiredContainers()) + assert.Empty(t, actual.getExpiredIDs()) t.Logf("Should be able to get all keys which are expired for backOff") testClock.Sleep(backOffInitDuration) - actKeyList := actual.getExpiredContainers() + actKeyList := actual.getExpiredIDs() assert.Equal(t, len(inputQueues), len(actKeyList)) for k := range inputQueues { assert.Contains(t, actKeyList, k) diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index f2f171a26..065b585dd 100644 --- a/pkg/server/helpers.go +++ b/pkg/server/helpers.go @@ -17,7 +17,6 @@ limitations under the License. package server import ( - "encoding/json" "fmt" "os" "path" @@ -26,15 +25,11 @@ import ( "strconv" "strings" - "github.com/containerd/containerd" "github.com/containerd/containerd/containers" - "github.com/containerd/containerd/content" "github.com/containerd/containerd/runtime/linux/runctypes" "github.com/containerd/typeurl" "github.com/docker/distribution/reference" imagedigest "github.com/opencontainers/go-digest" - "github.com/opencontainers/image-spec/identity" - imagespec "github.com/opencontainers/image-spec/specs-go/v1" runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/selinux/go-selinux" @@ -236,28 +231,25 @@ func getRepoDigestAndTag(namedRef reference.Named, digest imagedigest.Digest, sc return repoDigest, repoTag } -// localResolve resolves image reference locally and returns corresponding image metadata. It returns -// nil without error if the reference doesn't exist. -func (c *criService) localResolve(ctx context.Context, refOrID string) (*imagestore.Image, error) { +// localResolve resolves image reference locally and returns corresponding image metadata. It +// returns store.ErrNotExist if the reference doesn't exist. +func (c *criService) localResolve(refOrID string) (imagestore.Image, error) { getImageID := func(refOrId string) string { if _, err := imagedigest.Parse(refOrID); err == nil { return refOrID } return func(ref string) string { // ref is not image id, try to resolve it locally. + // TODO(random-liu): Handle this error better for debugging. normalized, err := util.NormalizeImageRef(ref) if err != nil { return "" } - image, err := c.client.GetImage(ctx, normalized.String()) + id, err := c.imageStore.Resolve(normalized.String()) if err != nil { return "" } - desc, err := image.Config(ctx) - if err != nil { - return "" - } - return desc.Digest.String() + return id }(refOrID) } @@ -266,14 +258,7 @@ func (c *criService) localResolve(ctx context.Context, refOrID string) (*imagest // Try to treat ref as imageID imageID = refOrID } - image, err := c.imageStore.Get(imageID) - if err != nil { - if err == store.ErrNotExist { - return nil, nil - } - return nil, errors.Wrapf(err, "failed to get image %q", imageID) - } - return &image, nil + return c.imageStore.Get(imageID) } // getUserFromImage gets uid or user name of the image user. @@ -298,12 +283,12 @@ func getUserFromImage(user string) (*int64, string) { // ensureImageExists returns corresponding metadata of the image reference, if image is not // pulled yet, the function will pull the image. func (c *criService) ensureImageExists(ctx context.Context, ref string) (*imagestore.Image, error) { - image, err := c.localResolve(ctx, ref) - if err != nil { - return nil, errors.Wrapf(err, "failed to resolve image %q", ref) + image, err := c.localResolve(ref) + if err != nil && err != store.ErrNotExist { + return nil, errors.Wrapf(err, "failed to get image %q", ref) } - if image != nil { - return image, nil + if err == nil { + return &image, nil } // Pull image to ensure the image exists resp, err := c.PullImage(ctx, &runtime.PullImageRequest{Image: &runtime.ImageSpec{Image: ref}}) @@ -314,56 +299,11 @@ func (c *criService) ensureImageExists(ctx context.Context, ref string) (*images newImage, err := c.imageStore.Get(imageID) if err != nil { // It's still possible that someone removed the image right after it is pulled. - return nil, errors.Wrapf(err, "failed to get image %q metadata after pulling", imageID) + return nil, errors.Wrapf(err, "failed to get image %q after pulling", imageID) } return &newImage, nil } -// imageInfo is the information about the image got from containerd. -type imageInfo struct { - id string - chainID imagedigest.Digest - size int64 - imagespec imagespec.Image -} - -// getImageInfo gets image info from containerd. -func getImageInfo(ctx context.Context, image containerd.Image) (*imageInfo, error) { - // Get image information. - diffIDs, err := image.RootFS(ctx) - if err != nil { - return nil, errors.Wrap(err, "failed to get image diffIDs") - } - chainID := identity.ChainID(diffIDs) - - size, err := image.Size(ctx) - if err != nil { - return nil, errors.Wrap(err, "failed to get image compressed resource size") - } - - desc, err := image.Config(ctx) - if err != nil { - return nil, errors.Wrap(err, "failed to get image config descriptor") - } - id := desc.Digest.String() - - rb, err := content.ReadBlob(ctx, image.ContentStore(), desc) - if err != nil { - return nil, errors.Wrap(err, "failed to read image config from content store") - } - var ociimage imagespec.Image - if err := json.Unmarshal(rb, &ociimage); err != nil { - return nil, errors.Wrapf(err, "failed to unmarshal image config %s", rb) - } - - return &imageInfo{ - id: id, - chainID: chainID, - size: size, - imagespec: ociimage, - }, nil -} - func initSelinuxOpts(selinuxOpt *runtime.SELinuxOption) (string, string, error) { if selinuxOpt == nil { return "", "", nil @@ -500,3 +440,21 @@ func (m orderedMounts) Swap(i, j int) { func (m orderedMounts) parts(i int) int { return strings.Count(filepath.Clean(m[i].ContainerPath), string(os.PathSeparator)) } + +// parseImageReferences parses a list of arbitrary image references and returns +// the repotags and repodigests +func parseImageReferences(refs []string) ([]string, []string) { + var tags, digests []string + for _, ref := range refs { + parsed, err := reference.ParseAnyReference(ref) + if err != nil { + continue + } + if _, ok := parsed.(reference.Canonical); ok { + digests = append(digests, parsed.String()) + } else if _, ok := parsed.(reference.Tagged); ok { + tags = append(tags, parsed.String()) + } + } + return tags, digests +} diff --git a/pkg/server/helpers_test.go b/pkg/server/helpers_test.go index b84efed44..712ca18dd 100644 --- a/pkg/server/helpers_test.go +++ b/pkg/server/helpers_test.go @@ -29,6 +29,8 @@ import ( runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" criconfig "github.com/containerd/cri/pkg/config" + "github.com/containerd/cri/pkg/store" + imagestore "github.com/containerd/cri/pkg/store/image" "github.com/containerd/cri/pkg/util" ) @@ -213,3 +215,58 @@ func TestOrderedMounts(t *testing.T) { sort.Stable(orderedMounts(mounts)) assert.Equal(t, expected, mounts) } + +func TestParseImageReferences(t *testing.T) { + refs := []string{ + "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + "gcr.io/library/busybox:1.2", + "sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + "arbitrary-ref", + } + expectedTags := []string{ + "gcr.io/library/busybox:1.2", + } + expectedDigests := []string{"gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582"} + tags, digests := parseImageReferences(refs) + assert.Equal(t, expectedTags, tags) + assert.Equal(t, expectedDigests, digests) +} + +func TestLocalResolve(t *testing.T) { + image := imagestore.Image{ + ID: "sha256:c75bebcdd211f41b3a460c7bf82970ed6c75acaab9cd4c9a4e125b03ca113799", + ChainID: "test-chain-id-1", + References: []string{ + "docker.io/library/busybox:latest", + "docker.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + }, + Size: 10, + } + c := newTestCRIService() + var err error + c.imageStore, err = imagestore.NewFakeStore([]imagestore.Image{image}) + assert.NoError(t, err) + + for _, ref := range []string{ + "sha256:c75bebcdd211f41b3a460c7bf82970ed6c75acaab9cd4c9a4e125b03ca113799", + "busybox", + "busybox:latest", + "busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + "library/busybox", + "library/busybox:latest", + "library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + "docker.io/busybox", + "docker.io/busybox:latest", + "docker.io/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + "docker.io/library/busybox", + "docker.io/library/busybox:latest", + "docker.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + } { + img, err := c.localResolve(ref) + assert.NoError(t, err) + assert.Equal(t, image, img) + } + img, err := c.localResolve("randomid") + assert.Equal(t, store.ErrNotExist, err) + assert.Equal(t, imagestore.Image{}, img) +} diff --git a/pkg/server/image_list.go b/pkg/server/image_list.go index 28abbecaa..cb1253cc4 100644 --- a/pkg/server/image_list.go +++ b/pkg/server/image_list.go @@ -19,8 +19,6 @@ package server import ( "golang.org/x/net/context" runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" - - imagestore "github.com/containerd/cri/pkg/store/image" ) // ListImages lists existing images. @@ -38,19 +36,3 @@ func (c *criService) ListImages(ctx context.Context, r *runtime.ListImagesReques return &runtime.ListImagesResponse{Images: images}, nil } - -// toCRIImage converts image to CRI image type. -func toCRIImage(image imagestore.Image) *runtime.Image { - runtimeImage := &runtime.Image{ - Id: image.ID, - RepoTags: image.RepoTags, - RepoDigests: image.RepoDigests, - Size_: uint64(image.Size), - } - uid, username := getUserFromImage(image.ImageSpec.Config.User) - if uid != nil { - runtimeImage.Uid = &runtime.Int64Value{Value: *uid} - } - runtimeImage.Username = username - return runtimeImage -} diff --git a/pkg/server/image_list_test.go b/pkg/server/image_list_test.go index f690eb286..7bd84cdd6 100644 --- a/pkg/server/image_list_test.go +++ b/pkg/server/image_list_test.go @@ -32,11 +32,13 @@ func TestListImages(t *testing.T) { c := newTestCRIService() imagesInStore := []imagestore.Image{ { - ID: "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - ChainID: "test-chainid-1", - RepoTags: []string{"tag-a-1", "tag-b-1"}, - RepoDigests: []string{"digest-a-1", "digest-b-1"}, - Size: 1000, + ID: "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + ChainID: "test-chainid-1", + References: []string{ + "gcr.io/library/busybox:latest", + "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + }, + Size: 1000, ImageSpec: imagespec.Image{ Config: imagespec.ImageConfig{ User: "root", @@ -44,11 +46,13 @@ func TestListImages(t *testing.T) { }, }, { - ID: "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - ChainID: "test-chainid-2", - RepoTags: []string{"tag-a-2", "tag-b-2"}, - RepoDigests: []string{"digest-a-2", "digest-b-2"}, - Size: 2000, + ID: "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + ChainID: "test-chainid-2", + References: []string{ + "gcr.io/library/alpine:latest", + "gcr.io/library/alpine@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + }, + Size: 2000, ImageSpec: imagespec.Image{ Config: imagespec.ImageConfig{ User: "1234:1234", @@ -56,11 +60,13 @@ func TestListImages(t *testing.T) { }, }, { - ID: "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - ChainID: "test-chainid-3", - RepoTags: []string{"tag-a-3", "tag-b-3"}, - RepoDigests: []string{"digest-a-3", "digest-b-3"}, - Size: 3000, + ID: "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + ChainID: "test-chainid-3", + References: []string{ + "gcr.io/library/ubuntu:latest", + "gcr.io/library/ubuntu@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + }, + Size: 3000, ImageSpec: imagespec.Image{ Config: imagespec.ImageConfig{ User: "nobody", @@ -71,30 +77,30 @@ func TestListImages(t *testing.T) { expect := []*runtime.Image{ { Id: "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - RepoTags: []string{"tag-a-1", "tag-b-1"}, - RepoDigests: []string{"digest-a-1", "digest-b-1"}, + RepoTags: []string{"gcr.io/library/busybox:latest"}, + RepoDigests: []string{"gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582"}, Size_: uint64(1000), Username: "root", }, { Id: "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - RepoTags: []string{"tag-a-2", "tag-b-2"}, - RepoDigests: []string{"digest-a-2", "digest-b-2"}, + RepoTags: []string{"gcr.io/library/alpine:latest"}, + RepoDigests: []string{"gcr.io/library/alpine@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582"}, Size_: uint64(2000), Uid: &runtime.Int64Value{Value: 1234}, }, { Id: "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - RepoTags: []string{"tag-a-3", "tag-b-3"}, - RepoDigests: []string{"digest-a-3", "digest-b-3"}, + RepoTags: []string{"gcr.io/library/ubuntu:latest"}, + RepoDigests: []string{"gcr.io/library/ubuntu@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582"}, Size_: uint64(3000), Username: "nobody", }, } - for _, i := range imagesInStore { - c.imageStore.Add(i) - } + var err error + c.imageStore, err = imagestore.NewFakeStore(imagesInStore) + assert.NoError(t, err) resp, err := c.ListImages(context.Background(), &runtime.ListImagesRequest{}) assert.NoError(t, err) diff --git a/pkg/server/image_load.go b/pkg/server/image_load.go index d7f5ff842..d3741a021 100644 --- a/pkg/server/image_load.go +++ b/pkg/server/image_load.go @@ -26,7 +26,6 @@ import ( api "github.com/containerd/cri/pkg/api/v1" "github.com/containerd/cri/pkg/containerd/importer" - imagestore "github.com/containerd/cri/pkg/store/image" ) // LoadImage loads a image into containerd. @@ -44,33 +43,11 @@ func (c *criService) LoadImage(ctx context.Context, r *api.LoadImageRequest) (*a return nil, errors.Wrap(err, "failed to import image") } for _, repoTag := range repoTags { - image, err := c.client.GetImage(ctx, repoTag) - if err != nil { - return nil, errors.Wrapf(err, "failed to get image %q", repoTag) + // Update image store to reflect the newest state in containerd. + if err := c.imageStore.Update(ctx, repoTag); err != nil { + return nil, errors.Wrapf(err, "failed to update image store %q", repoTag) } - info, err := getImageInfo(ctx, image) - if err != nil { - return nil, errors.Wrapf(err, "failed to get image %q info", repoTag) - } - id := info.id - - if err := c.createImageReference(ctx, id, image.Target()); err != nil { - return nil, errors.Wrapf(err, "failed to create image reference %q", id) - } - - img := imagestore.Image{ - ID: id, - RepoTags: []string{repoTag}, - ChainID: info.chainID.String(), - Size: info.size, - ImageSpec: info.imagespec, - Image: image, - } - - if err := c.imageStore.Add(img); err != nil { - return nil, errors.Wrapf(err, "failed to add image %q into store", id) - } - logrus.Debugf("Imported image with id %q, repo tag %q", id, repoTag) + logrus.Debugf("Imported image %q", repoTag) } return &api.LoadImageResponse{Images: repoTags}, nil } diff --git a/pkg/server/image_pull.go b/pkg/server/image_pull.go index a25d8285e..48bc4c562 100644 --- a/pkg/server/image_pull.go +++ b/pkg/server/image_pull.go @@ -34,7 +34,6 @@ import ( "golang.org/x/net/context" runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" - imagestore "github.com/containerd/cri/pkg/store/image" "github.com/containerd/cri/pkg/util" ) @@ -108,49 +107,34 @@ func (c *criService) PullImage(ctx context.Context, r *runtime.PullImageRequest) return nil, errors.Wrapf(err, "failed to pull and unpack image %q", ref) } - // Get image information. - info, err := getImageInfo(ctx, image) + configDesc, err := image.Config(ctx) if err != nil { - return nil, errors.Wrap(err, "failed to get image information") + return nil, errors.Wrap(err, "get image config descriptor") } - imageID := info.id + imageID := configDesc.Digest.String() repoDigest, repoTag := getRepoDigestAndTag(namedRef, image.Target().Digest, isSchema1) - for _, r := range []string{repoTag, repoDigest, imageID} { + for _, r := range []string{repoTag, repoDigest} { if r == "" { continue } if err := c.createImageReference(ctx, r, image.Target()); err != nil { return nil, errors.Wrapf(err, "failed to update image reference %q", r) } + // Update image store to reflect the newest state in containerd. + if err := c.imageStore.Update(ctx, r); err != nil { + return nil, errors.Wrapf(err, "failed to update image store %q", r) + } } logrus.Debugf("Pulled image %q with image id %q, repo tag %q, repo digest %q", imageRef, imageID, repoTag, repoDigest) - img := imagestore.Image{ - ID: imageID, - ChainID: info.chainID.String(), - Size: info.size, - ImageSpec: info.imagespec, - Image: image, - } - if repoDigest != "" { - img.RepoDigests = []string{repoDigest} - } - if repoTag != "" { - img.RepoTags = []string{repoTag} - } - - if err := c.imageStore.Add(img); err != nil { - return nil, errors.Wrapf(err, "failed to add image %q into store", img.ID) - } - // NOTE(random-liu): the actual state in containerd is the source of truth, even we maintain // in-memory image store, it's only for in-memory indexing. The image could be removed // by someone else anytime, before/during/after we create the metadata. We should always // check the actual state in containerd before using the image or returning status of the // image. - return &runtime.PullImageResponse{ImageRef: img.ID}, nil + return &runtime.PullImageResponse{ImageRef: imageID}, nil } // ParseAuth parses AuthConfig and returns username and password/secret required by containerd. diff --git a/pkg/server/image_remove.go b/pkg/server/image_remove.go index c27695bc4..f0ce7114f 100644 --- a/pkg/server/image_remove.go +++ b/pkg/server/image_remove.go @@ -20,9 +20,10 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "golang.org/x/net/context" runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" + + "github.com/containerd/cri/pkg/store" ) // RemoveImage removes the image. @@ -32,62 +33,33 @@ import ( // Remove the whole image no matter the it's image id or reference. This is the // semantic defined in CRI now. func (c *criService) RemoveImage(ctx context.Context, r *runtime.RemoveImageRequest) (*runtime.RemoveImageResponse, error) { - image, err := c.localResolve(ctx, r.GetImage().GetImage()) + image, err := c.localResolve(r.GetImage().GetImage()) if err != nil { + if err == store.ErrNotExist { + // return empty without error when image not found. + return &runtime.RemoveImageResponse{}, nil + } return nil, errors.Wrapf(err, "can not resolve %q locally", r.GetImage().GetImage()) } - if image == nil { - // return empty without error when image not found. - return &runtime.RemoveImageResponse{}, nil - } - // Exclude outdated image tag. - for i, tag := range image.RepoTags { - cImage, err := c.client.GetImage(ctx, tag) - if err != nil { - if errdefs.IsNotFound(err) { - continue - } - return nil, errors.Wrapf(err, "failed to get image %q", tag) + // Remove all image references. + for i, ref := range image.References { + var opts []images.DeleteOpt + if i == len(image.References)-1 { + // Delete the last image reference synchronously to trigger garbage collection. + // This is best effort. It is possible that the image reference is deleted by + // someone else before this point. + opts = []images.DeleteOpt{images.SynchronousDelete()} } - desc, err := cImage.Config(ctx) - if err != nil { - // We can only get image id by reading Config from content. - // If the config is missing, we will fail to get image id, - // So we won't be able to remove the image forever, - // and the cri plugin always reports the image is ok. - // But we also don't check it by manifest, - // It's possible that two manifest digests have the same image ID in theory. - // In theory it's possible that an image is compressed with different algorithms, - // then they'll have the same uncompressed id - image id, - // but different ids generated from compressed contents - manifest digest. - // So we decide to leave it. - // After all, the user can override the repoTag by pulling image again. - logrus.WithError(err).Errorf("Can't remove image,failed to get config for Image tag %q,id %q", tag, image.ID) - image.RepoTags = append(image.RepoTags[:i], image.RepoTags[i+1:]...) - continue - } - cID := desc.Digest.String() - if cID != image.ID { - logrus.Debugf("Image tag %q for %q is outdated, it's currently used by %q", tag, image.ID, cID) - image.RepoTags = append(image.RepoTags[:i], image.RepoTags[i+1:]...) - continue - } - } - - // Include all image references, including RepoTag, RepoDigest and id. - for _, ref := range append(image.RepoTags, image.RepoDigests...) { - err = c.client.ImageService().Delete(ctx, ref) + err = c.client.ImageService().Delete(ctx, ref, opts...) if err == nil || errdefs.IsNotFound(err) { + // Update image store to reflect the newest state in containerd. + if err := c.imageStore.Update(ctx, ref); err != nil { + return nil, errors.Wrapf(err, "failed to update image reference %q for %q", ref, image.ID) + } continue } - return nil, errors.Wrapf(err, "failed to delete image reference %q for image %q", ref, image.ID) + return nil, errors.Wrapf(err, "failed to delete image reference %q for %q", ref, image.ID) } - // Delete image id synchronously to trigger garbage collection. - err = c.client.ImageService().Delete(ctx, image.ID, images.SynchronousDelete()) - if err != nil && !errdefs.IsNotFound(err) { - return nil, errors.Wrapf(err, "failed to delete image id %q", image.ID) - } - c.imageStore.Delete(image.ID) return &runtime.RemoveImageResponse{}, nil } diff --git a/pkg/server/image_status.go b/pkg/server/image_status.go index b3b39ed90..8337bdb10 100644 --- a/pkg/server/image_status.go +++ b/pkg/server/image_status.go @@ -24,6 +24,7 @@ import ( "golang.org/x/net/context" runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" + "github.com/containerd/cri/pkg/store" imagestore "github.com/containerd/cri/pkg/store/image" imagespec "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -32,19 +33,19 @@ import ( // TODO(random-liu): We should change CRI to distinguish image id and image spec. (See // kubernetes/kubernetes#46255) func (c *criService) ImageStatus(ctx context.Context, r *runtime.ImageStatusRequest) (*runtime.ImageStatusResponse, error) { - image, err := c.localResolve(ctx, r.GetImage().GetImage()) + image, err := c.localResolve(r.GetImage().GetImage()) if err != nil { + if err == store.ErrNotExist { + // return empty without error when image not found. + return &runtime.ImageStatusResponse{}, nil + } return nil, errors.Wrapf(err, "can not resolve %q locally", r.GetImage().GetImage()) } - if image == nil { - // return empty without error when image not found. - return &runtime.ImageStatusResponse{}, nil - } // TODO(random-liu): [P0] Make sure corresponding snapshot exists. What if snapshot // doesn't exist? - runtimeImage := toCRIRuntimeImage(image) - info, err := c.toCRIImageInfo(ctx, image, r.GetVerbose()) + runtimeImage := toCRIImage(image) + info, err := c.toCRIImageInfo(ctx, &image, r.GetVerbose()) if err != nil { return nil, errors.Wrap(err, "failed to generate image info") } @@ -55,12 +56,13 @@ func (c *criService) ImageStatus(ctx context.Context, r *runtime.ImageStatusRequ }, nil } -// toCRIRuntimeImage converts internal image object to CRI runtime.Image. -func toCRIRuntimeImage(image *imagestore.Image) *runtime.Image { +// toCRIImage converts internal image object to CRI runtime.Image. +func toCRIImage(image imagestore.Image) *runtime.Image { + repoTags, repoDigests := parseImageReferences(image.References) runtimeImage := &runtime.Image{ Id: image.ID, - RepoTags: image.RepoTags, - RepoDigests: image.RepoDigests, + RepoTags: repoTags, + RepoDigests: repoDigests, Size_: uint64(image.Size), } uid, username := getUserFromImage(image.ImageSpec.Config.User) diff --git a/pkg/server/image_status_test.go b/pkg/server/image_status_test.go index 3be5ff1e9..f1ea878f4 100644 --- a/pkg/server/image_status_test.go +++ b/pkg/server/image_status_test.go @@ -31,11 +31,13 @@ import ( func TestImageStatus(t *testing.T) { testID := "sha256:d848ce12891bf78792cda4a23c58984033b0c397a55e93a1556202222ecc5ed4" image := imagestore.Image{ - ID: testID, - ChainID: "test-chain-id", - RepoTags: []string{"a", "b"}, - RepoDigests: []string{"c", "d"}, - Size: 1234, + ID: testID, + ChainID: "test-chain-id", + References: []string{ + "gcr.io/library/busybox:latest", + "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582", + }, + Size: 1234, ImageSpec: imagespec.Image{ Config: imagespec.ImageConfig{ User: "user:group", @@ -44,8 +46,8 @@ func TestImageStatus(t *testing.T) { } expected := &runtime.Image{ Id: testID, - RepoTags: []string{"a", "b"}, - RepoDigests: []string{"c", "d"}, + RepoTags: []string{"gcr.io/library/busybox:latest"}, + RepoDigests: []string{"gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582"}, Size_: uint64(1234), Username: "user", } @@ -59,7 +61,8 @@ func TestImageStatus(t *testing.T) { require.NotNil(t, resp) assert.Nil(t, resp.GetImage()) - c.imageStore.Add(image) + c.imageStore, err = imagestore.NewFakeStore([]imagestore.Image{image}) + assert.NoError(t, err) t.Logf("should return correct image status for exist image") resp, err = c.ImageStatus(context.Background(), &runtime.ImageStatusRequest{ diff --git a/pkg/server/restart.go b/pkg/server/restart.go index 4b11b7d77..560c75f0f 100644 --- a/pkg/server/restart.go +++ b/pkg/server/restart.go @@ -28,7 +28,6 @@ import ( containerdimages "github.com/containerd/containerd/images" "github.com/containerd/containerd/platforms" "github.com/containerd/typeurl" - "github.com/docker/distribution/reference" "github.com/docker/docker/pkg/system" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -97,16 +96,7 @@ func (c *criService) recover(ctx context.Context) error { if err != nil { return errors.Wrap(err, "failed to list images") } - images, err := loadImages(ctx, cImages, c.config.ContainerdConfig.Snapshotter) - if err != nil { - return errors.Wrap(err, "failed to load images") - } - for _, image := range images { - logrus.Debugf("Loaded image %+v", image) - if err := c.imageStore.Add(image); err != nil { - return errors.Wrapf(err, "failed to add image %q to store", image.ID) - } - } + loadImages(ctx, c.imageStore, cImages, c.config.ContainerdConfig.Snapshotter) // It's possible that containerd containers are deleted unexpectedly. In that case, // we can't even get metadata, we should cleanup orphaned sandbox/container directories @@ -404,26 +394,9 @@ func loadSandbox(ctx context.Context, cntr containerd.Container) (sandboxstore.S } // loadImages loads images from containerd. -// TODO(random-liu): Check whether image is unpacked, because containerd put image reference -// into store before image is unpacked. -func loadImages(ctx context.Context, cImages []containerd.Image, - snapshotter string) ([]imagestore.Image, error) { - // Group images by image id. - imageMap := make(map[string][]containerd.Image) +func loadImages(ctx context.Context, store *imagestore.Store, cImages []containerd.Image, + snapshotter string) { for _, i := range cImages { - desc, err := i.Config(ctx) - if err != nil { - logrus.WithError(err).Warnf("Failed to get image config for %q", i.Name()) - continue - } - id := desc.Digest.String() - imageMap[id] = append(imageMap[id], i) - } - var images []imagestore.Image - for id, imgs := range imageMap { - // imgs len must be > 0, or else the entry will not be created in - // previous loop. - i := imgs[0] ok, _, _, _, err := containerdimages.Check(ctx, i.ContentStore(), i.Target(), platforms.Default()) if err != nil { logrus.WithError(err).Errorf("Failed to check image content readiness for %q", i.Name()) @@ -436,48 +409,19 @@ func loadImages(ctx context.Context, cImages []containerd.Image, // Checking existence of top-level snapshot for each image being recovered. unpacked, err := i.IsUnpacked(ctx, snapshotter) if err != nil { - logrus.WithError(err).Warnf("Failed to Check whether image is unpacked for image %s", i.Name()) + logrus.WithError(err).Warnf("Failed to check whether image is unpacked for image %s", i.Name()) continue } if !unpacked { logrus.Warnf("The image %s is not unpacked.", i.Name()) // TODO(random-liu): Consider whether we should try unpack here. } - - info, err := getImageInfo(ctx, i) - if err != nil { - logrus.WithError(err).Warnf("Failed to get image info for %q", i.Name()) + if err := store.Update(ctx, i.Name()); err != nil { + logrus.WithError(err).Warnf("Failed to update reference for image %q", i.Name()) continue } - image := imagestore.Image{ - ID: id, - ChainID: info.chainID.String(), - Size: info.size, - ImageSpec: info.imagespec, - Image: i, - } - // Recover repo digests and repo tags. - for _, i := range imgs { - name := i.Name() - r, err := reference.ParseAnyReference(name) - if err != nil { - logrus.WithError(err).Warnf("Failed to parse image reference %q", name) - continue - } - if _, ok := r.(reference.Canonical); ok { - image.RepoDigests = append(image.RepoDigests, name) - } else if _, ok := r.(reference.Tagged); ok { - image.RepoTags = append(image.RepoTags, name) - } else if _, ok := r.(reference.Digested); ok { - // This is an image id. - continue - } else { - logrus.Warnf("Invalid image reference %q", name) - } - } - images = append(images, image) + logrus.Debugf("Loaded image %q", i.Name()) } - return images, nil } func cleanupOrphanedIDDirs(cntrs []containerd.Container, base string) error { diff --git a/pkg/server/service.go b/pkg/server/service.go index 13dc9255b..bf9f73e66 100644 --- a/pkg/server/service.go +++ b/pkg/server/service.go @@ -113,7 +113,7 @@ func NewCRIService(config criconfig.Config, client *containerd.Client) (CRIServi os: osinterface.RealOS{}, sandboxStore: sandboxstore.NewStore(), containerStore: containerstore.NewStore(), - imageStore: imagestore.NewStore(), + imageStore: imagestore.NewStore(client), snapshotStore: snapshotstore.NewStore(), sandboxNameIndex: registrar.NewRegistrar(), containerNameIndex: registrar.NewRegistrar(), @@ -157,7 +157,7 @@ func NewCRIService(config criconfig.Config, client *containerd.Client) (CRIServi return nil, errors.Wrap(err, "failed to create stream server") } - c.eventMonitor = newEventMonitor(c.containerStore, c.sandboxStore) + c.eventMonitor = newEventMonitor(c.containerStore, c.sandboxStore, c.imageStore) return c, nil } diff --git a/pkg/server/service_test.go b/pkg/server/service_test.go index 0bc81e670..15eb3a433 100644 --- a/pkg/server/service_test.go +++ b/pkg/server/service_test.go @@ -50,7 +50,7 @@ func newTestCRIService() *criService { imageFSPath: testImageFSPath, os: ostesting.NewFakeOS(), sandboxStore: sandboxstore.NewStore(), - imageStore: imagestore.NewStore(), + imageStore: imagestore.NewStore(nil), snapshotStore: snapshotstore.NewStore(), sandboxNameIndex: registrar.NewRegistrar(), containerStore: containerstore.NewStore(), diff --git a/pkg/store/image/fake_image.go b/pkg/store/image/fake_image.go new file mode 100644 index 000000000..b82d5c408 --- /dev/null +++ b/pkg/store/image/fake_image.go @@ -0,0 +1,34 @@ +/* +Copyright 2018 The Containerd Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package image + +import "github.com/pkg/errors" + +// NewFakeStore returns an image store with predefined images. +// Update is not allowed for this fake store. +func NewFakeStore(images []Image) (*Store, error) { + s := NewStore(nil) + for _, i := range images { + for _, ref := range i.References { + s.refCache[ref] = i.ID + } + if err := s.store.add(i); err != nil { + return nil, errors.Wrapf(err, "add image %q", i) + } + } + return s, nil +} diff --git a/pkg/store/image/image.go b/pkg/store/image/image.go index 80a11c448..f05609082 100644 --- a/pkg/store/image/image.go +++ b/pkg/store/image/image.go @@ -17,14 +17,21 @@ limitations under the License. package image import ( + "context" + "encoding/json" "sync" "github.com/containerd/containerd" + "github.com/containerd/containerd/content" + "github.com/containerd/containerd/errdefs" "github.com/docker/distribution/digestset" - godigest "github.com/opencontainers/go-digest" + imagedigest "github.com/opencontainers/go-digest" + imageidentity "github.com/opencontainers/image-spec/identity" imagespec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" - "github.com/containerd/cri/pkg/store" + storeutil "github.com/containerd/cri/pkg/store" + "github.com/containerd/cri/pkg/util" ) // Image contains all resources associated with the image. All fields @@ -32,10 +39,8 @@ import ( type Image struct { // Id of the image. Normally the digest of image config. ID string - // Other names by which this image is known. - RepoTags []string - // Digests by which this image is known. - RepoDigests []string + // References are references to the image, e.g. RepoTag and RepoDigest. + References []string // ChainID is the chainID of the image. ChainID string // Size is the compressed size of the image. @@ -48,28 +53,156 @@ type Image struct { // Store stores all images. type Store struct { + lock sync.RWMutex + // refCache is a containerd image reference to image id cache. + refCache map[string]string + // client is the containerd client. + client *containerd.Client + // store is the internal image store indexed by image id. + store *store +} + +// NewStore creates an image store. +func NewStore(client *containerd.Client) *Store { + return &Store{ + refCache: make(map[string]string), + client: client, + store: &store{ + images: make(map[string]Image), + digestSet: digestset.NewSet(), + }, + } +} + +// Update updates cache for a reference. +func (s *Store) Update(ctx context.Context, ref string) error { + s.lock.Lock() + defer s.lock.Unlock() + i, err := s.client.GetImage(ctx, ref) + if err != nil && !errdefs.IsNotFound(err) { + return errors.Wrap(err, "get image from containerd") + } + var img *Image + if err == nil { + img, err = getImage(ctx, i) + if err != nil { + return errors.Wrap(err, "get image info from containerd") + } + } + return s.update(ref, img) +} + +// update updates the internal cache. img == nil means that +// the image does not exist in containerd. +func (s *Store) update(ref string, img *Image) error { + oldID, oldExist := s.refCache[ref] + if img == nil { + // The image reference doesn't exist in containerd. + if oldExist { + // Remove the reference from the store. + s.store.delete(oldID, ref) + delete(s.refCache, ref) + } + return nil + } + if oldExist { + if oldID == img.ID { + return nil + } + // Updated. Remove tag from old image. + s.store.delete(oldID, ref) + } + // New image. Add new image. + s.refCache[ref] = img.ID + return s.store.add(*img) +} + +// getImage gets image information from containerd. +func getImage(ctx context.Context, i containerd.Image) (*Image, error) { + // Get image information. + diffIDs, err := i.RootFS(ctx) + if err != nil { + return nil, errors.Wrap(err, "get image diffIDs") + } + chainID := imageidentity.ChainID(diffIDs) + + size, err := i.Size(ctx) + if err != nil { + return nil, errors.Wrap(err, "get image compressed resource size") + } + + desc, err := i.Config(ctx) + if err != nil { + return nil, errors.Wrap(err, "get image config descriptor") + } + id := desc.Digest.String() + + rb, err := content.ReadBlob(ctx, i.ContentStore(), desc) + if err != nil { + return nil, errors.Wrap(err, "read image config from content store") + } + var ociimage imagespec.Image + if err := json.Unmarshal(rb, &ociimage); err != nil { + return nil, errors.Wrapf(err, "unmarshal image config %s", rb) + } + + return &Image{ + ID: id, + References: []string{i.Name()}, + ChainID: chainID.String(), + Size: size, + ImageSpec: ociimage, + Image: i, + }, nil +} + +// Resolve resolves a image reference to image id. +func (s *Store) Resolve(ref string) (string, error) { + s.lock.RLock() + defer s.lock.RUnlock() + id, ok := s.refCache[ref] + if !ok { + return "", storeutil.ErrNotExist + } + return id, nil +} + +// Get gets image metadata by image id. The id can be truncated. +// Returns various validation errors if the image id is invalid. +// Returns storeutil.ErrNotExist if the image doesn't exist. +func (s *Store) Get(id string) (Image, error) { + return s.store.get(id) +} + +// List lists all images. +func (s *Store) List() []Image { + return s.store.list() +} + +type store struct { lock sync.RWMutex images map[string]Image digestSet *digestset.Set } -// NewStore creates an image store. -func NewStore() *Store { - return &Store{ - images: make(map[string]Image), - digestSet: digestset.NewSet(), +func (s *store) list() []Image { + s.lock.RLock() + defer s.lock.RUnlock() + var images []Image + for _, i := range s.images { + images = append(images, i) } + return images } -// Add an image into the store. -func (s *Store) Add(img Image) error { +func (s *store) add(img Image) error { s.lock.Lock() defer s.lock.Unlock() if _, err := s.digestSet.Lookup(img.ID); err != nil { if err != digestset.ErrDigestNotFound { return err } - if err := s.digestSet.Add(godigest.Digest(img.ID)); err != nil { + if err := s.digestSet.Add(imagedigest.Digest(img.ID)); err != nil { return err } } @@ -80,44 +213,29 @@ func (s *Store) Add(img Image) error { s.images[img.ID] = img return nil } - // Or else, merge the repo tags/digests. - i.RepoTags = mergeStringSlices(i.RepoTags, img.RepoTags) - i.RepoDigests = mergeStringSlices(i.RepoDigests, img.RepoDigests) + // Or else, merge the references. + i.References = util.MergeStringSlices(i.References, img.References) s.images[img.ID] = i return nil } -// Get returns the image with specified id. Returns store.ErrNotExist if the -// image doesn't exist. -func (s *Store) Get(id string) (Image, error) { +func (s *store) get(id string) (Image, error) { s.lock.RLock() defer s.lock.RUnlock() digest, err := s.digestSet.Lookup(id) if err != nil { if err == digestset.ErrDigestNotFound { - err = store.ErrNotExist + err = storeutil.ErrNotExist } return Image{}, err } if i, ok := s.images[digest.String()]; ok { return i, nil } - return Image{}, store.ErrNotExist + return Image{}, storeutil.ErrNotExist } -// List lists all images. -func (s *Store) List() []Image { - s.lock.RLock() - defer s.lock.RUnlock() - var images []Image - for _, i := range s.images { - images = append(images, i) - } - return images -} - -// Delete deletes the image with specified id. -func (s *Store) Delete(id string) { +func (s *store) delete(id, ref string) { s.lock.Lock() defer s.lock.Unlock() digest, err := s.digestSet.Lookup(id) @@ -126,22 +244,16 @@ func (s *Store) Delete(id string) { // So we need to return if there are error. return } + i, ok := s.images[digest.String()] + if !ok { + return + } + i.References = util.SubtractStringSlice(i.References, ref) + if len(i.References) != 0 { + s.images[digest.String()] = i + return + } + // Remove the image if it is not referenced any more. s.digestSet.Remove(digest) // nolint: errcheck delete(s.images, digest.String()) } - -// mergeStringSlices merges 2 string slices into one and remove duplicated elements. -func mergeStringSlices(a []string, b []string) []string { - set := map[string]struct{}{} - for _, s := range a { - set[s] = struct{}{} - } - for _, s := range b { - set[s] = struct{}{} - } - var ss []string - for s := range set { - ss = append(ss, s) - } - return ss -} diff --git a/pkg/store/image/image_test.go b/pkg/store/image/image_test.go index 1f1f36809..a09ef550e 100644 --- a/pkg/store/image/image_test.go +++ b/pkg/store/image/image_test.go @@ -17,65 +17,61 @@ limitations under the License. package image import ( + "sort" "strings" "testing" - imagespec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/docker/distribution/digestset" assertlib "github.com/stretchr/testify/assert" - "github.com/containerd/cri/pkg/store" + storeutil "github.com/containerd/cri/pkg/store" ) -func TestImageStore(t *testing.T) { +func TestInternalStore(t *testing.T) { images := []Image{ { - ID: "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - ChainID: "test-chain-id-1", - RepoTags: []string{"tag-1"}, - RepoDigests: []string{"digest-1"}, - Size: 10, - ImageSpec: imagespec.Image{}, + ID: "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + ChainID: "test-chain-id-1", + References: []string{"ref-1"}, + Size: 10, }, { - ID: "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - ChainID: "test-chain-id-2abcd", - RepoTags: []string{"tag-2abcd"}, - RepoDigests: []string{"digest-2abcd"}, - Size: 20, - ImageSpec: imagespec.Image{}, + ID: "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + ChainID: "test-chain-id-2abcd", + References: []string{"ref-2abcd"}, + Size: 20, }, { - ID: "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - RepoTags: []string{"tag-4a333"}, - RepoDigests: []string{"digest-4a333"}, - ChainID: "test-chain-id-4a333", - Size: 30, - ImageSpec: imagespec.Image{}, + ID: "sha256:3123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + References: []string{"ref-4a333"}, + ChainID: "test-chain-id-4a333", + Size: 30, }, { - ID: "sha256:4123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", - RepoTags: []string{"tag-4abcd"}, - RepoDigests: []string{"digest-4abcd"}, - ChainID: "test-chain-id-4abcd", - Size: 40, - ImageSpec: imagespec.Image{}, + ID: "sha256:4123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + References: []string{"ref-4abcd"}, + ChainID: "test-chain-id-4abcd", + Size: 40, }, } assert := assertlib.New(t) genTruncIndex := func(normalName string) string { return normalName[:(len(normalName)+1)/2] } - s := NewStore() + s := &store{ + images: make(map[string]Image), + digestSet: digestset.NewSet(), + } t.Logf("should be able to add image") for _, img := range images { - err := s.Add(img) + err := s.add(img) assert.NoError(err) } t.Logf("should be able to get image") for _, v := range images { truncID := genTruncIndex(v.ID) - got, err := s.Get(truncID) + got, err := s.get(truncID) assert.NoError(err, "truncID:%s, fullID:%s", truncID, v.ID) assert.Equal(v, got) } @@ -83,7 +79,7 @@ func TestImageStore(t *testing.T) { 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) + got, err := s.get(truncID) assert.NoError(err, "truncID:%s, fullID:%s", truncID, v.ID) assert.Equal(v, got) } @@ -91,54 +87,162 @@ func TestImageStore(t *testing.T) { t.Logf("should not be able to get image by ambiguous prefix") ambiguousPrefixs := []string{"sha256", "sha256:"} for _, v := range ambiguousPrefixs { - _, err := s.Get(v) + _, err := s.get(v) assert.NotEqual(nil, err) } t.Logf("should be able to list images") - imgs := s.List() + imgs := s.list() assert.Len(imgs, len(images)) imageNum := len(images) for _, v := range images { truncID := genTruncIndex(v.ID) - oldRepoTag := v.RepoTags[0] - oldRepoDigest := v.RepoDigests[0] - newRepoTag := oldRepoTag + "new" - newRepoDigest := oldRepoDigest + "new" + oldRef := v.References[0] + newRef := oldRef + "new" - t.Logf("should be able to add new repo tags/digests") + t.Logf("should be able to add new references") newImg := v - newImg.RepoTags = []string{newRepoTag} - newImg.RepoDigests = []string{newRepoDigest} - err := s.Add(newImg) + newImg.References = []string{newRef} + err := s.add(newImg) assert.NoError(err) - got, err := s.Get(truncID) + got, err := s.get(truncID) assert.NoError(err) - assert.Len(got.RepoTags, 2) - assert.Contains(got.RepoTags, oldRepoTag, newRepoTag) - assert.Len(got.RepoDigests, 2) - assert.Contains(got.RepoDigests, oldRepoDigest, newRepoDigest) + assert.Len(got.References, 2) + assert.Contains(got.References, oldRef, newRef) - t.Logf("should not be able to add duplicated repo tags/digests") - err = s.Add(newImg) + t.Logf("should not be able to add duplicated references") + err = s.add(newImg) assert.NoError(err) - got, err = s.Get(truncID) + got, err = s.get(truncID) assert.NoError(err) - assert.Len(got.RepoTags, 2) - assert.Contains(got.RepoTags, oldRepoTag, newRepoTag) - assert.Len(got.RepoDigests, 2) - assert.Contains(got.RepoDigests, oldRepoDigest, newRepoDigest) + assert.Len(got.References, 2) + assert.Contains(got.References, oldRef, newRef) + + t.Logf("should be able to delete image references") + s.delete(truncID, oldRef) + got, err = s.get(truncID) + assert.NoError(err) + assert.Equal([]string{newRef}, got.References) t.Logf("should be able to delete image") - s.Delete(truncID) - imageNum-- - imgs = s.List() - assert.Len(imgs, imageNum) + s.delete(truncID, newRef) + got, err = s.get(truncID) + assert.Equal(storeutil.ErrNotExist, err) + assert.Equal(Image{}, got) - t.Logf("get should return empty struct and ErrNotExist after deletion") - img, err := s.Get(truncID) - assert.Equal(Image{}, img) - assert.Equal(store.ErrNotExist, err) + imageNum-- + imgs = s.list() + assert.Len(imgs, imageNum) + } +} + +func TestImageStore(t *testing.T) { + id := "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + newID := "sha256:9923456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + image := Image{ + ID: id, + ChainID: "test-chain-id-1", + References: []string{"ref-1"}, + Size: 10, + } + assert := assertlib.New(t) + + equal := func(i1, i2 Image) { + sort.Strings(i1.References) + sort.Strings(i2.References) + assert.Equal(i1, i2) + } + for desc, test := range map[string]struct { + ref string + image *Image + expected []Image + }{ + "nothing should happen if a non-exist ref disappear": { + ref: "ref-2", + image: nil, + expected: []Image{image}, + }, + "new ref for an existing image": { + ref: "ref-2", + image: &Image{ + ID: id, + ChainID: "test-chain-id-1", + References: []string{"ref-2"}, + Size: 10, + }, + expected: []Image{ + { + ID: id, + ChainID: "test-chain-id-1", + References: []string{"ref-1", "ref-2"}, + Size: 10, + }, + }, + }, + "new ref for a new image": { + ref: "ref-2", + image: &Image{ + ID: newID, + ChainID: "test-chain-id-2", + References: []string{"ref-2"}, + Size: 20, + }, + expected: []Image{ + image, + { + ID: newID, + ChainID: "test-chain-id-2", + References: []string{"ref-2"}, + Size: 20, + }, + }, + }, + "existing ref point to a new image": { + ref: "ref-1", + image: &Image{ + ID: newID, + ChainID: "test-chain-id-2", + References: []string{"ref-1"}, + Size: 20, + }, + expected: []Image{ + { + ID: newID, + ChainID: "test-chain-id-2", + References: []string{"ref-1"}, + Size: 20, + }, + }, + }, + "existing ref disappear": { + ref: "ref-1", + image: nil, + expected: []Image{}, + }, + } { + t.Logf("TestCase %q", desc) + s, err := NewFakeStore([]Image{image}) + assert.NoError(err) + assert.NoError(s.update(test.ref, test.image)) + + assert.Len(s.List(), len(test.expected)) + for _, expect := range test.expected { + got, err := s.Get(expect.ID) + assert.NoError(err) + equal(got, expect) + for _, ref := range expect.References { + id, err := s.Resolve(ref) + assert.NoError(err) + assert.Equal(expect.ID, id) + } + } + + if test.image == nil { + // Shouldn't be able to index by removed ref. + id, err := s.Resolve(test.ref) + assert.Equal(storeutil.ErrNotExist, err) + assert.Empty(id) + } } } diff --git a/pkg/util/strings.go b/pkg/util/strings.go index 2fea28d80..d5cbc2e8e 100644 --- a/pkg/util/strings.go +++ b/pkg/util/strings.go @@ -41,3 +41,19 @@ func SubtractStringSlice(ss []string, str string) []string { } return res } + +// MergeStringSlices merges 2 string slices into one and remove duplicated elements. +func MergeStringSlices(a []string, b []string) []string { + set := map[string]struct{}{} + for _, s := range a { + set[s] = struct{}{} + } + for _, s := range b { + set[s] = struct{}{} + } + var ss []string + for s := range set { + ss = append(ss, s) + } + return ss +} diff --git a/pkg/util/strings_test.go b/pkg/util/strings_test.go index d8ccbaa4b..8946453fa 100644 --- a/pkg/util/strings_test.go +++ b/pkg/util/strings_test.go @@ -46,3 +46,14 @@ func TestSubtractStringSlice(t *testing.T) { assert.Empty(t, SubtractStringSlice(nil, "hij")) assert.Empty(t, SubtractStringSlice([]string{}, "hij")) } + +func TestMergeStringSlices(t *testing.T) { + s1 := []string{"abc", "def", "ghi"} + s2 := []string{"def", "jkl", "mno"} + expect := []string{"abc", "def", "ghi", "jkl", "mno"} + result := MergeStringSlices(s1, s2) + assert.Len(t, result, len(expect)) + for _, s := range expect { + assert.Contains(t, result, s) + } +}