adding info map for verbose pod status #452

mikebrow · 2017-12-01T01:02:44Z

Commit 1: adds verbose info to pod status request
So far the additional info provided is the verbose "container" status.

There's a lot of good stuff already in the pod status response struct.

Commit 1: also adds sandboxID to the container status to allow for back tracking from the container to the pod sandbox.

Commit 2: fixes the pid reported by container status.

Signed-off-by: Mike Brown brownwm@us.ibm.com

mikebrow · 2017-12-01T16:47:21Z

verbose sand box inspect output:

mike@mike-VirtualBox:~/go/src/github.com/kubernetes-incubator/cri-containerd$ sudo crictl --runtime-endpoint /var/run/cri-containerd.sock inspects f43264caed6a518a56f6a012babd976837827a9cbcc4c3b6a3d44577ca1935c1 
{
  "status": {
    "id": "f43264caed6a518a56f6a012babd976837827a9cbcc4c3b6a3d44577ca1935c1",
    "metadata": {
      "name": "nginx-sandbox",
      "uid": "hdishd83djaidwnduwk28bcsb",
      "namespace": "default",
      "attempt": 1
    },
    "state": "SANDBOX_READY",
    "createdAt": "1511994755202029525",
    "network": {
      "ip": "10.88.6.31"
    },
    "linux": {
      "namespaces": {
        "options": {
          "hostNetwork": false,
          "hostPid": false,
          "hostIpc": false
        }
      }
    }
  },
  "sandboxContainerdPID": 18997,
  "sandboxContainerState": "running",
  "runtimeSpec": {
    "ociVersion": "1.0.0",
    "process": {
      "user": {
        "uid": 0,
        "gid": 0
      },
      "args": [
        "/pause"
      ],
      "env": [
        "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
      ],
      "cwd": "/",
      "capabilities": {
        "bounding": [
          "CAP_CHOWN",
          "CAP_DAC_OVERRIDE",
          "CAP_FSETID",
          "CAP_FOWNER",
          "CAP_MKNOD",
          "CAP_NET_RAW",
          "CAP_SETGID",
          "CAP_SETUID",
          "CAP_SETFCAP",
          "CAP_SETPCAP",
          "CAP_NET_BIND_SERVICE",
          "CAP_SYS_CHROOT",
          "CAP_KILL",
          "CAP_AUDIT_WRITE"
        ],
        "effective": [
          "CAP_CHOWN",
          "CAP_DAC_OVERRIDE",
          "CAP_FSETID",
          "CAP_FOWNER",
          "CAP_MKNOD",
          "CAP_NET_RAW",
          "CAP_SETGID",
          "CAP_SETUID",
          "CAP_SETFCAP",
          "CAP_SETPCAP",
          "CAP_NET_BIND_SERVICE",
          "CAP_SYS_CHROOT",
          "CAP_KILL",
          "CAP_AUDIT_WRITE"
        ],
        "inheritable": [
          "CAP_CHOWN",
          "CAP_DAC_OVERRIDE",
          "CAP_FSETID",
          "CAP_FOWNER",
          "CAP_MKNOD",
          "CAP_NET_RAW",
          "CAP_SETGID",
          "CAP_SETUID",
          "CAP_SETFCAP",
          "CAP_SETPCAP",
          "CAP_NET_BIND_SERVICE",
          "CAP_SYS_CHROOT",
          "CAP_KILL",
          "CAP_AUDIT_WRITE"
        ],
        "permitted": [
          "CAP_CHOWN",
          "CAP_DAC_OVERRIDE",
          "CAP_FSETID",
          "CAP_FOWNER",
          "CAP_MKNOD",
          "CAP_NET_RAW",
          "CAP_SETGID",
          "CAP_SETUID",
          "CAP_SETFCAP",
          "CAP_SETPCAP",
          "CAP_NET_BIND_SERVICE",
          "CAP_SYS_CHROOT",
          "CAP_KILL",
          "CAP_AUDIT_WRITE"
        ]
      },
      "rlimits": [
        {
          "type": "RLIMIT_NOFILE",
          "hard": 1024,
          "soft": 1024
        }
      ],
      "noNewPrivileges": true,
      "oomScoreAdj": -998
    },
    "root": {
      "path": "rootfs",
      "readonly": true
    },
    "mounts": [
      {
        "destination": "/proc",
        "type": "proc",
        "source": "proc"
      },
      {
        "destination": "/dev",
        "type": "tmpfs",
        "source": "tmpfs",
        "options": [
          "nosuid",
          "strictatime",
          "mode=755",
          "size=65536k"
        ]
      },
      {
        "destination": "/dev/pts",
        "type": "devpts",
        "source": "devpts",
        "options": [
          "nosuid",
          "noexec",
          "newinstance",
          "ptmxmode=0666",
          "mode=0620",
          "gid=5"
        ]
      },
      {
        "destination": "/dev/shm",
        "type": "tmpfs",
        "source": "shm",
        "options": [
          "nosuid",
          "noexec",
          "nodev",
          "mode=1777",
          "size=65536k"
        ]
      },
      {
        "destination": "/dev/mqueue",
        "type": "mqueue",
        "source": "mqueue",
        "options": [
          "nosuid",
          "noexec",
          "nodev"
        ]
      },
      {
        "destination": "/sys",
        "type": "sysfs",
        "source": "sysfs",
        "options": [
          "nosuid",
          "noexec",
          "nodev",
          "ro"
        ]
      }
    ],
    "linux": {
      "resources": {
        "devices": [
          {
            "allow": false,
            "access": "rwm"
          }
        ],
        "cpu": {
          "shares": 2
        }
      },
      "cgroupsPath": "/k8s.io/f43264caed6a518a56f6a012babd976837827a9cbcc4c3b6a3d44577ca1935c1",
      "namespaces": [
        {
          "type": "pid"
        },
        {
          "type": "ipc"
        },
        {
          "type": "uts"
        },
        {
          "type": "mount"
        },
        {
          "type": "network",
          "path": "/var/run/netns/cni-7ff29c8a-1981-0119-2535-f258d090bc5a"
        }
      ],
      "maskedPaths": [
        "/proc/kcore",
        "/proc/latency_stats",
        "/proc/timer_list",
        "/proc/timer_stats",
        "/proc/sched_debug",
        "/sys/firmware",
        "/proc/scsi"
      ],
      "readonlyPaths": [
        "/proc/asound",
        "/proc/bus",
        "/proc/fs",
        "/proc/irq",
        "/proc/sys",
        "/proc/sysrq-trigger"
      ]
    }
  },
  "snapshotKey": "f43264caed6a518a56f6a012babd976837827a9cbcc4c3b6a3d44577ca1935c1",
  "snapshotter": "overlayfs"
}

mikebrow · 2017-12-01T16:57:46Z

Can be tested via the changes here:
kubernetes-sigs/cri-tools#206

miaoyq · 2017-12-06T01:17:54Z

Build error:

32.87s$ make binaries
go build -o _output/cri-containerd \
		-tags 'seccomp apparmor' \
		-ldflags '-X github.com/kubernetes-incubator/cri-containerd/pkg/version.CRIContainerdVersion=1.0.0-alpha.1-141-gd083246' \
		-gcflags '' \
		github.com/kubernetes-incubator/cri-containerd/cmd/cri-containerd
# github.com/kubernetes-incubator/cri-containerd/pkg/server
pkg/server/sandbox_status.go:98:33: undefined: marshallToString
pkg/server/sandbox_status.go:99:34: undefined: marshallToString
pkg/server/sandbox_status.go:103:25: undefined: marshallToString
pkg/server/sandbox_status.go:110:25: undefined: marshallToString
pkg/server/sandbox_status.go:111:25: undefined: marshallToString

Signed-off-by: Mike Brown <brownwm@us.ibm.com>

mikebrow · 2017-12-06T01:43:39Z

@miaoyq yeah was in the middle of rebasing and correcting... pushed the merge first :-)

All fixes in. Updated to reflect the changes made in the other debug PRs. Should've tagged it WIP.. ready for review now!

mikebrow · 2017-12-06T01:44:15Z

Updated example output:

mike@mike-VirtualBox:~/go/src/github.com/kubernetes-incubator/test$ sudo crictl --runtime-endpoint /var/run/cri-containerd.sock inspects 0f8c455422a0546eb425a4fa305d5b7a0072109dff38fb94213a468ec27e680e
{
  "status": {
    "id": "0f8c455422a0546eb425a4fa305d5b7a0072109dff38fb94213a468ec27e680e",
    "metadata": {
      "name": "nginx-sandbox",
      "uid": "hdishd83djaidwnduwk28bcsb",
      "namespace": "default",
      "attempt": 1
    },
    "state": "SANDBOX_READY",
    "createdAt": "1512524016760395386",
    "network": {
      "ip": "10.88.6.32"
    },
    "linux": {
      "namespaces": {
        "options": {
          "hostNetwork": false,
          "hostPid": false,
          "hostIpc": false
        }
      }
    }
  },
  "info": {
    "pid": 9475,
    "state": "running",
    "snapshotKey": "0f8c455422a0546eb425a4fa305d5b7a0072109dff38fb94213a468ec27e680e",
    "snapshotter": "overlayfs",
    "runtimeSpec": {
      "ociVersion": "1.0.0",
      "process": {
        "user": {
          "uid": 0,
          "gid": 0
        },
        "args": [
          "/pause"
        ],
        "env": [
          "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
        ],
        "cwd": "/",
        "capabilities": {
          "bounding": [
            "CAP_CHOWN",
            "CAP_DAC_OVERRIDE",
            "CAP_FSETID",
            "CAP_FOWNER",
            "CAP_MKNOD",
            "CAP_NET_RAW",
            "CAP_SETGID",
            "CAP_SETUID",
            "CAP_SETFCAP",
            "CAP_SETPCAP",
            "CAP_NET_BIND_SERVICE",
            "CAP_SYS_CHROOT",
            "CAP_KILL",
            "CAP_AUDIT_WRITE"
          ],
          "effective": [
            "CAP_CHOWN",
            "CAP_DAC_OVERRIDE",
            "CAP_FSETID",
            "CAP_FOWNER",
            "CAP_MKNOD",
            "CAP_NET_RAW",
            "CAP_SETGID",
            "CAP_SETUID",
            "CAP_SETFCAP",
            "CAP_SETPCAP",
            "CAP_NET_BIND_SERVICE",
            "CAP_SYS_CHROOT",
            "CAP_KILL",
            "CAP_AUDIT_WRITE"
          ],
          "inheritable": [
            "CAP_CHOWN",
            "CAP_DAC_OVERRIDE",
            "CAP_FSETID",
            "CAP_FOWNER",
            "CAP_MKNOD",
            "CAP_NET_RAW",
            "CAP_SETGID",
            "CAP_SETUID",
            "CAP_SETFCAP",
            "CAP_SETPCAP",
            "CAP_NET_BIND_SERVICE",
            "CAP_SYS_CHROOT",
            "CAP_KILL",
            "CAP_AUDIT_WRITE"
          ],
          "permitted": [
            "CAP_CHOWN",
            "CAP_DAC_OVERRIDE",
            "CAP_FSETID",
            "CAP_FOWNER",
            "CAP_MKNOD",
            "CAP_NET_RAW",
            "CAP_SETGID",
            "CAP_SETUID",
            "CAP_SETFCAP",
            "CAP_SETPCAP",
            "CAP_NET_BIND_SERVICE",
            "CAP_SYS_CHROOT",
            "CAP_KILL",
            "CAP_AUDIT_WRITE"
          ]
        },
        "rlimits": [
          {
            "type": "RLIMIT_NOFILE",
            "hard": 1024,
            "soft": 1024
          }
        ],
        "noNewPrivileges": true,
        "oomScoreAdj": -998
      },
      "root": {
        "path": "rootfs",
        "readonly": true
      },
      "mounts": [
        {
          "destination": "/proc",
          "type": "proc",
          "source": "proc"
        },
        {
          "destination": "/dev",
          "type": "tmpfs",
          "source": "tmpfs",
          "options": [
            "nosuid",
            "strictatime",
            "mode=755",
            "size=65536k"
          ]
        },
        {
          "destination": "/dev/pts",
          "type": "devpts",
          "source": "devpts",
          "options": [
            "nosuid",
            "noexec",
            "newinstance",
            "ptmxmode=0666",
            "mode=0620",
            "gid=5"
          ]
        },
        {
          "destination": "/dev/shm",
          "type": "tmpfs",
          "source": "shm",
          "options": [
            "nosuid",
            "noexec",
            "nodev",
            "mode=1777",
            "size=65536k"
          ]
        },
        {
          "destination": "/dev/mqueue",
          "type": "mqueue",
          "source": "mqueue",
          "options": [
            "nosuid",
            "noexec",
            "nodev"
          ]
        },
        {
          "destination": "/sys",
          "type": "sysfs",
          "source": "sysfs",
          "options": [
            "nosuid",
            "noexec",
            "nodev",
            "ro"
          ]
        }
      ],
      "linux": {
        "resources": {
          "devices": [
            {
              "allow": false,
              "access": "rwm"
            }
          ],
          "cpu": {
            "shares": 2
          }
        },
        "cgroupsPath": "/k8s.io/0f8c455422a0546eb425a4fa305d5b7a0072109dff38fb94213a468ec27e680e",
        "namespaces": [
          {
            "type": "pid"
          },
          {
            "type": "ipc"
          },
          {
            "type": "uts"
          },
          {
            "type": "mount"
          },
          {
            "type": "network",
            "path": "/var/run/netns/cni-3f1dc682-cb91-29e8-e91e-38b30e4bf18f"
          }
        ],
        "maskedPaths": [
          "/proc/kcore",
          "/proc/latency_stats",
          "/proc/timer_list",
          "/proc/timer_stats",
          "/proc/sched_debug",
          "/sys/firmware",
          "/proc/scsi"
        ],
        "readonlyPaths": [
          "/proc/asound",
          "/proc/bus",
          "/proc/fs",
          "/proc/irq",
          "/proc/sys",
          "/proc/sysrq-trigger"
        ]
      }
    }
  }
}

miaoyq · 2017-12-06T03:19:01Z

pkg/server/sandbox_status.go

-	return &runtime.PodSandboxStatusResponse{Status: status}, nil
+	info, err := toCRISandboxContainerInfo(ctx, sandbox.Container, r.GetVerbose())
+	if err != nil {
+		return nil, fmt.Errorf("failed to get verbose sandbox container info: %v", err)


Could we log the error and Include it into info instead of return directly?
Even if the error is not nil, we should return status information and indicate the cause of the error in info.
Also #470, :)

docker do it like this:

# docker inspect ec [ { "Id": "ec864d7a574eef3958a142944d37f764617ea8d2c994d0012a6e45ed31ab0a5e", "Created": "2017-12-06T03:43:34.739382517Z", "Path": "top-dfsaf", "Args": [], "State": { "Status": "created", "Running": false, "Paused": false, "Restarting": false, "OOMKilled": false, "Dead": false, "Pid": 0, "ExitCode": 127, // "Error" indicates the reason of the error "Error": "oci runtime error: container_linux.go:247: starting container process caused \"exec: \\\"top-dfsaf\\\": executable file not found in $PATH\"\n", "StartedAt": "0001-01-01T00:00:00Z", "FinishedAt": "0001-01-01T00:00:00Z" }, ... ...

We can include this in info, wdyt?

Could we log the error and Include it into info instead of return directly?
Even if the error is not nil, we should return status information and indicate the cause of the error in info.

Currently there is no way to include the error message, but I agree we should not fail in most cases. We should do something similar with https://github.com/kubernetes-incubator/cri-containerd/pull/475/files.

docker do it like this

We've included most of the information for container status. We just don't save information for sandbox container. :)

yeah probably don't need it for the sandbox pause container... but will need to comb over the inspect for containers to make sure we're not missing anything useful.

Random-Liu · 2017-12-06T04:09:57Z

pkg/server/sandbox_status.go

+		return nil, nil
+	}
+
+	task, err := container.Task(ctx, nil)


We've got task status in PodSandboxStatus, we should pass it in.

Random-Liu · 2017-12-06T04:10:47Z

pkg/server/sandbox_status.go

