images, containers: converge metadata API conventions

The primary feature we get with this PR is support for filters and
labels on the image metadata store. In the process of doing this, the
conventions for the API have been converged between containers and
images, providing a model for other services.

With images, `Put` (renamed to `Update` briefly) has been split into a
`Create` and `Update`, allowing one to control the behavior around these
operations. `Update` now includes support for masking fields at the
datastore-level across both the containers and image service. Filters
are now just string values to interpreted directly within the data
store. This should allow for some interesting future use cases in which
the datastore might use the syntax for more efficient query paths.

The containers service has been updated to follow these conventions as
closely as possible.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

services/containers/service.go

@@ -1,21 +1,19 @@
 package containers
 
 import (
-	"strings"
-
 	"github.com/boltdb/bolt"
 	api "github.com/containerd/containerd/api/services/containers/v1"
 	eventsapi "github.com/containerd/containerd/api/services/events/v1"
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/events"
-	"github.com/containerd/containerd/filters"
 	"github.com/containerd/containerd/metadata"
 	"github.com/containerd/containerd/plugin"
 	"github.com/golang/protobuf/ptypes/empty"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func init() {
@@ -68,18 +66,8 @@ func (s *Service) Get(ctx context.Context, req *api.GetContainerRequest) (*api.G
 func (s *Service) List(ctx context.Context, req *api.ListContainersRequest) (*api.ListContainersResponse, error) {
 	var resp api.ListContainersResponse
 
-	var fs []filters.Filter
-	for _, s := range req.Filters {
-		f, err := filters.Parse(s)
-		if err != nil {
-			return nil, grpc.Errorf(codes.InvalidArgument, err.Error())
-		}
-
-		fs = append(fs, f)
-	}
-
 	return &resp, errdefs.ToGRPC(s.withStoreView(ctx, func(ctx context.Context, store containers.Store) error {
-		containers, err := store.List(ctx, fs...)
+		containers, err := store.List(ctx, req.Filters...)
 		if err != nil {
 			return err
 		}
@@ -121,52 +109,20 @@ func (s *Service) Create(ctx context.Context, req *api.CreateContainerRequest) (
 }
 
 func (s *Service) Update(ctx context.Context, req *api.UpdateContainerRequest) (*api.UpdateContainerResponse, error) {
+	if req.Container.ID == "" {
+		return nil, status.Errorf(codes.InvalidArgument, "Container.ID required")
+	}
 	var (
-		resp     api.UpdateContainerResponse
-		incoming = containerFromProto(&req.Container)
+		resp      api.UpdateContainerResponse
+		container = containerFromProto(&req.Container)
 	)
 
 	if err := s.withStoreUpdate(ctx, func(ctx context.Context, store containers.Store) error {
-
-		container, err := store.Get(ctx, incoming.ID)
-		if err != nil {
-			return err
-		}
-
-		if container.ID != incoming.ID {
-			return grpc.Errorf(codes.InvalidArgument, "container ids must match: %v != %v", container.ID, incoming.ID)
-		}
-
-		// apply the field mask. If you update this code, you better follow the
-		// field mask rules in field_mask.proto. If you don't know what this
-		// is, do not update this code.
+		var fieldpaths []string
 		if req.UpdateMask != nil && len(req.UpdateMask.Paths) > 0 {
 			for _, path := range req.UpdateMask.Paths {
-				if strings.HasPrefix(path, "labels.") {
-					key := strings.TrimPrefix(path, "labels.")
-					container.Labels[key] = incoming.Labels[key]
-					continue
-				}
-
-				switch path {
-				case "labels":
-					container.Labels = incoming.Labels
-				case "image":
-					container.Image = incoming.Image
-				case "runtime":
-					// TODO(stevvooe): Should this actually be allowed?
-					container.Runtime = incoming.Runtime
-				case "spec":
-					container.Spec = incoming.Spec
-				case "rootfs":
-					container.RootFS = incoming.RootFS
-				default:
-					return grpc.Errorf(codes.InvalidArgument, "cannot update %q field", path)
-				}
+				fieldpaths = append(fieldpaths, path)
 			}
-		} else {
-			// no field mask present, just replace everything
-			container = incoming
 		}
 
 		updated, err := store.Update(ctx, container)

services/images/client.go

@@ -6,7 +6,7 @@ import (
 	imagesapi "github.com/containerd/containerd/api/services/images/v1"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
-	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	ptypes "github.com/gogo/protobuf/types"
 )
 
 type remoteStore struct {
@@ -19,19 +19,6 @@ func NewStoreFromClient(client imagesapi.ImagesClient) images.Store {
 	}
 }
 
-func (s *remoteStore) Update(ctx context.Context, name string, desc ocispec.Descriptor) error {
-	// TODO(stevvooe): Consider that the remote may want to augment and return
-	// a modified image.
-	_, err := s.client.Update(ctx, &imagesapi.UpdateImageRequest{
-		Image: imagesapi.Image{
-			Name:   name,
-			Target: descToProto(&desc),
-		},
-	})
-
-	return errdefs.FromGRPC(err)
-}
-
 func (s *remoteStore) Get(ctx context.Context, name string) (images.Image, error) {
 	resp, err := s.client.Get(ctx, &imagesapi.GetImageRequest{
 		Name: name,
@@ -43,8 +30,10 @@ func (s *remoteStore) Get(ctx context.Context, name string) (images.Image, error
 	return imageFromProto(resp.Image), nil
 }
 
-func (s *remoteStore) List(ctx context.Context) ([]images.Image, error) {
-	resp, err := s.client.List(ctx, &imagesapi.ListImagesRequest{})
+func (s *remoteStore) List(ctx context.Context, filters ...string) ([]images.Image, error) {
+	resp, err := s.client.List(ctx, &imagesapi.ListImagesRequest{
+		Filters: filters,
+	})
 	if err != nil {
 		return nil, errdefs.FromGRPC(err)
 	}
@@ -52,6 +41,36 @@ func (s *remoteStore) List(ctx context.Context) ([]images.Image, error) {
 	return imagesFromProto(resp.Images), nil
 }
 
+func (s *remoteStore) Create(ctx context.Context, image images.Image) (images.Image, error) {
+	created, err := s.client.Create(ctx, &imagesapi.CreateImageRequest{
+		Image: imageToProto(&image),
+	})
+	if err != nil {
+		return images.Image{}, errdefs.FromGRPC(err)
+	}
+
+	return imageFromProto(&created.Image), nil
+}
+
+func (s *remoteStore) Update(ctx context.Context, image images.Image, fieldpaths ...string) (images.Image, error) {
+	var updateMask *ptypes.FieldMask
+	if len(fieldpaths) > 0 {
+		updateMask = &ptypes.FieldMask{
+			Paths: fieldpaths,
+		}
+	}
+
+	updated, err := s.client.Update(ctx, &imagesapi.UpdateImageRequest{
+		Image:      imageToProto(&image),
+		UpdateMask: updateMask,
+	})
+	if err != nil {
+		return images.Image{}, errdefs.FromGRPC(err)
+	}
+
+	return imageFromProto(&updated.Image), nil
+}
+
 func (s *remoteStore) Delete(ctx context.Context, name string) error {
 	_, err := s.client.Delete(ctx, &imagesapi.DeleteImageRequest{
 		Name: name,

services/images/helpers.go

@@ -29,15 +29,21 @@ func imagesFromProto(imagespb []imagesapi.Image) []images.Image {
 
 func imageToProto(image *images.Image) imagesapi.Image {
 	return imagesapi.Image{
-		Name:   image.Name,
-		Target: descToProto(&image.Target),
+		Name:      image.Name,
+		Labels:    image.Labels,
+		Target:    descToProto(&image.Target),
+		CreatedAt: image.CreatedAt,
+		UpdatedAt: image.UpdatedAt,
 	}
 }
 
 func imageFromProto(imagepb *imagesapi.Image) images.Image {
 	return images.Image{
-		Name:   imagepb.Name,
-		Target: descFromProto(&imagepb.Target),
+		Name:      imagepb.Name,
+		Labels:    imagepb.Labels,
+		Target:    descFromProto(&imagepb.Target),
+		CreatedAt: imagepb.CreatedAt,
+		UpdatedAt: imagepb.UpdatedAt,
 	}
 }
 

services/images/service.go

@@ -12,6 +12,8 @@ import (
 	"github.com/golang/protobuf/ptypes/empty"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func init() {
@@ -63,38 +65,88 @@ func (s *Service) Get(ctx context.Context, req *imagesapi.GetImageRequest) (*ima
 	}))
 }
 
-func (s *Service) Update(ctx context.Context, req *imagesapi.UpdateImageRequest) (*imagesapi.UpdateImageResponse, error) {
+func (s *Service) List(ctx context.Context, req *imagesapi.ListImagesRequest) (*imagesapi.ListImagesResponse, error) {
+	var resp imagesapi.ListImagesResponse
+
+	return &resp, errdefs.ToGRPC(s.withStoreView(ctx, func(ctx context.Context, store images.Store) error {
+		images, err := store.List(ctx, req.Filters...)
+		if err != nil {
+			return err
+		}
+
+		resp.Images = imagesToProto(images)
+		return nil
+	}))
+}
+
+func (s *Service) Create(ctx context.Context, req *imagesapi.CreateImageRequest) (*imagesapi.CreateImageResponse, error) {
+	if req.Image.Name == "" {
+		return nil, status.Errorf(codes.InvalidArgument, "Image.Name required")
+	}
+
+	var (
+		image = imageFromProto(&req.Image)
+		resp  imagesapi.CreateImageResponse
+	)
 	if err := s.withStoreUpdate(ctx, func(ctx context.Context, store images.Store) error {
-		return store.Update(ctx, req.Image.Name, descFromProto(&req.Image.Target))
+		created, err := store.Create(ctx, image)
+		if err != nil {
+			return err
+		}
+
+		resp.Image = imageToProto(&created)
+		return nil
+	}); err != nil {
+		return nil, errdefs.ToGRPC(err)
+	}
+
+	if err := s.emit(ctx, "/images/create", &eventsapi.ImageCreate{
+		Name:   resp.Image.Name,
+		Labels: resp.Image.Labels,
+	}); err != nil {
+		return nil, err
+	}
+
+	return &resp, nil
+
+}
+
+func (s *Service) Update(ctx context.Context, req *imagesapi.UpdateImageRequest) (*imagesapi.UpdateImageResponse, error) {
+	if req.Image.Name == "" {
+		return nil, status.Errorf(codes.InvalidArgument, "Image.Name required")
+	}
+
+	var (
+		image = imageFromProto(&req.Image)
+		resp  imagesapi.UpdateImageResponse
+	)
+	if err := s.withStoreUpdate(ctx, func(ctx context.Context, store images.Store) error {
+		var fieldpaths []string
+		if req.UpdateMask != nil && len(req.UpdateMask.Paths) > 0 {
+			for _, path := range req.UpdateMask.Paths {
+				fieldpaths = append(fieldpaths, path)
+			}
+		}
+
+		updated, err := store.Update(ctx, image, fieldpaths...)
+		if err != nil {
+			return err
+		}
+
+		resp.Image = imageToProto(&updated)
+		return nil
 	}); err != nil {
 		return nil, errdefs.ToGRPC(err)
 	}
 
 	if err := s.emit(ctx, "/images/update", &eventsapi.ImageUpdate{
-		Name:   req.Image.Name,
-		Labels: req.Image.Labels,
+		Name:   resp.Image.Name,
+		Labels: resp.Image.Labels,
 	}); err != nil {
 		return nil, err
 	}
 
-	// TODO: get image back out to return
-	return &imagesapi.UpdateImageResponse{
-	//Image: nil,
-	}, nil
-}
-
-func (s *Service) List(ctx context.Context, _ *imagesapi.ListImagesRequest) (*imagesapi.ListImagesResponse, error) {
-	var resp imagesapi.ListImagesResponse
-
-	return &resp, s.withStoreView(ctx, func(ctx context.Context, store images.Store) error {
-		images, err := store.List(ctx)
-		if err != nil {
-			return errdefs.ToGRPC(err)
-		}
-
-		resp.Images = imagesToProto(images)
-		return nil
-	})
+	return &resp, nil
 }
 
 func (s *Service) Delete(ctx context.Context, req *imagesapi.DeleteImageRequest) (*empty.Empty, error) {