Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite timestamps in container/sandbox status. #211

Merged

Conversation

Random-Liu
Copy link
Contributor

Rewrite timestamps in status to make it more user friendly. After this change:

$ crictl inspects 6ecc8a386012a
{
  "status": {
    "id": "6ecc8a386012aa3d8cc41e986263a56c039044b7f9dcceda012cc3fc045e58c9",
    "metadata": {
      "attempt": 0,
      "name": "kube-dns-6c857864fb-z88p2",
      "namespace": "kube-system",
      "uid": "64489eae-d610-11e7-8e8b-42010af00002"
    },
    "state": "SANDBOX_NOTREADY",
    "createdAt": "2017-11-30T20:52:44.926737345Z",
    "network": {
      "ip": "10.88.22.12"
    },
    "linux": {
      "namespaces": {
        "options": {
          "hostIpc": false,
          "hostNetwork": false,
          "hostPid": false
        }
      }
    },
    "labels": {
      "io.kubernetes.pod.name": "kube-dns-6c857864fb-z88p2",
      "io.kubernetes.pod.namespace": "kube-system",
      "io.kubernetes.pod.uid": "64489eae-d610-11e7-8e8b-42010af00002",
      "k8s-app": "kube-dns",
      "pod-template-hash": "2741342096"
    },
    "annotations": {
      "kubernetes.io/config.seen": "2017-11-30T20:52:41.854730835Z",
      "kubernetes.io/config.source": "api",
      "scheduler.alpha.kubernetes.io/critical-pod": ""
    }
  }
}
$  crictl inspect 98ad5842a6284 -q
{
  "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"
  }
}

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

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 5, 2017
@Random-Liu
Copy link
Contributor Author

/cc @mikebrow

Copy link
Contributor

@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.

The code looks good as one solution to resolve the map order issue. But as seen over here:
containerd/cri#475

We don't really want a map or array of verbose info that is not specified with a predefined field name.

Per the solution used above ^ isn't it much better/easier to just deprecate map and add predefined verbose info structs? Even if we go with map we still need predefined map keys so we know what to look for if we are interested in something specific. And if we go that route we could output the fields we want in any sequence we want. So either way, predefined struct, or predefined keys.. we would not need to sort. Thoughts?

if err != nil {
return "", err
}
jsonMap["createdAt"] = time.Unix(0, cs.CreatedAt).Format(time.RFC3339Nano)
Copy link
Contributor

Choose a reason for hiding this comment

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

thumbs up!

@feiskyer
Copy link
Member

feiskyer commented Dec 6, 2017

Per the solution used above ^ isn't it much better/easier to just deprecate map and add predefined verbose info structs? Even if we go with map we still need predefined map keys so we know what to look for if we are interested in something specific. And if we go that route we could output the fields we want in any sequence we want. So either way, predefined struct, or predefined keys.. we would not need to sort.

Predefined struct or keys are other ways for getting a sorted result. But they need to redefine the sandbox/container/image status structs or hack with struct fields, which seems also not graceful.

I'm ok with current implementation. Let's get this in and iterate with better solutions in the future (if there are).

@feiskyer feiskyer added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 6, 2017
@feiskyer feiskyer merged commit 23a53d2 into kubernetes-sigs:master Dec 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants