Merge pull request #1338 from nalind/fix-runasuser-1.9-cache

[release-1.9] imageService: cache information about images

pkg/storage/image.go

@@ -39,19 +39,30 @@ type ImageResult struct {
 	Size         *uint64
 	Digest       digest.Digest
 	ConfigDigest digest.Digest
+	User         string
 }
 
 type indexInfo struct {
 	name   string
 	secure bool
 }
 
+// A set of information that we prefer to cache about images, so that we can
+// avoid having to reread them every time we need to return information about
+// images.
+type imageCacheItem struct {
+	user         string
+	size         *uint64
+	configDigest digest.Digest
+}
+
 type imageService struct {
 	store                 storage.Store
 	defaultTransport      string
 	insecureRegistryCIDRs []*net.IPNet
 	indexConfigs          map[string]*indexInfo
 	registries            []string
+	imageCache            map[string]imageCacheItem
 }
 
 // sizer knows its size.
@@ -171,15 +182,38 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s
 			return nil, err
 		}
 		if image, err := istorage.Transport.GetStoreImage(svc.store, ref); err == nil {
-			img, err := ref.NewImageSource(systemContext)
-			if err != nil {
-				return nil, err
-			}
-			size := imageSize(img)
-			configDigest, err := imageConfigDigest(img, nil)
-			img.Close()
-			if err != nil {
-				return nil, err
+			var user string
+			var size *uint64
+			var configDigest digest.Digest
+			if cacheItem, ok := svc.imageCache[image.ID]; ok {
+				user, size, configDigest = cacheItem.user, cacheItem.size, cacheItem.configDigest
+			} else {
+				img, err := ref.NewImageSource(systemContext)
+				if err != nil {
+					return nil, err
+				}
+				size = imageSize(img)
+				configDigest, err = imageConfigDigest(img, nil)
+				img.Close()
+				if err != nil {
+					return nil, err
+				}
+				imageFull, err := ref.NewImage(systemContext)
+				if err != nil {
+					return nil, err
+				}
+				defer imageFull.Close()
+				imageConfig, err := imageFull.OCIConfig()
+				if err != nil {
+					return nil, err
+				}
+				user = imageConfig.Config.User
+				cacheItem := imageCacheItem{
+					user:         user,
+					size:         size,
+					configDigest: configDigest,
+				}
+				svc.imageCache[image.ID] = cacheItem
 			}
 			name, tags, digests := sortNamesByType(image.Names)
 			imageDigest, repoDigests := svc.makeRepoDigests(digests, tags, image.ID)
@@ -191,27 +225,73 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s
 				Size:         size,
 				Digest:       imageDigest,
 				ConfigDigest: configDigest,
+				User:         user,
 			})
 		}
 	} else {
 		images, err := svc.store.Images()
 		if err != nil {
 			return nil, err
 		}
-		for _, image := range images {
-			ref, err := istorage.Transport.ParseStoreReference(svc.store, "@"+image.ID)
-			if err != nil {
-				return nil, err
+		visited := make(map[string]struct{})
+		defer func() {
+			// We built a map using IDs of images that we looked
+			// at, so remove any items from the cache that don't
+			// correspond to any of those IDs.
+			removedIDs := make([]string, 0, len(svc.imageCache))
+			for imageID := range svc.imageCache {
+				if _, keep := visited[imageID]; !keep {
+					// We have cached data for an image
+					// with this ID, but it's not in the
+					// list of images now, so the image has
+					// been removed.
+					removedIDs = append(removedIDs, imageID)
+				}
 			}
-			img, err := ref.NewImageSource(systemContext)
-			if err != nil {
-				return nil, err
+			// Handle the removals.
+			for _, removedID := range removedIDs {
+				delete(svc.imageCache, removedID)
 			}
-			size := imageSize(img)
-			configDigest, err := imageConfigDigest(img, nil)
-			img.Close()
-			if err != nil {
-				return nil, err
+		}()
+		for _, image := range images {
+			visited[image.ID] = struct{}{}
+			var user string
+			var size *uint64
+			var configDigest digest.Digest
+			if cacheItem, ok := svc.imageCache[image.ID]; ok {
+				user, size, configDigest = cacheItem.user, cacheItem.size, cacheItem.configDigest
+			} else {
+				ref, err := istorage.Transport.ParseStoreReference(svc.store, "@"+image.ID)
+				if err != nil {
+					return nil, err
+				}
+				img, err := ref.NewImageSource(systemContext)
+				if err != nil {
+					return nil, err
+				}
+				size = imageSize(img)
+				configDigest, err = imageConfigDigest(img, nil)
+				img.Close()
+				if err != nil {
+					return nil, err
+				}
+				imageFull, err := ref.NewImage(systemContext)
+				if err != nil {
+					return nil, err
+				}
+				defer imageFull.Close()
+
+				imageConfig, err := imageFull.OCIConfig()
+				if err != nil {
+					return nil, err
+				}
+				user = imageConfig.Config.User
+				cacheItem := imageCacheItem{
+					user:         user,
+					size:         size,
+					configDigest: configDigest,
+				}
+				svc.imageCache[image.ID] = cacheItem
 			}
 			name, tags, digests := sortNamesByType(image.Names)
 			imageDigest, repoDigests := svc.makeRepoDigests(digests, tags, image.ID)
@@ -223,6 +303,7 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s
 				Size:         size,
 				Digest:       imageDigest,
 				ConfigDigest: configDigest,
+				User:         user,
 			})
 		}
 	}
@@ -246,6 +327,16 @@ func (svc *imageService) ImageStatus(systemContext *types.SystemContext, nameOrI
 	if err != nil {
 		return nil, err
 	}
+	imageFull, err := ref.NewImage(systemContext)
+	if err != nil {
+		return nil, err
+	}
+	defer imageFull.Close()
+
+	imageConfig, err := imageFull.OCIConfig()
+	if err != nil {
+		return nil, err
+	}
 
 	img, err := ref.NewImageSource(systemContext)
 	if err != nil {
@@ -268,6 +359,7 @@ func (svc *imageService) ImageStatus(systemContext *types.SystemContext, nameOrI
 		Size:         size,
 		Digest:       imageDigest,
 		ConfigDigest: configDigest,
+		User:         imageConfig.Config.User,
 	}
 
 	return &result, nil
@@ -585,6 +677,7 @@ func GetImageService(store storage.Store, defaultTransport string, insecureRegis
 		indexConfigs:          make(map[string]*indexInfo, 0),
 		insecureRegistryCIDRs: make([]*net.IPNet, 0),
 		registries:            cleanRegistries,
+		imageCache:            make(map[string]imageCacheItem),
 	}
 
 	insecureRegistries = append(insecureRegistries, "127.0.0.0/8")

server/container_create.go

@@ -446,7 +446,7 @@ func setupContainerUser(specgen *generate.Generator, rootfs string, sc *pb.Linux
 		containerUser := ""
 		// Case 1: run as user is set by kubelet
 		if sc.GetRunAsUser() != nil {
-			containerUser = strconv.FormatInt(sc.GetRunAsUser().Value, 10)
+			containerUser = strconv.FormatInt(sc.GetRunAsUser().GetValue(), 10)
 		} else {
 			// Case 2: run as username is set by kubelet
 			userName := sc.GetRunAsUsername()

server/image_list.go

@@ -31,20 +31,20 @@ func (s *Server) ListImages(ctx context.Context, req *pb.ListImagesRequest) (res
 	}
 	resp = &pb.ListImagesResponse{}
 	for _, result := range results {
+		resImg := &pb.Image{
+			Id:          result.ID,
+			RepoTags:    result.RepoTags,
+			RepoDigests: result.RepoDigests,
+		}
+		uid, username := getUserFromImage(result.User)
+		if uid != nil {
+			resImg.Uid = &pb.Int64Value{Value: *uid}
+		}
+		resImg.Username = username
 		if result.Size != nil {
-			resp.Images = append(resp.Images, &pb.Image{
-				Id:          result.ID,
-				RepoTags:    result.RepoTags,
-				RepoDigests: result.RepoDigests,
-				Size_:       *result.Size,
-			})
-		} else {
-			resp.Images = append(resp.Images, &pb.Image{
-				Id:          result.ID,
-				RepoTags:    result.RepoTags,
-				RepoDigests: result.RepoDigests,
-			})
+			resImg.Size_ = *result.Size
 		}
+		resp.Images = append(resp.Images, resImg)
 	}
 	logrus.Debugf("ListImagesResponse: %+v", resp)
 	return resp, nil

server/image_status.go

@@ -2,6 +2,8 @@ package server
 
 import (
 	"fmt"
+	"strconv"
+	"strings"
 	"time"
 
 	"github.com/containers/storage"
@@ -37,23 +39,62 @@ func (s *Server) ImageStatus(ctx context.Context, req *pb.ImageStatusRequest) (r
 			return nil, err
 		}
 	}
-	// match just the first registry as that's what kube meant
-	image = images[0]
-	status, err := s.StorageImageServer().ImageStatus(s.ImageContext(), image)
-	if err != nil {
-		if errors.Cause(err) == storage.ErrImageUnknown {
-			return &pb.ImageStatusResponse{}, nil
+	var (
+		notfound bool
+		lastErr  error
+	)
+	for _, image := range images {
+		status, err := s.StorageImageServer().ImageStatus(s.ImageContext(), image)
+		if err != nil {
+			if errors.Cause(err) == storage.ErrImageUnknown {
+				logrus.Warnf("imageStatus: can't find %s", image)
+				notfound = true
+				continue
+			}
+			logrus.Warnf("imageStatus: error getting status from %s: %v", image, err)
+			lastErr = err
+			continue
+		}
+		resp = &pb.ImageStatusResponse{
+			Image: &pb.Image{
+				Id:          status.ID,
+				RepoTags:    status.RepoTags,
+				RepoDigests: status.RepoDigests,
+				Size_:       *status.Size,
+			},
+		}
+		uid, username := getUserFromImage(status.User)
+		if uid != nil {
+			resp.Image.Uid = &pb.Int64Value{Value: *uid}
 		}
-		return nil, err
+		resp.Image.Username = username
+		break
 	}
-	resp = &pb.ImageStatusResponse{
-		Image: &pb.Image{
-			Id:          status.ID,
-			RepoTags:    status.RepoTags,
-			RepoDigests: status.RepoDigests,
-			Size_:       *status.Size,
-		},
+	if lastErr != nil && resp == nil {
+		return nil, lastErr
+	}
+	if notfound && resp == nil {
+		return &pb.ImageStatusResponse{}, nil
 	}
 	logrus.Debugf("ImageStatusResponse: %+v", resp)
 	return resp, nil
 }
+
+// getUserFromImage gets uid or user name of the image user.
+// If user is numeric, it will be treated as uid; or else, it is treated as user name.
+func getUserFromImage(user string) (*int64, string) {
+	// return both empty if user is not specified in the image.
+	if user == "" {
+		return nil, ""
+	}
+	// split instances where the id may contain user:group
+	user = strings.Split(user, ":")[0]
+	// user could be either uid or user name. Try to interpret as numeric uid.
+	uid, err := strconv.ParseInt(user, 10, 64)
+	if err != nil {
+		// If user is non numeric, assume it's user name.
+		return nil, user
+	}
+	// If user is a numeric uid.
+	return &uid, ""
+}