Upload container runtime log to sd/es. #59103

Random-Liu · 2018-01-31T09:22:17Z

I've verified this in my environment. My stackdriver has an extra container-runtime entry for node log, and it collects container runtime daemon log correctly.

@yujuhong @feiskyer @crassirostris @piosz
@kubernetes/sig-node-pr-reviews @kubernetes/sig-instrumentation-pr-reviews
Signed-off-by: Lantao Liu lantaol@google.com

Release note:

Container runtime daemon (e.g. dockerd) logs in GCE cluster will be uploaded to stackdriver and elasticsearch with tag `container-runtime`

Random-Liu · 2018-01-31T18:38:39Z

@crassirostris Please note that, with this there will be both docker tag and container-runtime tag in GKE. And for docker as container runtime, their content will be the same. Is this acceptable?

Random-Liu · 2018-01-31T18:41:43Z

/test pull-kubernetes-unit

yujuhong · 2018-02-01T18:03:43Z

/cc @igorpeshansky for input on how tags are consumed in stackdriver.

coffeepac · 2018-02-02T18:46:03Z

@Random-Liu this should rev the version of the fluentd-es addon container image. need to update:

cluster/addons/fluentd-elasticsearch/fluentd-es-image/Makefile#L19
cluster/addons/fluentd-elasticsearch/fluentd-es-ds.yaml#L51 (and four other locations in this file)

current value is v2.0.4, I believe that v2.0.5 is appropriate.

igorpeshansky · 2018-02-03T03:32:26Z

fluentd-gcp actually expects the tag to be in a certain format and parses it to extract container identification. So you can't just use an arbitrary tag -- the logs will not be attributed to the correct container.

yujuhong · 2018-02-06T23:08:18Z

fluentd-gcp actually expects the tag to be in a certain format and parses it to extract container identification. So you can't just use an arbitrary tag -- the logs will not be attributed to the correct container.

@igorpeshansky the tag is added for the container runtime (e.g., docker daemon) log itself, which does not map to any "container". Does the same rule still apply?

igorpeshansky · 2018-02-06T23:41:29Z

Well, the question is which resource in Stackdriver the log is going to be associated with. By default, the logging agent expects the tag to be in a certain format to extract the resource information -- if the tag doesn't match, it won't be able to infer the resource, and there is currently no easy way to supply one directly to the agent.

Random-Liu · 2018-02-06T23:49:03Z

current value is v2.0.4, I believe that v2.0.5 is appropriate.

@coffeepac Thanks! I'll do that if people are ok with this PR. :)

By default, the logging agent expects the tag to be in a certain format to extract the resource information

@igorpeshansky This tag container-runtime is for container runtime daemon log, e.g. docker log. It is not for container log. The certain format you linked seems to be tag for container log?

yujuhong · 2018-02-06T23:53:14Z

@igorpeshansky This tag container-runtime is for container runtime daemon log, e.g. docker log. It is not for container log. The certain format you linked seems to be tag for container log?

@igorpeshansky I also didn't find anything special usage of the "docker" tag in the files you linked. If that's the case, does fluentd simply pass the tag to stackdriver?

igorpeshansky · 2018-02-07T00:02:11Z

Right, in that case fluentd will default to the instance resource and pass the tag to Stackdriver. There will be no association with the Kubernetes node or any other Kubernetes entity.

Random-Liu · 2018-02-07T00:12:40Z

Right, in that case fluentd will default to the instance resource and pass the tag to Stackdriver. There will be no association with the Kubernetes node or any other Kubernetes entity.

Can I understand this as for docker daemon log, either docker tag or container-runtime tag, it doesn't make difference to stackdriver?

Do we assume that there is a docker log tag in stackdriver? If not, it seems fine to gradually switch to container-runtime tag. :)

yujuhong · 2018-02-07T00:13:28Z

Right, in that case fluentd will default to the instance resource and pass the tag to Stackdriver. There will be no association with the Kubernetes node or any other Kubernetes entity.

I see. The logs are only associated with the GCE VM instances, and these logs will have two tags (container-runtime and docker) for the time being.

igorpeshansky · 2018-02-07T00:38:41Z

There is no docker tag in Stackdriver — unknown tags are translated into log names, which are per-project and user-defined. Feel free to use either.

Random-Liu · 2018-02-07T01:48:46Z

@igorpeshansky Thanks a lot for the information. :)

roberthbailey · 2018-02-07T23:42:31Z

/approve (for cluster directory changes)

yujuhong · 2018-02-07T23:45:32Z

