adding info map for verbose image status #470

mikebrow · 2017-12-04T21:17:50Z

This commit addresses #359 image status debug, adds verbose info to image status request, and also does some refactoring.

Signed-off-by: Mike Brown [email protected]

mikebrow · 2017-12-04T21:18:58Z

mike@mike-VirtualBox:~/go/src/github.com/kubernetes-incubator/cri-tools$ sudo crictl --runtime-endpoint /var/run/cri-containerd.sock inspecti busybox
{
  "status": {
    "id": "sha256:6ad733544a6317992a6fac4eb19fe1df577d4dec7529efec28a5bd0edad0fd30",
    "repoTags": [
      "docker.io/library/busybox:latest"
    ],
    "repoDigests": [
      "docker.io/library/busybox@sha256:bbc3a03235220b170ba48a157dd097dd1379299370e1ed99ce976df0355d24f0"
    ],
    "size": "720019",
    "username": ""
  },
  "imageConfig": {
    "Env": [
      "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
    ],
    "Cmd": [
      "sh"
    ]
  },
  "imageConfigDescriptor": {
    "mediaType": "application/vnd.docker.container.image.v1+json",
    "digest": "sha256:6ad733544a6317992a6fac4eb19fe1df577d4dec7529efec28a5bd0edad0fd30",
    "size": 1497
  },
  "imageTargetDescriptor": {
    "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
    "digest": "sha256:bbc3a03235220b170ba48a157dd097dd1379299370e1ed99ce976df0355d24f0",
    "size": 2699
  },
  "imageTargetDigests": [
    "sha256:0271b8eebde3fa9a6126b1f2335e170f902731ab4942f9f1914e77016540c7bb"
  ],
  "imageTargetDigestsInfo": [
    {
      "Digest": "sha256:0271b8eebde3fa9a6126b1f2335e170f902731ab4942f9f1914e77016540c7bb",
      "Size": 1337856,
      "CreatedAt": "2017-11-02T17:24:57.366043703Z",
      "UpdatedAt": "2017-11-02T17:24:57.386371651Z",
      "Labels": null
    }
  ]
}

miaoyq

I think we can rename the name corresponding with OCI, like:
imageConfig -> config
imageConfigDescriptor -> configDescriptor
imageTargetDescriptor -> manifestDescribor
imageTargetDigestsInfo -> layerInfo

wdyt?

miaoyq · 2017-12-05T03:31:01Z

pkg/server/image_status.go

