Skip to content
This repository was archived by the owner on Mar 9, 2022. It is now read-only.

Use one big info struct before we change info to an array. #475

Merged

Conversation

Random-Liu
Copy link
Member

Related to #472.
Currently map doesn't preserve order. Use a big struct to enforce the field order.

@mikebrow PTAL. If this makes sense, we should apply the same behavior to the other status functions.

Signed-off-by: Lantao Liu [email protected]

@Random-Liu
Copy link
Member Author

Example output:

$ crictl inspect 98ad5842a6284
{
  "status": {
    "id": "98ad5842a62844f62d4468c52dbfcc081a08ba25c95c842a967de7058f60dd24",
    "metadata": {
      "attempt": 0,
      "name": "nginx"
    },
    "state": "CONTAINER_EXITED",
    "createdAt": "2017-11-30T21:57:26.038813532Z",
    "startedAt": "2017-11-30T21:57:26.235406355Z",
    "finishedAt": "2017-12-01T20:08:56.523559866Z",
    "exitCode": 255,
    "image": {
      "image": "gcr.io/google-containers/nginx-slim-amd64:0.20"
    },
    "imageRef": "gcr.io/google-containers/nginx-slim-amd64@sha256:6654db6d4028756062edac466454ee5c9cf9b20ef79e35a81e3c840031eb1e2b",
    "reason": "Unknown",
    "message": "",
    "labels": {
      "io.kubernetes.container.name": "nginx",
      "io.kubernetes.pod.name": "nginx",
      "io.kubernetes.pod.namespace": "e2e-tests-kubectl-2jdtb",
      "io.kubernetes.pod.uid": "6ffc887b-d619-11e7-8e8b-42010af00002"
    },
    "annotations": {
      "io.kubernetes.container.hash": "cfada9cb",
      "io.kubernetes.container.ports": "[{\"containerPort\":80,\"protocol\":\"TCP\"}]",
      "io.kubernetes.container.restartCount": "0",
      "io.kubernetes.container.terminationMessagePath": "/dev/termination-log",
      "io.kubernetes.container.terminationMessagePolicy": "File",
      "io.kubernetes.pod.terminationGracePeriod": "30"
    },
    "mounts": [
      {
        "containerPath": "/var/run/secrets/kubernetes.io/serviceaccount",
        "hostPath": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/volumes/kubernetes.io~secret/default-token-4ztmf",
        "propagation": "PROPAGATION_PRIVATE",
        "readonly": true,
        "selinuxRelabel": false
      },
      {
        "containerPath": "/etc/hosts",
        "hostPath": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/etc-hosts",
        "propagation": "PROPAGATION_PRIVATE",
        "readonly": false,
        "selinuxRelabel": false
      },
      {
        "containerPath": "/dev/termination-log",
        "hostPath": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/containers/nginx/5f93e895",
        "propagation": "PROPAGATION_PRIVATE",
        "readonly": false,
        "selinuxRelabel": false
      }
    ],
    "logPath": "/var/log/pods/6ffc887b-d619-11e7-8e8b-42010af00002/nginx_0.log"
  },
  "info": {
    "sandboxID": "4ed90697eba7b9e9661599b450c5ed5e28219471e1d52b2d3f1d18495a18bd70",
    "pid": 3664,
    "removing": false,
    "snapshotKey": "98ad5842a62844f62d4468c52dbfcc081a08ba25c95c842a967de7058f60dd24",
    "snapshotter": "overlayfs",
    "config": {
      "metadata": {
        "name": "nginx"
      },
      "image": {
        "image": "gcr.io/google-containers/nginx-slim-amd64@sha256:6654db6d4028756062edac466454ee5c9cf9b20ef79e35a81e3c840031eb1e2b"
      },
      "envs": [
        {
          "key": "KUBERNETES_PORT",
          "value": "tcp://10.0.0.1:443"
        },
        {
          "key": "KUBERNETES_PORT_443_TCP",
          "value": "tcp://10.0.0.1:443"
        },
        {
          "key": "KUBERNETES_PORT_443_TCP_PROTO",
          "value": "tcp"
        },
        {
          "key": "KUBERNETES_PORT_443_TCP_PORT",
          "value": "443"
        },
        {
          "key": "KUBERNETES_PORT_443_TCP_ADDR",
          "value": "10.0.0.1"
        },
        {
          "key": "KUBERNETES_SERVICE_HOST",
          "value": "10.0.0.1"
        },
        {
          "key": "KUBERNETES_SERVICE_PORT",
          "value": "443"
        },
        {
          "key": "KUBERNETES_SERVICE_PORT_HTTPS",
          "value": "443"
        }
      ],
      "mounts": [
        {
          "container_path": "/var/run/secrets/kubernetes.io/serviceaccount",
          "host_path": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/volumes/kubernetes.io~secret/default-token-4ztmf",
          "readonly": true
        },
        {
          "container_path": "/etc/hosts",
          "host_path": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/etc-hosts"
        },
        {
          "container_path": "/dev/termination-log",
          "host_path": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/containers/nginx/5f93e895"
        }
      ],
      "labels": {
        "io.kubernetes.container.name": "nginx",
        "io.kubernetes.pod.name": "nginx",
        "io.kubernetes.pod.namespace": "e2e-tests-kubectl-2jdtb",
        "io.kubernetes.pod.uid": "6ffc887b-d619-11e7-8e8b-42010af00002"
      },
      "annotations": {
        "io.kubernetes.container.hash": "cfada9cb",
        "io.kubernetes.container.ports": "[{\"containerPort\":80,\"protocol\":\"TCP\"}]",
        "io.kubernetes.container.restartCount": "0",
        "io.kubernetes.container.terminationMessagePath": "/dev/termination-log",
        "io.kubernetes.container.terminationMessagePolicy": "File",
        "io.kubernetes.pod.terminationGracePeriod": "30"
      },
      "log_path": "nginx_0.log",
      "linux": {
        "resources": {
          "cpu_shares": 2,
          "oom_score_adj": 1000
        },
        "security_context": {
          "namespace_options": {},
          "run_as_user": {}
        }
      }
    },
    "runtimeSpec": {
      "ociVersion": "1.0.0",
      "process": {
        "user": {
          "uid": 0,
          "gid": 0
        },
        "args": [
          "nginx",
          "-g",
          "daemon off;"
        ],
        "env": [
          "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
          "KUBERNETES_PORT=tcp://10.0.0.1:443",
          "KUBERNETES_PORT_443_TCP=tcp://10.0.0.1:443",
          "KUBERNETES_PORT_443_TCP_PROTO=tcp",
          "KUBERNETES_PORT_443_TCP_PORT=443",
          "KUBERNETES_PORT_443_TCP_ADDR=10.0.0.1",
          "KUBERNETES_SERVICE_HOST=10.0.0.1",
          "KUBERNETES_SERVICE_PORT=443",
          "KUBERNETES_SERVICE_PORT_HTTPS=443"
        ],
        "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
          }
        ],
        "apparmorProfile": "cri-containerd.apparmor.d",
        "oomScoreAdj": 1000
      },
      "root": {
        "path": "rootfs"
      },
      "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"
          ]
        },
        {
          "destination": "/sys/fs/cgroup",
          "type": "cgroup",
          "source": "cgroup",
          "options": [
            "nosuid",
            "noexec",
            "nodev",
            "relatime",
            "ro"
          ]
        },
        {
          "destination": "/etc/resolv.conf",
          "type": "bind",
          "source": "/var/lib/cri-containerd/sandboxes/4ed90697eba7b9e9661599b450c5ed5e28219471e1d52b2d3f1d18495a18bd70/resolv.conf",
          "options": [
            "rbind",
            "rprivate",
            "rw"
          ]
        },
        {
          "destination": "/dev/shm",
          "type": "bind",
          "source": "/var/lib/cri-containerd/sandboxes/4ed90697eba7b9e9661599b450c5ed5e28219471e1d52b2d3f1d18495a18bd70/shm",
          "options": [
            "rbind",
            "rprivate",
            "rw"
          ]
        },
        {
          "destination": "/var/run/secrets/kubernetes.io/serviceaccount",
          "type": "bind",
          "source": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/volumes/kubernetes.io~secret/default-token-4ztmf",
          "options": [
            "rbind",
            "rprivate",
            "ro"
          ]
        },
        {
          "destination": "/etc/hosts",
          "type": "bind",
          "source": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/etc-hosts",
          "options": [
            "rbind",
            "rprivate",
            "rw"
          ]
        },
        {
          "destination": "/dev/termination-log",
          "type": "bind",
          "source": "/var/lib/kubelet/pods/6ffc887b-d619-11e7-8e8b-42010af00002/containers/nginx/5f93e895",
          "options": [
            "rbind",
            "rprivate",
            "rw"
          ]
        }
      ],
      "linux": {
        "resources": {
          "devices": [
            {
              "allow": false,
              "access": "rwm"
            }
          ],
          "memory": {
            "limit": 0
          },
          "cpu": {
            "shares": 2,
            "quota": 0,
            "period": 0
          }
        },
        "cgroupsPath": "/kubepods/besteffort/pod6ffc887b-d619-11e7-8e8b-42010af00002/98ad5842a62844f62d4468c52dbfcc081a08ba25c95c842a967de7058f60dd24",
        "namespaces": [
          {
            "type": "pid"
          },
          {
            "type": "ipc",
            "path": "/proc/3346/ns/ipc"
          },
          {
            "type": "uts",
            "path": "/proc/3346/ns/uts"
          },
          {
            "type": "mount"
          },
          {
            "type": "network",
            "path": "/proc/3346/ns/net"
          }
        ],
        "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"
        ]
      }
    }
  }
}