/lgtm

Random-Liu · 2018-02-08T02:08:44Z

@coffeepac This is only a config change. Do we really want to build a new image with new version?

I feel like we should only update the config version https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/fluentd-elasticsearch/fluentd-es-configmap.yaml#L4

Random-Liu · 2018-02-08T02:11:00Z

@coffeepac Updated from 0.1.3 to 0.1.4

Random-Liu · 2018-02-08T02:20:06Z

Apply LGTM based on #59103 (comment)

coffeepac · 2018-02-08T17:36:57Z

@Random-Liu looks good! thanks.

coffeepac · 2018-02-08T17:37:31Z

/test pull-kubernetes-bazel-test

crassirostris · 2018-02-12T12:43:16Z

cluster/gce/gci/configure-helper.sh

@@ -2041,6 +2041,11 @@ function start-fluentd-resource-update {
  wait-for-apiserver-and-update-fluentd ${fluentd_gcp_yaml} &
 }

+# Update {{ container-runtime }} with actual container runtime name.
+function update-container-runtime {
+  sed -i -e "s@{{ *container_runtime *}}@${CONTAINER_RUNTIME_NAME:-docker}@g" "$1"


Please make a local variable from the argument, so it's much easier to read this code later (i.e. local -r configmap_path="$1")

crassirostris

Thanks a lot for the change!

Please bump fluentd-gcp configmap version & both for fluentd-gcp and fluentd-es sync configmap version in the configmap manifest file & in the damonset manifest file

Random-Liu · 2018-02-12T18:11:55Z

Please bump fluentd-gcp configmap version & both for fluentd-gcp and fluentd-es sync configmap version in the configmap manifest file & in the damonset manifest file

Done. fluentd-es-config is changed to 0.1.4. fluentd-gcp-config is changed to 1.2.4.
Thanks for reviewing!

Random-Liu · 2018-02-12T21:30:15Z

/test pull-kubernetes-bazel-test

crassirostris

One nit, otherwise LGTM, thanks for the change!

crassirostris · 2018-02-13T09:16:36Z

cluster/gce/config-default.sh

@@ -82,7 +82,8 @@ NODE_IMAGE_PROJECT=${KUBE_GCE_NODE_PROJECT:-cos-cloud}
 NODE_SERVICE_ACCOUNT=${KUBE_GCE_NODE_SERVICE_ACCOUNT:-default}
 CONTAINER_RUNTIME=${KUBE_CONTAINER_RUNTIME:-docker}
 CONTAINER_RUNTIME_ENDPOINT=${KUBE_CONTAINER_RUNTIME_ENDPOINT:-}
-LOAD_IMAGE_COMMAND=${KUBE_LOAD_IMAGE_COMMAND:-docker load -i}
+CONTAINER_RUNTIME_NAME=${KUBE_CONTAINER_RUNTIME_NAME:-}
+LOAD_IMAGE_COMMAND=${KUBE_LOAD_IMAGE_COMMAND:-}


Why is this parameters changed? I don't understand how it's related to the rest of the changes

Because we don't need a default value here. The default value is set at https://github.com/kubernetes/kubernetes/blob/master/cluster/gce/gci/configure.sh#L261.

I added the initial code, just want to do a bit cleanup. :P

OK, thanks for the explanation!

Random-Liu · 2018-02-13T18:23:45Z

/test pull-kubernetes-bazel-test

Signed-off-by: Lantao Liu <lantaol@google.com>

crassirostris

/lgtm

crassirostris · 2018-02-14T10:07:44Z

cluster/gce/config-default.sh

@@ -82,7 +82,8 @@ NODE_IMAGE_PROJECT=${KUBE_GCE_NODE_PROJECT:-cos-cloud}
 NODE_SERVICE_ACCOUNT=${KUBE_GCE_NODE_SERVICE_ACCOUNT:-default}
 CONTAINER_RUNTIME=${KUBE_CONTAINER_RUNTIME:-docker}
 CONTAINER_RUNTIME_ENDPOINT=${KUBE_CONTAINER_RUNTIME_ENDPOINT:-}
-LOAD_IMAGE_COMMAND=${KUBE_LOAD_IMAGE_COMMAND:-docker load -i}
+CONTAINER_RUNTIME_NAME=${KUBE_CONTAINER_RUNTIME_NAME:-}
+LOAD_IMAGE_COMMAND=${KUBE_LOAD_IMAGE_COMMAND:-}


OK, thanks for the explanation!

crassirostris · 2018-02-14T10:08:25Z

/approve

crassirostris · 2018-02-14T10:09:05Z

/retest

k8s-ci-robot · 2018-02-14T10:09:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: crassirostris, Random-Liu, roberthbailey, yujuhong

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these OWNERS Files:

~~cluster/OWNERS~~ [roberthbailey]
~~test/e2e/instrumentation/logging/OWNERS~~ [crassirostris]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-github-robot · 2018-02-14T10:47:18Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-02-14T11:33:20Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot requested review from bowei, coffeepac, crassirostris, piosz and roberthbailey January 31, 2018 09:22

Random-Liu assigned crassirostris and yujuhong Jan 31, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 7, 2018

Random-Liu force-pushed the upload-container-runtime-log branch from de37dcb to 5c4a9ec Compare February 8, 2018 02:10

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 8, 2018

Random-Liu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 8, 2018

crassirostris reviewed Feb 12, 2018

View reviewed changes

crassirostris suggested changes Feb 12, 2018

View reviewed changes

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 12, 2018

crassirostris reviewed Feb 13, 2018

View reviewed changes

Upload container runtime log to sd/es.

8d920d0

Signed-off-by: Lantao Liu <lantaol@google.com>

Random-Liu force-pushed the upload-container-runtime-log branch from 29c970b to 8d920d0 Compare February 13, 2018 18:25

crassirostris approved these changes Feb 14, 2018

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 14, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2018

k8s-github-robot merged commit bc9c6df into kubernetes:master Feb 14, 2018

Random-Liu deleted the upload-container-runtime-log branch February 14, 2018 18:58

Random-Liu mentioned this pull request Feb 14, 2018

Add cri-containerd daemon log support. kubernetes/test-infra#6823

Merged

Random-Liu mentioned this pull request Mar 19, 2018

CRI: Support daemon logs for other CRI container runtime in fluentd? #55501

Closed

wojtek-t mentioned this pull request Apr 5, 2018

'PATCH node-status' latency slo violations #62064

Closed

Upload container runtime log to sd/es. #59103

Upload container runtime log to sd/es. #59103

Conversation

Random-Liu commented Jan 31, 2018 • edited Loading

Random-Liu commented Jan 31, 2018

Random-Liu commented Jan 31, 2018 • edited Loading

yujuhong commented Feb 1, 2018

coffeepac commented Feb 2, 2018

igorpeshansky commented Feb 3, 2018

yujuhong commented Feb 6, 2018

igorpeshansky commented Feb 6, 2018

Random-Liu commented Feb 6, 2018 • edited Loading

yujuhong commented Feb 6, 2018

igorpeshansky commented Feb 7, 2018

Random-Liu commented Feb 7, 2018

yujuhong commented Feb 7, 2018

igorpeshansky commented Feb 7, 2018

Random-Liu commented Feb 7, 2018

roberthbailey commented Feb 7, 2018

yujuhong commented Feb 7, 2018

Random-Liu commented Feb 8, 2018 • edited Loading

Random-Liu commented Feb 8, 2018

Random-Liu commented Feb 8, 2018

coffeepac commented Feb 8, 2018

coffeepac commented Feb 8, 2018

crassirostris Feb 12, 2018

Choose a reason for hiding this comment

Random-Liu Feb 12, 2018

Choose a reason for hiding this comment

crassirostris left a comment

Choose a reason for hiding this comment

Random-Liu commented Feb 12, 2018 • edited Loading

Random-Liu commented Feb 12, 2018

crassirostris left a comment

Choose a reason for hiding this comment

crassirostris Feb 13, 2018

Choose a reason for hiding this comment

Random-Liu Feb 13, 2018

Choose a reason for hiding this comment

crassirostris Feb 14, 2018

Choose a reason for hiding this comment

Random-Liu commented Feb 13, 2018

crassirostris left a comment

Choose a reason for hiding this comment

crassirostris Feb 14, 2018

Choose a reason for hiding this comment

crassirostris commented Feb 14, 2018

crassirostris commented Feb 14, 2018

k8s-ci-robot commented Feb 14, 2018

k8s-github-robot commented Feb 14, 2018

k8s-github-robot commented Feb 14, 2018

Random-Liu commented Jan 31, 2018 •

edited

Loading

Random-Liu commented Jan 31, 2018 •

edited

Loading

Random-Liu commented Feb 6, 2018 •

edited

Loading

Random-Liu commented Feb 8, 2018 •

edited

Loading

Random-Liu commented Feb 12, 2018 •

edited

Loading