Get kubelet log from journald when kubelet is running as a systemd service in e2e tests #19306

yifan-gu · 2016-01-05T23:53:07Z

When the kubelet is running as a systemd service, we need to get the kubelet log from journald instead of /var/log/kubelet.log

I tested by running some jenkins e2e jobs against my own coreos/e2e branch. This works.The kubelet logs can be found here (kube-proxy log is still empty, because I haven't run them as static pods on that branch yet).

cc @dchen1107 @jonboulle @ixdy @zmerlynn

k8s-github-robot · 2016-01-05T23:53:42Z

Labelling this PR as size/M

k8s-bot · 2016-01-06T00:32:22Z

GCE e2e test build/test passed for commit 887ed273227fdda07011af6ba26f8ebf765691c9.

ixdy · 2016-01-06T00:44:30Z

cluster/gce/config-default.sh

@@ -36,6 +36,11 @@ NODE_IMAGE_PROJECT=${KUBE_GCE_NODE_PROJECT:-"${MASTER_IMAGE_PROJECT}"}
 CONTAINER_RUNTIME=${KUBE_CONTAINER_RUNTIME:-docker}
 RKT_VERSION=${KUBE_RKT_VERSION:-0.5.5}

+# If the distro is CoreOS, kubelet will run as a systemd service.
+if [[ "${OS_DISTRIBUTION}" == "coreos" ]]; then


Looking at https://github.com/kubernetes/kubernetes/blob/master/cluster/saltbase/pillar/systemd.sls, I worry this is a bit too fragile.

Can we instead somehow detect at run-time in the code whether the kubelet is running under systemd or not?

@ixdy ACK, I will give a try tomorrow, thanks!

@ixdy How about ssh'ing to all the hosts, and then check the return code of 'sudo systemctl status kubelet.service' ?

could you use go-systemd RunningFromSystemService?

oops, sorry,missed exactly where you're checking this from - I doubt you want to expose this from within the kubelet.

k8s-bot · 2016-01-06T00:55:29Z

GCE e2e test build/test passed for commit 03fc3c27715e08083eacea5ac2371784f175341a.

yifan-gu · 2016-01-07T21:36:40Z

Pushed a new version, which checks the return code of systemctl status kubelet.service at runtime to determine whether kubelet is running as a systemd service.
Verified by running e2e tests against gce/coreos on my branch. It successfully gets the kubelet logs

k8s-bot · 2016-01-07T22:05:27Z

GCE e2e build/test failed for commit ed5ff44bf8bb4264e9ba3d3c9508417ff7c20b5d.

yifan-gu · 2016-01-07T22:13:54Z

@ixdy This hardcodes "kubelet.service" as the name of the kubelet systemd service. Might not be the best solution, but should be better than my original one. Feedback is welcome :)

ixdy · 2016-01-07T22:27:28Z

test/e2e/core.go

@@ -97,3 +123,29 @@ func logCore(cmds []command, hosts []string, dir, provider string) {
 	}
 	wg.Wait()
 }
+
+func findKubeletSystemdService(provider string, hosts ...string) (bool, error) {


two minor nits:

maybe "isUsingSystemdKubelet" is a better name for this function?

you never use the error return value - should this return only bool?

@ixdy Didn't follow what you meant by "never use the error return value"? If there's an error, a message will be printed.

k8s-bot · 2016-01-08T02:37:24Z

GCE e2e test build/test passed for commit ddddb4aa7d6efbce7fa18cf85db5589475db47fc.

ixdy · 2016-01-08T02:43:43Z

test/e2e/core.go

@@ -97,3 +123,29 @@ func logCore(cmds []command, hosts []string, dir, provider string) {
 	}
 	wg.Wait()
 }
+
+func isUsingSystemdKubelet(provider string, hosts ...string) (bool, error) {


there are only two return points from this function on lines 147 and 150, and neither one returns an error.

@ixdy Got you. Removed the returned error. Thanks!

k8s-bot · 2016-01-08T18:59:29Z

GCE e2e build/test failed for commit c3405f54ea6d7f563630569520fe78d4cdbf1b3a.

ixdy · 2016-01-08T19:33:07Z

test/e2e/core.go

+	cmds := []command{{"cat /var/log/kube-proxy.log", "kube-proxy"}}
+
+	found := isUsingSystemdKubelet(provider, hosts...)
+	if found {


one more minor nit (sorry): rather than using the variable found, maybe just have

if isUsingSystemdKubelet(provider, hosts...) { ... } else { ... }

similarly for the master.

k8s-bot · 2016-01-08T21:29:50Z

GCE e2e build/test failed for commit f01c621888ab693cecc10b347d60bb7196fd995d.

yifan-gu · 2016-01-08T22:18:27Z

@k8s-bot test this please

derekwaynecarr · 2016-01-08T22:22:48Z

test/e2e/core.go

-		{"cat /var/log/supervisor/supervisord.log", "supervisord"},
+	cmds := []command{{"cat /var/log/kube-proxy.log", "kube-proxy"}}
+	if isUsingSystemdKubelet(provider, hosts...) {
+		cmds = append(cmds, command{"sudo journalctl --output=cat -u kubelet.service", "kubelet"})


can we let the name of kubelet.service get passed down somehow rather than hard-code it?

we could factor out "kubelet.service" into a constant, sure. how likely is this to change, though?

k8s-bot · 2016-01-08T23:10:19Z

GCE e2e test build/test passed for commit f01c621888ab693cecc10b347d60bb7196fd995d.

yifan-gu · 2016-01-08T23:39:15Z

can we let the name of kubelet.service get passed down somehow rather than hard-code it?

@derekwaynecarr I feel it's better. Also if pass the name of the service, do we still need to check kubelet service at runtime?
For example, If we pass a flag --kubelet-service-name="", can we assume if the flag is non-empty, the kubelet is running as a service?
@ixdy

ixdy · 2016-01-09T01:35:54Z

I think this is probably fine as-is.

yifan-gu · 2016-01-09T01:47:44Z

I think this is probably fine as-is.

@ixdy Sorry, do you mean the current approach (which hardcodes the kubelet.service) is fine, or passing --kubelet-service-name flag is fine?

ixdy · 2016-01-09T01:55:58Z

Approach as currently implemented (everything hard-coded) is fine with me.

jonboulle · 2016-01-09T13:27:28Z

test/e2e/core.go

+			if err != nil {
+				fmt.Printf("Error running command: %v\n", err)
+			}
+			results[i] = result.Code


This is going to set to 0 if err != nil, is that right/expected?

@jonboulle So in this case we assume the kubelet is running as systemd service even when there are errors during SSH().

@jonboulle raises a good point. if a node has gone away (and thus this ssh call fails), we don't want that to influence the choice of logging location for the rest of the nodes.

I think we should probably be using only successful ssh connections to detect whether nodes are using systemd or not. Could probably even shortcut - return true/false for the first host that we successfully connect to, rather than checking all. I imagine that's the easiest fix.

@ixdy Actually SSH failures doens't affect this as the code is still 0 when there is an error. But I made a change so instead of assume "kubelet is running as systemd service", now we assume "kubelet is not running as systemd service" and try to find the first successful detection of the kubelet.service.

yifan-gu · 2016-01-11T23:21:16Z

@ixdy So is this OK to merge or you have more concerns?

yifan-gu · 2016-01-12T21:27:34Z

ping @ixdy ?

If there is any successful detection of kubelet.service, fetch the kubelet logs using journalctl.

yifan-gu · 2016-01-12T23:53:27Z

@ixdy Updated.
Now if there's any successful detection of kubelet.service, then use journalctl to fetch the log.

ixdy · 2016-01-13T00:15:08Z

LGTM, thanks

k8s-bot · 2016-01-13T00:42:44Z

GCE e2e test build/test passed for commit 3b66179039f36f10722c08dfb034db588f2ba603.

k8s-bot · 2016-01-13T00:56:15Z

GCE e2e test build/test passed for commit fb5ee73.

k8s-github-robot · 2016-01-13T10:55:59Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-bot · 2016-01-13T11:33:01Z

GCE e2e test build/test passed for commit fb5ee73.

k8s-github-robot · 2016-01-13T11:33:13Z

Automatic merge from submit-queue

Auto commit by PR queue bot

googlebot added the cla: yes label Jan 5, 2016

k8s-github-robot assigned ixdy Jan 5, 2016

k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 5, 2016

yifan-gu changed the title ~~Kubelet service e2e~~ Get kubelet log from journald when kubelet is running as a systemd service Jan 5, 2016

yifan-gu changed the title ~~Get kubelet log from journald when kubelet is running as a systemd service~~ Get kubelet log from journald when kubelet is running as a systemd service in e2e tests Jan 5, 2016

yifan-gu force-pushed the kubelet_service_e2e branch from 887ed27 to 03fc3c2 Compare January 6, 2016 00:11

ixdy reviewed Jan 6, 2016
View reviewed changes

yifan-gu force-pushed the kubelet_service_e2e branch from 03fc3c2 to ed5ff44 Compare January 7, 2016 21:33

ixdy reviewed Jan 7, 2016
View reviewed changes

yifan-gu force-pushed the kubelet_service_e2e branch from ed5ff44 to ddddb4a Compare January 8, 2016 02:02

ixdy reviewed Jan 8, 2016
View reviewed changes

yifan-gu force-pushed the kubelet_service_e2e branch from ddddb4a to c3405f5 Compare January 8, 2016 18:26

ixdy reviewed Jan 8, 2016
View reviewed changes

yifan-gu force-pushed the kubelet_service_e2e branch from c3405f5 to f01c621 Compare January 8, 2016 20:54

derekwaynecarr reviewed Jan 8, 2016
View reviewed changes

jonboulle reviewed Jan 9, 2016
View reviewed changes

yifan-gu force-pushed the kubelet_service_e2e branch from f01c621 to 3b66179 Compare January 12, 2016 23:51

e2e: Add support for getting kubelet logs from journald.

fb5ee73

If there is any successful detection of kubelet.service, fetch the kubelet logs using journalctl.

yifan-gu force-pushed the kubelet_service_e2e branch from 3b66179 to fb5ee73 Compare January 12, 2016 23:53

ixdy added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 13, 2016

k8s-github-robot pushed a commit that referenced this pull request Jan 13, 2016

Merge pull request #19306 from yifan-gu/kubelet_service_e2e

e1a71fe

Auto commit by PR queue bot

k8s-github-robot merged commit e1a71fe into kubernetes:master Jan 13, 2016

Get kubelet log from journald when kubelet is running as a systemd service in e2e tests #19306

Get kubelet log from journald when kubelet is running as a systemd service in e2e tests #19306

Conversation

yifan-gu commented Jan 5, 2016

k8s-github-robot commented Jan 5, 2016

k8s-bot commented Jan 6, 2016

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

k8s-bot commented Jan 6, 2016

yifan-gu commented Jan 7, 2016

k8s-bot commented Jan 7, 2016

yifan-gu commented Jan 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Jan 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Jan 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Jan 8, 2016

yifan-gu commented Jan 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-bot commented Jan 8, 2016

yifan-gu commented Jan 8, 2016

ixdy commented Jan 9, 2016

yifan-gu commented Jan 9, 2016

ixdy commented Jan 9, 2016

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

yifan-gu commented Jan 11, 2016

yifan-gu commented Jan 12, 2016

yifan-gu commented Jan 12, 2016

ixdy commented Jan 13, 2016

k8s-bot commented Jan 13, 2016

k8s-bot commented Jan 13, 2016

k8s-github-robot commented Jan 13, 2016

k8s-bot commented Jan 13, 2016

k8s-github-robot commented Jan 13, 2016