@Random-Liu Random-Liu force-pushed the order-container-status-fields branch from d9eaee8 to d015545 Compare December 5, 2017 18:26
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/LGTM

See small nit for todo or issue is fine.

@@ -94,44 +98,51 @@ func toCRIContainerStatus(container containerstore.Container, spec *runtime.Imag
}
}

type containerInfo struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the todo here for all this stuff to be specified in CRI (containerInfo, "info") etc.

if err == nil {
info["runtimeSpec"] = marshallToString(oldSpec)
} else {
info["runtimeSpec"] = err.Error()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly failing (completely) when debug info is not available limits the ability to debug.. and this is supposed to be verbose for debug purposes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, now we only have one big struct now, there is no where to put the error message. :(
I could log the error and return nil.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@mikebrow
Copy link
Member

mikebrow commented Dec 5, 2017

@Random-Liu ... This exercise shows that we don't want a map or array of verbose info that is not specified with a predefined field name.

Instead of moving from map to array.. let's move to adding these info structs (omit empty) to the CRI api and loose the map or flag the map deprecated.

@Random-Liu
Copy link
Member Author

Random-Liu commented Dec 5, 2017

@mikebrow We can't add info struct or these constant keys into CRI. The reason is that CRI doesn't know what the underlying container runtime is, different container runtimes may want to provide different debug information.
E.g. runtimeSpec won't make sense to non-OCI container runtime, pid won't make sense to VM etc.

That's why we have to use a generic type for those info.

@Random-Liu Random-Liu force-pushed the order-container-status-fields branch from d015545 to 85b943e Compare December 5, 2017 21:37
@mikebrow
Copy link
Member

mikebrow commented Dec 5, 2017

@mikebrow We can add info struct or these constant keys into CRI. The reason is that CRI doesn't know what the underlying container runtime is, different container runtimes may want to provide different debug information.
E.g. runtimeSpec won't make sense to non-OCI container runtime, pid won't make sense to VM etc.

That's why we have to use a generic type for those info.

Yes, and they can leave those nil. Or we could make a OCIInfo Struct and a VMInfo Struct.

Also we could just specify the key names (strings) and then output them in the order of our choosing. They could just PR additional CRI Info (OCI) or (VM) fields.

We could output in sequence the fields we know about (while emptying those from the map) then output any remaining fields.

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/LGTM

@Random-Liu
Copy link
Member Author

Random-Liu commented Dec 5, 2017

Yes, and they can leave those nil. Or we could make a OCIInfo Struct and a VMInfo Struct.

Also we could just specify the key names (strings) and then output them in the order of our choosing. They could just PR additional CRI Info (OCI) or (VM) fields.

We could output in sequence the fields we know about (while emptying those from the map) then output any remaining fields.

@mikebrow Sorry, I meant we can't. Typo...

I doubt we want to complicate CRI just for those additional debug information. It's a overhead to maintain an API (backward compatibility, versioning etc.), I prefer not to complicate CRI just for debugging purpose. A list of arbitrary string seems still a better option.

@Random-Liu Random-Liu merged commit 11eb24c into containerd:master Dec 5, 2017
@Random-Liu Random-Liu deleted the order-container-status-fields branch December 5, 2017 22:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants