From 8957156c4114a7dc2c589ec596fc81cab693e3b9 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 25 Jul 2017 15:39:16 -0400 Subject: [PATCH 1/3] Parse out image names as repotags and repodigests Parse the set of image names as tagged references, canonical references, or repository names to which we add the default tag, and return them in libkpod.ImageData reports. Signed-off-by: Nalin Dahyabhai --- libkpod/image/imageData.go | 45 +++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/libkpod/image/imageData.go b/libkpod/image/imageData.go index 390b4ebe..2cdb7fa0 100644 --- a/libkpod/image/imageData.go +++ b/libkpod/image/imageData.go @@ -4,6 +4,7 @@ import ( "encoding/json" "time" + "github.com/containers/image/docker/reference" "github.com/containers/storage" "github.com/kubernetes-incubator/cri-o/libkpod/driver" digest "github.com/opencontainers/go-digest" @@ -15,8 +16,8 @@ import ( // nolint type ImageData struct { ID string - Names []string - Digests []digest.Digest + Tags []string + Digests []string Parent string Comment string Created *time.Time @@ -58,6 +59,32 @@ type rootFS struct { Layers []string } +// ParseImageNames parses the names we've stored with an image into a list of +// tagged references and a list of references which contain digests. +func ParseImageNames(names []string) (tags, digests []string, err error) { + for _, name := range names { + if named, err := reference.ParseNamed(name); err == nil { + if digested, ok := named.(reference.Digested); ok { + canonical, err := reference.WithDigest(named, digested.Digest()) + if err == nil { + digests = append(digests, canonical.String()) + } + } else { + if reference.IsNameOnly(named) { + named = reference.TagNameOnly(named) + } + if tagged, ok := named.(reference.Tagged); ok { + namedTagged, err := reference.WithTag(named, tagged.Tag()) + if err == nil { + tags = append(tags, namedTagged.String()) + } + } + } + } + } + return tags, digests, nil +} + // 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) @@ -69,7 +96,7 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { if err != nil { return nil, errors.Wrapf(err, "error reading image %q", name) } - digests, err := getDigests(*img) + blobDigests, err := getDigests(*img) if err != nil { return nil, err } @@ -77,8 +104,8 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { var bigData interface{} ctrConfig := containerConfig{} container := "" - if len(digests) > 0 { - bd, err := store.ImageBigData(img.ID, string(digests[len(digests)-1])) + if len(blobDigests) > 0 { + bd, err := store.ImageBigData(img.ID, string(blobDigests[len(blobDigests)-1])) if err != nil { return nil, err } @@ -98,6 +125,11 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { } } + tags, digests, err := ParseImageNames(img.Names) + if err != nil { + return nil, errors.Wrapf(err, "error parsing image names for %q", name) + } + driverName, err := driver.GetDriverName(store) if err != nil { return nil, err @@ -107,6 +139,7 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { if err != nil { return nil, err } + driverMetadata, err := driver.GetDriverMetadata(store, topLayerID) if err != nil { return nil, err @@ -128,7 +161,7 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { return &ImageData{ ID: img.ID, - Names: img.Names, + Tags: tags, Digests: digests, Parent: string(cid.Docker.Parent), Comment: cid.OCIv1.History[0].Comment, From 7e9ac9700bf72eea4645133a275de227cb391984 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 25 Jul 2017 15:23:43 -0400 Subject: [PATCH 2/3] Avoid duplicate image configuration parsing logic Don't bother trying to find and parse the image's configuration blob after we've already done it; just reuse the value. This frees us from making the assumption that the last blob which was committed to local storage was the image's configuration blob. Signed-off-by: Nalin Dahyabhai --- libkpod/image/imageData.go | 70 +++----------------------------------- 1 file changed, 4 insertions(+), 66 deletions(-) diff --git a/libkpod/image/imageData.go b/libkpod/image/imageData.go index 2cdb7fa0..5f5c3e23 100644 --- a/libkpod/image/imageData.go +++ b/libkpod/image/imageData.go @@ -1,13 +1,12 @@ package image import ( - "encoding/json" "time" "github.com/containers/image/docker/reference" "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" "github.com/pkg/errors" ) @@ -22,7 +21,7 @@ type ImageData struct { Comment string Created *time.Time Container string - ContainerConfig containerConfig + ContainerConfig docker.Config Author string Config ociv1.ImageConfig Architecture string @@ -33,27 +32,6 @@ type ImageData struct { RootFS ociv1.RootFS } -type containerConfig struct { - Hostname string - Domainname string - User string - AttachStdin bool - AttachStdout bool - AttachStderr bool - Tty bool - OpenStdin bool - StdinOnce bool - Env []string - Cmd []string - ArgsEscaped bool - Image digest.Digest - Volumes map[string]interface{} - WorkingDir string - Entrypoint []string - Labels interface{} - OnBuild []string -} - type rootFS struct { Type string Layers []string @@ -96,34 +74,6 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { if err != nil { return nil, errors.Wrapf(err, "error reading image %q", name) } - blobDigests, err := getDigests(*img) - if err != nil { - return nil, err - } - - var bigData interface{} - ctrConfig := containerConfig{} - container := "" - if len(blobDigests) > 0 { - bd, err := store.ImageBigData(img.ID, string(blobDigests[len(blobDigests)-1])) - if err != nil { - return nil, err - } - err = json.Unmarshal(bd, &bigData) - if err != nil { - return nil, err - } - - container = (bigData.(map[string]interface{})["container"]).(string) - cc, err := json.MarshalIndent((bigData.(map[string]interface{})["container_config"]).(map[string]interface{}), "", " ") - if err != nil { - return nil, err - } - err = json.Unmarshal(cc, &ctrConfig) - if err != nil { - return nil, err - } - } tags, digests, err := ParseImageNames(img.Names) if err != nil { @@ -166,8 +116,8 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { Parent: string(cid.Docker.Parent), Comment: cid.OCIv1.History[0].Comment, Created: cid.OCIv1.Created, - Container: container, - ContainerConfig: ctrConfig, + Container: cid.Docker.Container, + ContainerConfig: cid.Docker.ContainerConfig, Author: cid.OCIv1.Author, Config: cid.OCIv1.Config, Architecture: cid.OCIv1.Architecture, @@ -181,15 +131,3 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { RootFS: cid.OCIv1.RootFS, }, nil } - -func getDigests(img storage.Image) ([]digest.Digest, error) { - metadata, err := ParseMetadata(img) - if err != nil { - return nil, err - } - digests := []digest.Digest{} - for _, blob := range metadata.Blobs { - digests = append(digests, blob.Digest) - } - return digests, nil -} From cb0bb94c68b06b33859d23dc4a7ec495ee435fc6 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 25 Jul 2017 15:33:41 -0400 Subject: [PATCH 3/3] Avoid parsing image metadata Avoid parsing metadata that the image library keeps in order to find an image's top layer and creation date; instead, use the values which the storage library now makes available, which will be correct once we merge PR #654 or something like it. Instead of assuming the last blob which was added for the image was the manifest, read it directly and compute its digest ourselves. Signed-off-by: Nalin Dahyabhai --- cmd/kpod/images.go | 25 ++++++++---------- libkpod/image/image.go | 52 +++++++++++++------------------------- libkpod/image/imageData.go | 15 ++++------- libkpod/image/metadata.go | 32 ----------------------- 4 files changed, 33 insertions(+), 91 deletions(-) delete mode 100644 libkpod/image/metadata.go diff --git a/cmd/kpod/images.go b/cmd/kpod/images.go index 22943043..3be0f303 100644 --- a/cmd/kpod/images.go +++ b/cmd/kpod/images.go @@ -6,8 +6,8 @@ import ( "text/template" "github.com/containers/storage" - "github.com/kubernetes-incubator/cri-o/libkpod/image" libkpodimage "github.com/kubernetes-incubator/cri-o/libkpod/image" + digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" "github.com/urfave/cli" ) @@ -15,7 +15,7 @@ import ( type imageOutputParams struct { ID string Name string - Digest string + Digest digest.Digest CreatedAt string Size string } @@ -132,16 +132,14 @@ 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 { - imageMetadata, err := image.ParseMetadata(img) - if err != nil { - fmt.Println(err) + createdTime := img.Created.Format("Jan 2, 2006 15:04") + + name := "" + if len(img.Names) > 0 { + name = img.Names[0] } - createdTime := imageMetadata.CreatedTime.Format("Jan 2, 2006 15:04") - digest := "" - if len(imageMetadata.Blobs) > 0 { - digest = string(imageMetadata.Blobs[0].Digest) - } - size, _ := libkpodimage.Size(store, img) + + digest, size, _ := libkpodimage.DigestAndSize(store, img) if quiet { fmt.Printf("%-64s\n", img.ID) @@ -151,14 +149,13 @@ func outputImages(store storage.Store, images []storage.Image, format string, ha params := imageOutputParams{ ID: img.ID, - Name: img.Names[0], + Name: name, Digest: digest, CreatedAt: createdTime, Size: libkpodimage.FormattedSize(size), } if hasTemplate { - err = outputUsingTemplate(format, params) - if err != nil { + if err := outputUsingTemplate(format, params); err != nil { return err } continue diff --git a/libkpod/image/image.go b/libkpod/image/image.go index 2a840d50..e40076d1 100644 --- a/libkpod/image/image.go +++ b/libkpod/image/image.go @@ -8,6 +8,7 @@ import ( is "github.com/containers/image/storage" "github.com/containers/storage" "github.com/kubernetes-incubator/cri-o/libkpod/common" + digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -41,13 +42,13 @@ func ParseFilter(store storage.Store, filter string) (*FilterParams, error) { params.label = pair[1] case "before": if img, err := findImageInSlice(images, pair[1]); err == nil { - params.beforeImage, _ = getCreatedTime(img) + params.beforeImage = img.Created } else { return nil, fmt.Errorf("no such id: %s", pair[0]) } case "since": if img, err := findImageInSlice(images, pair[1]); err == nil { - params.sinceImage, _ = getCreatedTime(img) + params.sinceImage = img.Created } else { return nil, fmt.Errorf("no such id: %s``", pair[0]) } @@ -123,11 +124,7 @@ func matchesBeforeImage(image storage.Image, name string, params *FilterParams) if params.beforeImage.IsZero() { return true } - createdTime, err := getCreatedTime(image) - if err != nil { - return false - } - if createdTime.Before(params.beforeImage) { + if image.Created.Before(params.beforeImage) { return true } return false @@ -139,11 +136,7 @@ func matchesSinceImage(image storage.Image, name string, params *FilterParams) b if params.sinceImage.IsZero() { return true } - createdTime, err := getCreatedTime(image) - if err != nil { - return false - } - if createdTime.After(params.beforeImage) { + if image.Created.After(params.sinceImage) { return true } return false @@ -219,43 +212,32 @@ func findImageInSlice(images []storage.Image, ref string) (storage.Image, error) return storage.Image{}, errors.New("could not find image") } -// Size returns the size of the image in the given store -func Size(store storage.Store, img storage.Image) (int64, error) { +// DigestAndSize returns the size of the image in the given 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) { is.Transport.SetStore(store) storeRef, err := is.Transport.ParseStoreReference(store, "@"+img.ID) if err != nil { - return -1, err + return "", -1, errors.Wrapf(err, "error parsing image reference %q", "@"+img.ID) } imgRef, err := storeRef.NewImage(nil) if err != nil { - return -1, err + return "", -1, errors.Wrapf(err, "error reading image %q", img.ID) } defer imgRef.Close() imgSize, err := imgRef.Size() if err != nil { - return -1, err + return "", -1, errors.Wrapf(err, "error reading size of image %q", img.ID) } - return imgSize, nil -} - -// GetTopLayerID returns the ID of the top layer of the image -func GetTopLayerID(img storage.Image) (string, error) { - metadata, err := ParseMetadata(img) + manifest, _, err := imgRef.Manifest() if err != nil { - return "", err + return "", -1, errors.Wrapf(err, "error reading manifest for image %q", img.ID) } - // Get the digest of the first blob - digest := string(metadata.Blobs[0].Digest) - // Return the first layer associated with the given digest - return metadata.Layers[digest][0], nil -} - -func getCreatedTime(image storage.Image) (time.Time, error) { - metadata, err := ParseMetadata(image) - if err != nil { - return time.Time{}, err + manifestDigest := digest.Digest("") + if len(manifest) > 0 { + manifestDigest = digest.Canonical.FromBytes(manifest) } - return metadata.CreatedTime, nil + return manifestDigest, imgSize, nil } // GetImagesMatchingFilter returns a slice of all images in the store that match the provided FilterParams. diff --git a/libkpod/image/imageData.go b/libkpod/image/imageData.go index 5f5c3e23..5c6da28a 100644 --- a/libkpod/image/imageData.go +++ b/libkpod/image/imageData.go @@ -7,6 +7,7 @@ import ( "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" "github.com/pkg/errors" ) @@ -17,6 +18,7 @@ type ImageData struct { ID string Tags []string Digests []string + Digest digest.Digest Parent string Comment string Created *time.Time @@ -32,11 +34,6 @@ type ImageData struct { RootFS ociv1.RootFS } -type rootFS struct { - Type string - Layers []string -} - // ParseImageNames parses the names we've stored with an image into a list of // tagged references and a list of references which contain digests. func ParseImageNames(names []string) (tags, digests []string, err error) { @@ -85,10 +82,7 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { return nil, err } - topLayerID, err := GetTopLayerID(*img) - if err != nil { - return nil, err - } + topLayerID := img.TopLayer driverMetadata, err := driver.GetDriverMetadata(store, topLayerID) if err != nil { @@ -104,7 +98,7 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { return nil, err } - virtualSize, err := Size(store, *img) + digest, virtualSize, err := DigestAndSize(store, *img) if err != nil { return nil, err } @@ -113,6 +107,7 @@ func GetImageData(store storage.Store, name string) (*ImageData, error) { ID: img.ID, Tags: tags, Digests: digests, + Digest: digest, Parent: string(cid.Docker.Parent), Comment: cid.OCIv1.History[0].Comment, Created: cid.OCIv1.Created, diff --git a/libkpod/image/metadata.go b/libkpod/image/metadata.go deleted file mode 100644 index 53a1cc47..00000000 --- a/libkpod/image/metadata.go +++ /dev/null @@ -1,32 +0,0 @@ -package image - -import ( - "encoding/json" - "strings" - "time" - - "github.com/containers/image/types" - "github.com/containers/storage" -) - -// Metadata stores all of the metadata for an image -type Metadata struct { - Tag string `json:"tag"` - CreatedTime time.Time `json:"created-time"` - ID string `json:"id"` - Blobs []types.BlobInfo `json:"blob-list"` - Layers map[string][]string `json:"layers"` - SignatureSizes []string `json:"signature-sizes"` -} - -// ParseMetadata takes an image, parses the json stored in it's metadata -// field, and converts it to a Metadata struct -func ParseMetadata(image storage.Image) (Metadata, error) { - var m Metadata - - dec := json.NewDecoder(strings.NewReader(image.Metadata)) - if err := dec.Decode(&m); err != nil { - return Metadata{}, err - } - return m, nil -}