Merge pull request #700 from nalind/kpod-updates

kpod: display image dates from inspection data, and don't display non-OCI settings in "inspect"

cmd/kpod/images.go

@@ -136,14 +136,17 @@ func outputHeader(truncate, digests bool) {
 
 func outputImages(store storage.Store, images []storage.Image, format string, hasTemplate, truncate, digests, quiet bool) error {
 	for _, img := range images {
-		createdTime := img.Created.Format("Jan 2, 2006 15:04")
+		createdTime := img.Created
 
 		name := ""
 		if len(img.Names) > 0 {
 			name = img.Names[0]
 		}
 
-		digest, size, _ := libkpodimage.DigestAndSize(store, img)
+		info, digest, size, _ := libkpodimage.InfoAndDigestAndSize(store, img)
+		if info != nil {
+			createdTime = info.Created
+		}
 
 		if quiet {
 			fmt.Printf("%-64s\n", img.ID)
@@ -155,7 +158,7 @@ func outputImages(store storage.Store, images []storage.Image, format string, ha
 			ID:        img.ID,
 			Name:      name,
 			Digest:    digest,
-			CreatedAt: createdTime,
+			CreatedAt: createdTime.Format("Jan 2, 2006 15:04"),
 			Size:      libkpodimage.FormattedSize(size),
 		}
 		if hasTemplate {

libkpod/containerData.go

@@ -2,13 +2,13 @@ package libkpod
 
 import (
 	"encoding/json"
+	"os"
 	"time"
 
 	"k8s.io/apimachinery/pkg/fields"
 	pb "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 
 	"github.com/containers/storage"
-	"github.com/kubernetes-incubator/cri-o/cmd/kpod/docker"
 	"github.com/kubernetes-incubator/cri-o/libkpod/common"
 	"github.com/kubernetes-incubator/cri-o/libkpod/driver"
 	libkpodimage "github.com/kubernetes-incubator/cri-o/libkpod/image"
@@ -30,17 +30,15 @@ type ContainerData struct {
 	Metadata         *pb.ContainerMetadata
 	BundlePath       string
 	StopSignal       string
-	Type             string `json:"type"`
+	FromImage        string `json:"Image,omitempty"`
-	FromImage        string `json:"image,omitempty"`
+	FromImageID      string `json:"ImageID"`
-	FromImageID      string `json:"image-id"`
+	MountPoint       string `json:"Mountpoint,omitempty"`
-	MountPoint       string `json:"mountpoint,omitempty"`
 	MountLabel       string
 	Mounts           []specs.Mount
 	AppArmorProfile  string
-	ImageAnnotations map[string]string `json:"annotations,omitempty"`
+	ImageAnnotations map[string]string `json:"Annotations,omitempty"`
-	ImageCreatedBy   string            `json:"created-by,omitempty"`
+	ImageCreatedBy   string            `json:"CreatedBy,omitempty"`
-	OCIv1            v1.Image          `json:"ociv1,omitempty"`
+	Config           v1.ImageConfig    `json:"Config,omitempty"`
-	Docker           docker.V2Image    `json:"docker,omitempty"`
 	SizeRw           uint              `json:"SizeRw,omitempty"`
 	SizeRootFs       uint              `json:"SizeRootFs,omitempty"`
 	Args             []string
@@ -62,19 +60,31 @@ func GetContainerData(store storage.Store, name string, size bool) (*ContainerDa
 	if err != nil {
 		return nil, errors.Wrapf(err, "error reading build container %q", name)
 	}
-	cid, err := libkpodimage.GetContainerCopyData(store, name)
+	container, err := store.Container(name)
 	if err != nil {
-		return nil, errors.Wrapf(err, "error reading container image data")
+		return nil, errors.Wrapf(err, "error reading container data")
-	}
-	config, err := store.FromContainerDirectory(ctr.ID(), "config.json")
-	if err != nil {
-		return nil, err
 	}
 
-	var m specs.Spec
+	// The runtime configuration won't exist if the container has never been started by cri-o or kpod,
+	// so treat a not-exist error as non-fatal.
+	m := getBlankSpec()
+	config, err := store.FromContainerDirectory(ctr.ID(), "config.json")
+	if err != nil && !os.IsNotExist(errors.Cause(err)) {
+		return nil, err
+	}
+	if len(config) > 0 {
 		if err = json.Unmarshal(config, &m); err != nil {
 			return nil, err
 		}
+	}
+
+	if container.ImageID == "" {
+		return nil, errors.Errorf("error reading container image data: container is not based on an image")
+	}
+	imageData, err := libkpodimage.GetImageData(store, container.ImageID)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error reading container image data")
+	}
 
 	driverName, err := driver.GetDriverName(store)
 	if err != nil {
@@ -84,10 +94,20 @@ func GetContainerData(store storage.Store, name string, size bool) (*ContainerDa
 	if err != nil {
 		return nil, err
 	}
+	layer, err := store.Layer(topLayer)
+	if err != nil {
+		return nil, err
+	}
 	driverMetadata, err := driver.GetDriverMetadata(store, topLayer)
 	if err != nil {
 		return nil, err
 	}
+	imageName := ""
+	if len(imageData.Tags) > 0 {
+		imageName = imageData.Tags[0]
+	} else if len(imageData.Digests) > 0 {
+		imageName = imageData.Digests[0]
+	}
 	data := &ContainerData{
 		ID:               ctr.ID(),
 		Name:             ctr.Name(),
@@ -99,14 +119,12 @@ func GetContainerData(store storage.Store, name string, size bool) (*ContainerDa
 		BundlePath:       ctr.BundlePath(),
 		StopSignal:       ctr.GetStopSignal(),
 		Args:             m.Process.Args,
-		Type:             cid.Type,
+		FromImage:        imageName,
-		FromImage:        cid.FromImage,
+		FromImageID:      container.ImageID,
-		FromImageID:      cid.FromImageID,
+		MountPoint:       layer.MountPoint,
-		MountPoint:       cid.MountPoint,
+		ImageAnnotations: imageData.Annotations,
-		ImageAnnotations: cid.ImageAnnotations,
+		ImageCreatedBy:   imageData.CreatedBy,
-		ImageCreatedBy:   cid.ImageCreatedBy,
+		Config:           imageData.Config,
-		OCIv1:            cid.OCIv1,
-		Docker:           cid.Docker,
 		GraphDriver: driverData{
 			Name: driverName,
 			Data: driverMetadata,
@@ -155,21 +173,38 @@ func inspectContainer(store storage.Store, container string) (*oci.Container, er
 	return ociCtr, nil
 }
 
+func getBlankSpec() specs.Spec {
+	return specs.Spec{
+		Process:     &specs.Process{},
+		Root:        &specs.Root{},
+		Mounts:      []specs.Mount{},
+		Hooks:       &specs.Hooks{},
+		Annotations: make(map[string]string),
+		Linux:       &specs.Linux{},
+		Solaris:     &specs.Solaris{},
+		Windows:     &specs.Windows{},
+	}
+}
+
 // get an oci.Container instance for a given container ID
 func getOCIContainer(store storage.Store, container string) (*oci.Container, error) {
 	ctr, err := FindContainer(store, container)
 	if err != nil {
 		return nil, err
 	}
+
+	// The runtime configuration won't exist if the container has never been started by cri-o or kpod,
+	// so treat a not-exist error as non-fatal.
+	m := getBlankSpec()
 	config, err := store.FromContainerDirectory(ctr.ID, "config.json")
-	if err != nil {
+	if err != nil && !os.IsNotExist(errors.Cause(err)) {
 		return nil, err
 	}
-
+	if len(config) > 0 {
-	var m specs.Spec
 		if err = json.Unmarshal(config, &m); err != nil {
 			return nil, err
 		}
+	}
 
 	labels := make(map[string]string)
 	err = json.Unmarshal([]byte(m.Annotations[annotations.Labels]), &labels)

libkpod/image/image.go

@@ -6,6 +6,8 @@ import (
 	"time"
 
 	is "github.com/containers/image/storage"
+	"github.com/containers/image/transports"
+	"github.com/containers/image/types"
 	"github.com/containers/storage"
 	"github.com/kubernetes-incubator/cri-o/libkpod/common"
 	digest "github.com/opencontainers/go-digest"
@@ -65,13 +67,28 @@ func matchesFilter(store storage.Store, image storage.Image, name string, params
 	if params == nil {
 		return true
 	}
+
+	storeRef, err := is.Transport.ParseStoreReference(store, "@"+image.ID)
+	if err != nil {
+		return false
+	}
+	img, err := storeRef.NewImage(nil)
+	if err != nil {
+		return false
+	}
+	defer img.Close()
+	info, err := img.Inspect()
+	if err != nil {
+		return false
+	}
+
 	if params.dangling != "" && !matchesDangling(name, params.dangling) {
 		return false
-	} else if params.label != "" && !matchesLabel(image, store, params.label) {
+	} else if params.label != "" && !matchesLabel(info, store, params.label) {
 		return false
-	} else if !params.beforeImage.IsZero() && !matchesBeforeImage(image, name, params) {
+	} else if !params.beforeImage.IsZero() && !matchesBeforeImage(info, name, params) {
 		return false
-	} else if !params.sinceImage.IsZero() && !matchesSinceImage(image, name, params) {
+	} else if !params.sinceImage.IsZero() && !matchesSinceImage(info, name, params) {
 		return false
 	} else if params.referencePattern != "" && !MatchesReference(name, params.referencePattern) {
 		return false
@@ -88,21 +105,7 @@ func matchesDangling(name string, dangling string) bool {
 	return false
 }
 
-func matchesLabel(image storage.Image, store storage.Store, label string) bool {
+func matchesLabel(info *types.ImageInspectInfo, store storage.Store, label string) bool {
-	storeRef, err := is.Transport.ParseStoreReference(store, "@"+image.ID)
-	if err != nil {
-
-	}
-	img, err := storeRef.NewImage(nil)
-	if err != nil {
-		return false
-	}
-	defer img.Close()
-	info, err := img.Inspect()
-	if err != nil {
-		return false
-	}
-
 	pair := strings.SplitN(label, "=", 2)
 	for key, value := range info.Labels {
 		if key == pair[0] {
@@ -120,11 +123,8 @@ func matchesLabel(image storage.Image, store storage.Store, label string) bool {
 
 // Returns true if the image was created since the filter image.  Returns
 // false otherwise
-func matchesBeforeImage(image storage.Image, name string, params *FilterParams) bool {
+func matchesBeforeImage(info *types.ImageInspectInfo, name string, params *FilterParams) bool {
-	if params.beforeImage.IsZero() {
+	if info.Created.Before(params.beforeImage) {
-		return true
-	}
-	if image.Created.Before(params.beforeImage) {
 		return true
 	}
 	return false
@@ -132,11 +132,8 @@ func matchesBeforeImage(image storage.Image, name string, params *FilterParams)
 
 // Returns true if the image was created since the filter image.  Returns
 // false otherwise
-func matchesSinceImage(image storage.Image, name string, params *FilterParams) bool {
+func matchesSinceImage(info *types.ImageInspectInfo, name string, params *FilterParams) bool {
-	if params.sinceImage.IsZero() {
+	if info.Created.After(params.sinceImage) {
-		return true
-	}
-	if image.Created.After(params.sinceImage) {
 		return true
 	}
 	return false
@@ -178,7 +175,7 @@ func FormattedSize(size int64) string {
 	return fmt.Sprintf("%.4g %s", formattedSize, suffixes[count])
 }
 
-// FindImage searches for an image with a matching the given name or ID in the given store
+// FindImage searches for a *storage.Image with a matching the given name or ID in the given store.
 func FindImage(store storage.Store, image string) (*storage.Image, error) {
 	var img *storage.Image
 	ref, err := is.Transport.ParseStoreReference(store, image)
@@ -198,6 +195,23 @@ func FindImage(store storage.Store, image string) (*storage.Image, error) {
 	return img, nil
 }
 
+// FindImageRef searches for and returns a new types.Image matching the given name or ID in the given store.
+func FindImageRef(store storage.Store, image string) (types.Image, error) {
+	img, err := FindImage(store, image)
+	if err != nil {
+		return nil, errors.Wrapf(err, "unable to locate image %q", image)
+	}
+	ref, err := is.Transport.ParseStoreReference(store, "@"+img.ID)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error parsing reference to image %q", img.ID)
+	}
+	imgRef, err := ref.NewImage(nil)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error reading image %q", img.ID)
+	}
+	return imgRef, nil
+}
+
 func findImageInSlice(images []storage.Image, ref string) (storage.Image, error) {
 	for _, image := range images {
 		if MatchesID(image.ID, ref) {
@@ -212,32 +226,35 @@ func findImageInSlice(images []storage.Image, ref string) (storage.Image, error)
 	return storage.Image{}, errors.New("could not find image")
 }
 
-// DigestAndSize returns the size of the image in the given store and the
+// InfoAndDigestAndSize returns the inspection info and size of the image in the given
-// digest of its manifest, if it has one, or "" if it doesn't.
+// store and the digest of its manifest, if it has one, or "" if it doesn't.
-func DigestAndSize(store storage.Store, img storage.Image) (digest.Digest, int64, error) {
+func InfoAndDigestAndSize(store storage.Store, img storage.Image) (*types.ImageInspectInfo, digest.Digest, int64, error) {
-	is.Transport.SetStore(store)
+	imgRef, err := FindImageRef(store, "@"+img.ID)
-	storeRef, err := is.Transport.ParseStoreReference(store, "@"+img.ID)
 	if err != nil {
-		return "", -1, errors.Wrapf(err, "error parsing image reference %q", "@"+img.ID)
+		return nil, "", -1, errors.Wrapf(err, "error reading image %q", img.ID)
-	}
-	imgRef, err := storeRef.NewImage(nil)
-	if err != nil {
-		return "", -1, errors.Wrapf(err, "error reading image %q", img.ID)
 	}
 	defer imgRef.Close()
+	return infoAndDigestAndSize(imgRef)
+}
+
+func infoAndDigestAndSize(imgRef types.Image) (*types.ImageInspectInfo, digest.Digest, int64, error) {
 	imgSize, err := imgRef.Size()
 	if err != nil {
-		return "", -1, errors.Wrapf(err, "error reading size of image %q", img.ID)
+		return nil, "", -1, errors.Wrapf(err, "error reading size of image %q", transports.ImageName(imgRef.Reference()))
 	}
 	manifest, _, err := imgRef.Manifest()
 	if err != nil {
-		return "", -1, errors.Wrapf(err, "error reading manifest for image %q", img.ID)
+		return nil, "", -1, errors.Wrapf(err, "error reading manifest for image %q", transports.ImageName(imgRef.Reference()))
 	}
 	manifestDigest := digest.Digest("")
 	if len(manifest) > 0 {
 		manifestDigest = digest.Canonical.FromBytes(manifest)
 	}
-	return manifestDigest, imgSize, nil
+	info, err := imgRef.Inspect()
+	if err != nil {
+		return nil, "", -1, errors.Wrapf(err, "error inspecting image %q", transports.ImageName(imgRef.Reference()))
+	}
+	return info, manifestDigest, imgSize, nil
 }
 
 // GetImagesMatchingFilter returns a slice of all images in the store that match the provided FilterParams.

libkpod/image/imageData.go

@@ -1,11 +1,12 @@
 package image
 
 import (
+	"encoding/json"
 	"time"
 
 	"github.com/containers/image/docker/reference"
+	"github.com/containers/image/transports"
 	"github.com/containers/storage"
-	"github.com/kubernetes-incubator/cri-o/cmd/kpod/docker"
 	"github.com/kubernetes-incubator/cri-o/libkpod/driver"
 	digest "github.com/opencontainers/go-digest"
 	ociv1 "github.com/opencontainers/image-spec/specs-go/v1"
@@ -19,15 +20,15 @@ type ImageData struct {
 	Tags         []string
 	Digests      []string
 	Digest       digest.Digest
-	Parent          string
 	Comment      string
 	Created      *time.Time
 	Container    string
-	ContainerConfig docker.Config
 	Author       string
 	Config       ociv1.ImageConfig
 	Architecture string
 	OS           string
+	Annotations  map[string]string
+	CreatedBy    string
 	Size         uint
 	VirtualSize  uint
 	GraphDriver  driver.Data
@@ -60,6 +61,20 @@ func ParseImageNames(names []string) (tags, digests []string, err error) {
 	return tags, digests, nil
 }
 
+func annotations(manifest []byte, manifestType string) map[string]string {
+	annotations := make(map[string]string)
+	switch manifestType {
+	case ociv1.MediaTypeImageManifest:
+		var m ociv1.Manifest
+		if err := json.Unmarshal(manifest, &m); err == nil {
+			for k, v := range m.Annotations {
+				annotations[k] = v
+			}
+		}
+	}
+	return annotations
+}
+
 // GetImageData gets the ImageData for a container with the given name in the given store.
 func GetImageData(store storage.Store, name string) (*ImageData, error) {
 	img, err := FindImage(store, name)
@@ -67,10 +82,11 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) {
 		return nil, errors.Wrapf(err, "error reading image %q", name)
 	}
 
-	cid, err := GetImageCopyData(store, name)
+	imgRef, err := FindImageRef(store, "@"+img.ID)
 	if err != nil {
-		return nil, errors.Wrapf(err, "error reading image %q", name)
+		return nil, errors.Wrapf(err, "error reading image %q", img.ID)
 	}
+	defer imgRef.Close()
 
 	tags, digests, err := ParseImageNames(img.Names)
 	if err != nil {
@@ -79,50 +95,70 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) {
 
 	driverName, err := driver.GetDriverName(store)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrapf(err, "error reading name of storage driver")
 	}
 
 	topLayerID := img.TopLayer
 
 	driverMetadata, err := driver.GetDriverMetadata(store, topLayerID)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrapf(err, "error asking storage driver %q for metadata", driverName)
 	}
 
 	layer, err := store.Layer(topLayerID)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrapf(err, "error reading information about layer %q", topLayerID)
 	}
 	size, err := store.DiffSize(layer.Parent, layer.ID)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrapf(err, "error determining size of layer %q", layer.ID)
 	}
 
-	digest, virtualSize, err := DigestAndSize(store, *img)
+	imgSize, err := imgRef.Size()
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrapf(err, "error determining size of image %q", transports.ImageName(imgRef.Reference()))
+	}
+
+	manifest, manifestType, err := imgRef.Manifest()
+	if err != nil {
+		return nil, errors.Wrapf(err, "error reading manifest for image %q", img.ID)
+	}
+	manifestDigest := digest.Digest("")
+	if len(manifest) > 0 {
+		manifestDigest = digest.Canonical.FromBytes(manifest)
+	}
+	annotations := annotations(manifest, manifestType)
+
+	config, err := imgRef.OCIConfig()
+	if err != nil {
+		return nil, errors.Wrapf(err, "error reading image configuration for %q", img.ID)
+	}
+	historyComment := ""
+	historyCreatedBy := ""
+	if len(config.History) > 0 {
+		historyComment = config.History[len(config.History)-1].Comment
+		historyCreatedBy = config.History[len(config.History)-1].CreatedBy
 	}
 
 	return &ImageData{
 		ID:           img.ID,
 		Tags:         tags,
 		Digests:      digests,
-		Digest:          digest,
+		Digest:       manifestDigest,
-		Parent:          string(cid.Docker.Parent),
+		Comment:      historyComment,
-		Comment:         cid.OCIv1.History[0].Comment,
+		Created:      config.Created,
-		Created:         cid.OCIv1.Created,
+		Author:       config.Author,
-		Container:       cid.Docker.Container,
+		Config:       config.Config,
-		ContainerConfig: cid.Docker.ContainerConfig,
+		Architecture: config.Architecture,
-		Author:          cid.OCIv1.Author,
+		OS:           config.OS,
-		Config:          cid.OCIv1.Config,
+		Annotations:  annotations,
-		Architecture:    cid.OCIv1.Architecture,
+		CreatedBy:    historyCreatedBy,
-		OS:              cid.OCIv1.OS,
 		Size:         uint(size),
-		VirtualSize:     uint(virtualSize),
+		VirtualSize:  uint(size + imgSize),
 		GraphDriver: driver.Data{
 			Name: driverName,
 			Data: driverMetadata,
 		},
-		RootFS: cid.OCIv1.RootFS,
+		RootFS: config.RootFS,
 	}, nil
 }