+	if err != nil {
+		info["imageTargetDigests"] = err.Error()
+	} else {
+		info["imageTargetDigests"] = marshallToString(digests)


I think imageTargetDigests is redundant since imageTargetDigestsInfo have contained this content.

changed my mind, sounds good..

mikebrow · 2017-12-05T13:34:41Z

I think we can rename the name corresponding with OCI, like:
imageConfig -> config
imageConfigDescriptor -> configDescriptor
imageTargetDescriptor -> manifestDescribor
imageTargetDigestsInfo -> layerInfo

wdyt?

I like it! Will do. and done

mikebrow · 2017-12-06T00:01:48Z

Updated example output:

mike@mike-VirtualBox:~/go/src/github.com/kubernetes-incubator/cri-tools$ sudo crictl --runtime-endpoint /var/run/cri-containerd.sock inspecti busybox
{
  "status": {
    "id": "sha256:6ad733544a6317992a6fac4eb19fe1df577d4dec7529efec28a5bd0edad0fd30",
    "repoTags": [
      "docker.io/library/busybox:latest"
    ],
    "repoDigests": [
      "docker.io/library/busybox@sha256:bbc3a03235220b170ba48a157dd097dd1379299370e1ed99ce976df0355d24f0"
    ],
    "size": "720019",
    "username": ""
  },
  "info": {
    "config": {
      "Env": [
        "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
      ],
      "Cmd": [
        "sh"
      ]
    },
    "configDescriptor": {
      "mediaType": "application/vnd.docker.container.image.v1+json",
      "digest": "sha256:6ad733544a6317992a6fac4eb19fe1df577d4dec7529efec28a5bd0edad0fd30",
      "size": 1497
    },
    "manifestDescriptor": {
      "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
      "digest": "sha256:bbc3a03235220b170ba48a157dd097dd1379299370e1ed99ce976df0355d24f0",
      "size": 2699
    },
    "layerInfo": [
      {
        "Digest": "sha256:0271b8eebde3fa9a6126b1f2335e170f902731ab4942f9f1914e77016540c7bb",
        "Size": 1337856,
        "CreatedAt": "2017-11-02T17:24:57.366043703Z",
        "UpdatedAt": "2017-11-02T17:24:57.386371651Z",
        "Labels": null
      }
    ]
  }
}

miaoyq · 2017-12-06T00:38:12Z

/lgtm

Random-Liu · 2017-12-11T18:59:04Z

pkg/server/image_status.go

+		glog.Errorf("Failed to get target digests %q: %v", i.Name(), err)
+	} else {
+		dia = make([]content.Info, len(digests))
+		for i, d := range digests {


This is wrong. RootFS digest is uncompressed digest, content is compressed.
You won't be able to get things from content store info with RootFS digest:

{ "status": { "id": "sha256:c30178c5239f2937c21c261b0365efcda25be4921ccb95acd63beeeb78786f27", "repoTags": [ "docker.io/library/busybox:1.26" ], "repoDigests": [ "docker.io/library/busybox@sha256:be3c11fdba7cfe299214e46edc642e09514dbb9bbefcd0d3836c05a1e0cd0642" ], "size": "701277", "username": "" }, "info": { "config": { "Env": [ "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ], "Cmd": [ "sh" ] }, "configDescriptor": { "mediaType": "application/vnd.docker.container.image.v1+json", "digest": "sha256:c30178c5239f2937c21c261b0365efcda25be4921ccb95acd63beeeb78786f27", "size": 1507 }, "manifestDescriptor": { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "digest": "sha256:be3c11fdba7cfe299214e46edc642e09514dbb9bbefcd0d3836c05a1e0cd0642", "size": 527 }, "layerInfo": [ { "Digest": "", "Size": 0, "CreatedAt": "0001-01-01T00:00:00Z", "UpdatedAt": "0001-01-01T00:00:00Z", "Labels": null } ] } }

We may want to include the whole image spec imagespec.Image, which has already contained everything.
We also need ChainID here, because that's how we find the corresponding snapshot in containerd.

Agree we should omit, if content isn't accessible.
Agree, we may want to include the whole imagespec.Image.
Agree, to add chainID.

@mikebrow You could change imagestore.Image.Config to imagestore.Image.ImageSpec and change all the other referencing place.

So that you don't need to read that from containerd again.

@Random-Liu added a commit to place the full OCI image spec into the imagestore.. Debugging it now.

Signed-off-by: Mike Brown <[email protected]>

mikebrow · 2017-12-12T21:52:54Z

Updated inspect:

mike@mike-VirtualBox:~/go/src/github.com/kubernetes-incubator/test$ sudo crictl inspecti nginx
{
  "status": {
    "id": "sha256:f895b3fb9e3032cddf68d798ce00c46be433e15285c99b12d51c1b1ae7671334",
    "repoTags": [
      "docker.io/library/nginx:latest"
    ],
    "repoDigests": [
      "docker.io/library/nginx@sha256:c319c3d05d7e00ace3c90d0829d9a4f35bf3adaf0f7134f31fd599edeefe9a08"
    ],
    "size": "44448522",
    "username": ""
  },
  "info": {
    "chainID": "sha256:370edf08d0553496c68f9f53a7f2aff9fbf835440ed6c97b754c4f90b2d14ab6",
    "imageSpec": {
      "created": "2017-12-12T05:15:03.787465224Z",
      "architecture": "amd64",
      "os": "linux",
      "config": {
        "ExposedPorts": {
          "80/tcp": {}
        },
        "Env": [
          "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
          "NGINX_VERSION=1.13.7-1~stretch",
          "NJS_VERSION=1.13.7.0.1.15-1~stretch"
        ],
        "Cmd": [
          "nginx",
          "-g",
          "daemon off;"
        ],
        "Labels": {
          "maintainer": "NGINX Docker Maintainers \[email protected]\u003e"
        },
        "StopSignal": "SIGTERM"
      },
      "rootfs": {
        "type": "layers",
        "diff_ids": [
          "sha256:2ec5c0a4cb57c0af7c16ceda0b0a87a54f01f027ed33836a5669ca266cafe97a",
          "sha256:938981ec0340b38cf7a3572ac2634bbcc1666bffcec8b393f4a4c1ffb0ab568d",
          "sha256:995f02eaa054bf72f518582c3da77130d460269771405cd695d75b80488a6b88"
        ]
      },
      "history": [
        {
          "created": "2017-12-12T01:44:43.599554271Z",
          "created_by": "/bin/sh -c #(nop) ADD file:f30a8b5b7cdc9ba33a250899308b490baa9f7a9b29d3a85bd16200aa0a28a04a in / "
        },
        {
          "created": "2017-12-12T01:44:43.84546055Z",
          "created_by": "/bin/sh -c #(nop)  CMD [\"bash\"]",
          "empty_layer": true
        },
        {
          "created": "2017-12-12T05:14:40.530307095Z",
          "created_by": "/bin/sh -c #(nop)  LABEL maintainer=NGINX Docker Maintainers \[email protected]\u003e",
          "empty_layer": true
        },
        {
          "created": "2017-12-12T05:14:40.730784142Z",
          "created_by": "/bin/sh -c #(nop)  ENV NGINX_VERSION=1.13.7-1~stretch",
          "empty_layer": true
        },
        {
          "created": "2017-12-12T05:14:40.93841268Z",
          "created_by": "/bin/sh -c #(nop)  ENV NJS_VERSION=1.13.7.0.1.15-1~stretch",
          "empty_layer": true
        },
        {
          "created": "2017-12-12T05:14:59.805295768Z",
          "created_by": "/bin/sh -c set -x \t\u0026\u0026 apt-get update \t\u0026\u0026 apt-get install --no-install-recommends --no-install-suggests -y gnupg1 \t\u0026\u0026 \tNGINX_GPGKEY=573BFD6B3D8FBC641079A6ABABF5BD827BD9BF62; \tfound=''; \tfor server in \t\tha.pool.sks-keyservers.net \t\thkp://keyserver.ubuntu.com:80 \t\thkp://p80.pool.sks-keyservers.net:80 \t\tpgp.mit.edu \t; do \t\techo \"Fetching GPG key $NGINX_GPGKEY from $server\"; \t\tapt-key adv --keyserver \"$server\" --keyserver-options timeout=10 --recv-keys \"$NGINX_GPGKEY\" \u0026\u0026 found=yes \u0026\u0026 break; \tdone; \ttest -z \"$found\" \u0026\u0026 echo \u003e\u00262 \"error: failed to fetch GPG key $NGINX_GPGKEY\" \u0026\u0026 exit 1; \tapt-get remove --purge --auto-remove -y gnupg1 \u0026\u0026 rm -rf /var/lib/apt/lists/* \t\u0026\u0026 dpkgArch=\"$(dpkg --print-architecture)\" \t\u0026\u0026 nginxPackages=\" \t\tnginx=${NGINX_VERSION} \t\tnginx-module-xslt=${NGINX_VERSION} \t\tnginx-module-geoip=${NGINX_VERSION} \t\tnginx-module-image-filter=${NGINX_VERSION} \t\tnginx-module-njs=${NJS_VERSION} \t\" \t\u0026\u0026 case \"$dpkgArch\" in \t\tamd64|i386) \t\t\techo \"deb http://nginx.org/packages/mainline/debian/ stretch nginx\" \u003e\u003e /etc/apt/sources.list \t\t\t\u0026\u0026 apt-get update \t\t\t;; \t\t*) \t\t\techo \"deb-src http://nginx.org/packages/mainline/debian/ stretch nginx\" \u003e\u003e /etc/apt/sources.list \t\t\t\t\t\t\u0026\u0026 tempDir=\"$(mktemp -d)\" \t\t\t\u0026\u0026 chmod 777 \"$tempDir\" \t\t\t\t\t\t\u0026\u0026 savedAptMark=\"$(apt-mark showmanual)\" \t\t\t\t\t\t\u0026\u0026 apt-get update \t\t\t\u0026\u0026 apt-get build-dep -y $nginxPackages \t\t\t\u0026\u0026 ( \t\t\t\tcd \"$tempDir\" \t\t\t\t\u0026\u0026 DEB_BUILD_OPTIONS=\"nocheck parallel=$(nproc)\" \t\t\t\t\tapt-get source --compile $nginxPackages \t\t\t) \t\t\t\t\t\t\u0026\u0026 apt-mark showmanual | xargs apt-mark auto \u003e /dev/null \t\t\t\u0026\u0026 { [ -z \"$savedAptMark\" ] || apt-mark manual $savedAptMark; } \t\t\t\t\t\t\u0026\u0026 ls -lAFh \"$tempDir\" \t\t\t\u0026\u0026 ( cd \"$tempDir\" \u0026\u0026 dpkg-scanpackages . \u003e Packages ) \t\t\t\u0026\u0026 grep '^Package: ' \"$tempDir/Packages\" \t\t\t\u0026\u0026 echo \"deb [ trusted=yes ] file://$tempDir ./\" \u003e /etc/apt/sources.list.d/temp.list \t\t\t\u0026\u0026 apt-get -o Acquire::GzipIndexes=false update \t\t\t;; \tesac \t\t\u0026\u0026 apt-get install --no-install-recommends --no-install-suggests -y \t\t\t\t\t\t$nginxPackages \t\t\t\t\t\tgettext-base \t\u0026\u0026 rm -rf /var/lib/apt/lists/* \t\t\u0026\u0026 if [ -n \"$tempDir\" ]; then \t\tapt-get purge -y --auto-remove \t\t\u0026\u0026 rm -rf \"$tempDir\" /etc/apt/sources.list.d/temp.list; \tfi"
        },
        {
          "created": "2017-12-12T05:15:03.15757127Z",
          "created_by": "/bin/sh -c ln -sf /dev/stdout /var/log/nginx/access.log \t\u0026\u0026 ln -sf /dev/stderr /var/log/nginx/error.log"
        },
        {
          "created": "2017-12-12T05:15:03.379026129Z",
          "created_by": "/bin/sh -c #(nop)  EXPOSE 80/tcp",
          "empty_layer": true
        },
        {
          "created": "2017-12-12T05:15:03.582079995Z",
          "created_by": "/bin/sh -c #(nop)  STOPSIGNAL [SIGTERM]",
          "empty_layer": true
        },
        {
          "created": "2017-12-12T05:15:03.787465224Z",
          "created_by": "/bin/sh -c #(nop)  CMD [\"nginx\" \"-g\" \"daemon off;\"]",
          "empty_layer": true
        }
      ]
    }
  }
}

Signed-off-by: Mike Brown <[email protected]>

mikebrow · 2017-12-12T22:00:51Z

comments addressed.

Random-Liu · 2017-12-12T23:03:39Z

@mikebrow THIS LOOKS COOL!

Random-Liu · 2017-12-12T23:06:22Z

pkg/store/image/image.go

-	Config *imagespec.ImageConfig
+	// ImageSpec is the oci image structure which describes basic information about the image.
+	ImageSpec imagespec.Image
+


Unnecessary empty line. I'll clean this up in my coming cleanup PR.

Random-Liu · 2017-12-12T23:07:32Z

pkg/store/image/image_test.go

 		},
 		{
 			ID:          "sha256:2123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
 			ChainID:     "test-chain-id-2abcd",
 			RepoTags:    []string{"tag-2abcd"},
 			RepoDigests: []string{"digest-2abcd"},
 			Size:        20,
-			Config:      &imagespec.ImageConfig{},


Why removing this? Will add it back in my cleanup PR.

because the config is in the image struct...

Random-Liu · 2017-12-12T23:09:48Z

/lgtm

k8s-ci-robot added cncf-cla: yes size/M labels Dec 4, 2017

mikebrow changed the title ~~adding info map for verbose image status~~ [WIP] adding info map for verbose image status Dec 4, 2017

mikebrow changed the title ~~[WIP] adding info map for verbose image status~~ adding info map for verbose image status Dec 5, 2017

mikebrow requested review from Random-Liu and abhi December 5, 2017 01:02

miaoyq reviewed Dec 5, 2017

View reviewed changes

miaoyq mentioned this pull request Dec 6, 2017

adding info map for verbose pod status #452

Merged

Random-Liu self-assigned this Dec 11, 2017

Random-Liu reviewed Dec 11, 2017

View reviewed changes

adding info map for verbose image status

03ac989

Signed-off-by: Mike Brown <[email protected]>

k8s-ci-robot added size/L and removed size/M labels Dec 12, 2017

mikebrow changed the title ~~adding info map for verbose image status~~ [WIP] adding info map for verbose image status Dec 12, 2017

adds oci image spec to image info placed into imagestore

31223fd

Signed-off-by: Mike Brown <[email protected]>

mikebrow changed the title ~~[WIP] adding info map for verbose image status~~ adding info map for verbose image status Dec 12, 2017

Random-Liu reviewed Dec 12, 2017

View reviewed changes

k8s-ci-robot added the lgtm label Dec 12, 2017

Random-Liu merged commit a9c7237 into containerd:master Dec 12, 2017

Random-Liu mentioned this pull request Dec 13, 2017

Minor cleanup. #493

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding info map for verbose image status #470

adding info map for verbose image status #470

mikebrow commented Dec 4, 2017 •

edited

Loading

mikebrow commented Dec 4, 2017 •

edited

Loading

miaoyq left a comment •

edited

Loading

miaoyq Dec 5, 2017

mikebrow Dec 5, 2017 •

edited

Loading

mikebrow Dec 6, 2017

mikebrow commented Dec 5, 2017 •

edited

Loading

mikebrow commented Dec 6, 2017

miaoyq commented Dec 6, 2017 •

edited

Loading

Random-Liu Dec 11, 2017 •

edited

Loading

mikebrow Dec 11, 2017

Random-Liu Dec 11, 2017

mikebrow Dec 12, 2017

mikebrow commented Dec 12, 2017

mikebrow commented Dec 12, 2017

Random-Liu commented Dec 12, 2017

Random-Liu Dec 12, 2017

mikebrow Dec 12, 2017

Random-Liu Dec 13, 2017

Random-Liu Dec 12, 2017

mikebrow Dec 12, 2017

mikebrow Dec 12, 2017

Random-Liu Dec 13, 2017

Random-Liu commented Dec 12, 2017

adding info map for verbose image status #470

adding info map for verbose image status #470

Conversation

mikebrow commented Dec 4, 2017 • edited Loading

mikebrow commented Dec 4, 2017 • edited Loading

miaoyq left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikebrow Dec 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikebrow commented Dec 5, 2017 • edited Loading

mikebrow commented Dec 6, 2017

miaoyq commented Dec 6, 2017 • edited Loading

Random-Liu Dec 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikebrow commented Dec 12, 2017

mikebrow commented Dec 12, 2017

Random-Liu commented Dec 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Random-Liu commented Dec 12, 2017

mikebrow commented Dec 4, 2017 •

edited

Loading

mikebrow commented Dec 4, 2017 •

edited

Loading

miaoyq left a comment •

edited

Loading

mikebrow Dec 5, 2017 •

edited

Loading

mikebrow commented Dec 5, 2017 •

edited

Loading

miaoyq commented Dec 6, 2017 •

edited

Loading

Random-Liu Dec 11, 2017 •

edited

Loading