libkpod: remove non-OCI information from inspect

Remove the non-OCI version of the configuration structure, and the
parent image ID, from the ImageData that libkpod returns.

At runtime, cri-o is only ever going to use the OCI configuration
(possibly converted from another format by the image library) when
setting up a container, so it can be confusing to display settings in
"kpod inspect" that might be discarded when we try to run a container.

Remove the non-OCI version of the configuration structure, and the
hard-coded Type field, from the ContainerData that libkpod returns.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

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,
-	if err = json.Unmarshal(config, &m); err != nil {
+	// 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,20 +173,37 @@ 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 {
-	if err = json.Unmarshal(config, &m); err != nil {
+			return nil, err
-		return nil, err
+		}
 	}
 
 	labels := make(map[string]string)

libkpod/image/image.go

@@ -6,6 +6,7 @@ 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"
@@ -174,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)
@@ -194,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) {
@@ -211,23 +229,22 @@ func findImageInSlice(images []storage.Image, ref string) (storage.Image, error)
 // InfoAndDigestAndSize returns the inspection info and size of the image in the given
 // store and the digest of its manifest, if it has one, or "" if it doesn't.
 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 nil, "", -1, errors.Wrapf(err, "error parsing image reference %q", "@"+img.ID)
-	}
-	imgRef, err := storeRef.NewImage(nil)
 	if err != nil {
 		return nil, "", -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 nil, "", -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 nil, "", -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 {
@@ -235,7 +252,7 @@ func InfoAndDigestAndSize(store storage.Store, img storage.Image) (*types.ImageI
 	}
 	info, err := imgRef.Inspect()
 	if err != nil {
-		return nil, "", -1, errors.Wrapf(err, "error inspecting image %q", img.ID)
+		return nil, "", -1, errors.Wrapf(err, "error inspecting image %q", transports.ImageName(imgRef.Reference()))
 	}
 	return info, manifestDigest, imgSize, nil
 }

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"
@@ -15,23 +16,23 @@ import (
 // ImageData handles the data used when inspecting a container
 // nolint
 type ImageData struct {
-	ID              string
+	ID           string
-	Tags            []string
+	Tags         []string
-	Digests         []string
+	Digests      []string
-	Digest          digest.Digest
+	Digest       digest.Digest
-	Parent          string
+	Comment      string
-	Comment         string
+	Created      *time.Time
-	Created         *time.Time
+	Container    string
-	Container       string
+	Author       string
-	ContainerConfig docker.Config
+	Config       ociv1.ImageConfig
-	Author          string
+	Architecture string
-	Config          ociv1.ImageConfig
+	OS           string
-	Architecture    string
+	Annotations  map[string]string
-	OS              string
+	CreatedBy    string
-	Size            uint
+	Size         uint
-	VirtualSize     uint
+	VirtualSize  uint
-	GraphDriver     driver.Data
+	GraphDriver  driver.Data
-	RootFS          ociv1.RootFS
+	RootFS       ociv1.RootFS
 }
 
 // ParseImageNames parses the names we've stored with an image into a list of
@@ -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 := InfoAndDigestAndSize(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,
+		ID:           img.ID,
-		Tags:            tags,
+		Tags:         tags,
-		Digests:         digests,
+		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),
-		Size:            uint(size),
+		VirtualSize:  uint(size + imgSize),
-		VirtualSize:     uint(virtualSize),
 		GraphDriver: driver.Data{
 			Name: driverName,
 			Data: driverMetadata,
 		},
-		RootFS: cid.OCIv1.RootFS,
+		RootFS: config.RootFS,
 	}, nil
 }