+	if err == nil {
+		info["info"] = string(m)
+	} else {
+		glog.Errorf("failed to marshal info %v: %v", si, err)


I still think fail to marshal is a logic issue, we should return error and fail loudly instead of hiding the failure.

depends who's calling and what they can do with the error. I think the important thing is to make sure the errors are displayed and since it's extra info I didn't want this extra verbose info to be the reason for someone's cluster failing... So I'm torn on which way to go.

Random-Liu · 2017-12-06T04:11:07Z

pkg/server/sandbox_status.go

+		Pid:         pid,
+		State:       string(status.Status),
+		SnapshotKey: snapshotkey,
+		Snapshotter: snapshotter,


We should include sandbox config, network namespace.

agree.. was tired when I did the sandbox one

Random-Liu · 2017-12-06T04:12:43Z

pkg/server/sandbox_status.go

-	return &runtime.PodSandboxStatusResponse{Status: status}, nil
+	info, err := toCRISandboxContainerInfo(ctx, sandbox.Container, r.GetVerbose())
+	if err != nil {
+		return nil, fmt.Errorf("failed to get verbose sandbox container info: %v", err)


Could we log the error and Include it into info instead of return directly?
Even if the error is not nil, we should return status information and indicate the cause of the error in info.

Currently there is no way to include the error message, but I agree we should not fail in most cases. We should do something similar with https://github.com/kubernetes-incubator/cri-containerd/pull/475/files.

Random-Liu · 2017-12-06T04:14:29Z

pkg/server/sandbox_status.go

-	return &runtime.PodSandboxStatusResponse{Status: status}, nil
+	info, err := toCRISandboxContainerInfo(ctx, sandbox.Container, r.GetVerbose())
+	if err != nil {
+		return nil, fmt.Errorf("failed to get verbose sandbox container info: %v", err)


docker do it like this

We've included most of the information for container status. We just don't save information for sandbox container. :)

Random-Liu · 2017-12-06T04:15:19Z

/lgtm

LGTM overall. I'll merge this and send another PR to address the comments I mentioned above.

k8s-ci-robot added cncf-cla: yes size/S size/M and removed size/S labels Dec 1, 2017

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

mikebrow requested a review from Random-Liu December 1, 2017 16:48

mikebrow assigned abhi Dec 1, 2017

mikebrow mentioned this pull request Dec 1, 2017

Implement CRI Debug API #359

Closed

4 tasks

Random-Liu self-assigned this Dec 1, 2017

containerd deleted a comment from k8s-ci-robot Dec 1, 2017

Random-Liu added this to the v1.0.0-beta.0 milestone Dec 4, 2017

adding info map for verbose pod status

220411b

Signed-off-by: Mike Brown <brownwm@us.ibm.com>

mikebrow requested a review from miaoyq December 6, 2017 01:52

miaoyq reviewed Dec 6, 2017

View reviewed changes

Random-Liu reviewed Dec 6, 2017

View reviewed changes

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

Random-Liu merged commit 77a8ccb into containerd:master Dec 6, 2017

abel-von mentioned this pull request Feb 27, 2023

PodSandboxStatus may be blocked if one of the shim Status ttrpc call blocks containerd/containerd#8167

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding info map for verbose pod status #452

adding info map for verbose pod status #452

mikebrow commented Dec 1, 2017 •

edited

Loading

mikebrow commented Dec 1, 2017

mikebrow commented Dec 1, 2017

miaoyq commented Dec 6, 2017

mikebrow commented Dec 6, 2017 •

edited

Loading

mikebrow commented Dec 6, 2017

miaoyq Dec 6, 2017 •

edited

Loading

miaoyq Dec 6, 2017 •

edited

Loading

Random-Liu Dec 6, 2017

Random-Liu Dec 6, 2017

mikebrow Dec 6, 2017

Random-Liu Dec 6, 2017

Random-Liu Dec 6, 2017

mikebrow Dec 6, 2017 •

edited

Loading

Random-Liu Dec 6, 2017

mikebrow Dec 6, 2017

Random-Liu Dec 6, 2017

Random-Liu Dec 6, 2017

Random-Liu commented Dec 6, 2017

adding info map for verbose pod status #452

adding info map for verbose pod status #452

Conversation

mikebrow commented Dec 1, 2017 • edited Loading

mikebrow commented Dec 1, 2017

mikebrow commented Dec 1, 2017

miaoyq commented Dec 6, 2017

mikebrow commented Dec 6, 2017 • edited Loading

mikebrow commented Dec 6, 2017

miaoyq Dec 6, 2017 • edited Loading

Choose a reason for hiding this comment

miaoyq Dec 6, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikebrow Dec 6, 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

Choose a reason for hiding this comment

Random-Liu commented Dec 6, 2017

mikebrow commented Dec 1, 2017 •

edited

Loading

mikebrow commented Dec 6, 2017 •

edited

Loading

miaoyq Dec 6, 2017 •

edited

Loading

miaoyq Dec 6, 2017 •

edited

Loading

mikebrow Dec 6, 2017 •

edited

Loading