Fix the race when monit is used to monitor services which are also #8931

dchen1107 · 2015-05-28T18:08:43Z

started at boot time via init.d at boot time.

I ran e2e tests earlier this morning, but failed due to e2e current breakage. I manually tested this on machine without current patch, manually running service kubelet start (not restart) will reproduce binding issue reported in #8742

# ps -ef | grep kubelet
root      5609     1  1 17:38 ?        00:00:12 /usr/local/bin/kubelet --api_servers=https://kubernetes-master --cloud_provider=gce --config=/etc/kubernetes/manifests --allow_privileged=False --v=2 --cluster_dns=10.0.0.10 --cluster_domain=cluster.local --configure-cbr0=true --cgroup_root=/

# service kubelet start
[ ok ] Starting The Kubernetes container manager: kubelet.

# ps -ef | grep kubelet
root     16737     1  9 17:56 ?        00:00:01 /usr/local/bin/kubelet --api_servers=https://kubernetes-master --cloud_provider=gce --config=/etc/kubernetes/manifests --allow_privileged=False --v=2 --cluster_dns=10.0.0.10 --cluster_domain=cluster.local --configure-cbr0=true --cgroup_root=/

cc/ @vmarmol @roberthbailey

vmarmol · 2015-05-28T18:21:39Z

cluster/saltbase/salt/kube-proxy/initd

@@ -40,6 +40,10 @@ DAEMON_USER=root
 #
 do_start()
 {
+        # Avoid race at boot time when both monit and init.d start the service
+        PID=$(ps -ef | grep $DAEMON | grep -v grep | awk '{print $2}')


is pidof not available?

I thought pidof here reports a wrong pid in this case. Just checked it, actually pidof does scan /proc and find matching name. Will update it.

roberthbailey · 2015-05-28T18:37:25Z

Thanks Dawn!

vmarmol · 2015-05-28T18:45:58Z

cluster/saltbase/salt/kube-proxy/initd

+        # Avoid a potential race at boot time when both monit and init.d start
+        # the same service
+        PIDS=$(pidof $DAEMON)
+        for PID in "${PIDS}"; do


I think we don't want this quoted, else it treats it as a single string.

Oops, same mistake again. Fixed. Thanks for catching this, and luckily we only have one for each daemon here. :-(

started at boot time via init.d at boot time.

vmarmol · 2015-05-28T19:01:12Z

LGTM, thanks @dchen1107!

Fix the race when monit is used to monitor services which are also

googlebot added the cla: yes label May 28, 2015

vmarmol self-assigned this May 28, 2015

vmarmol reviewed May 28, 2015
View reviewed changes

dchen1107 force-pushed the clean branch from 3bc095e to 4d49669 Compare May 28, 2015 18:46

Fix the race when monit is used to monitor services which are also

79a2a4f

started at boot time via init.d at boot time.

dchen1107 force-pushed the clean branch from 4d49669 to 79a2a4f Compare May 28, 2015 18:58

vmarmol added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 28, 2015

rjnagal added a commit that referenced this pull request May 29, 2015

Merge pull request #8931 from dchen1107/clean

6274d84

Fix the race when monit is used to monitor services which are also

rjnagal merged commit 6274d84 into kubernetes:master May 29, 2015

justinsb mentioned this pull request May 30, 2015

WIP: Workaround for race between monit and init #8743

Closed

saad-ali mentioned this pull request Jul 9, 2015

Flaky e2e: Proxy version v1 should proxy logs on node (Failed 4 times in the last 30 runs. Stability: 86 % #10792

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the race when monit is used to monitor services which are also #8931

Fix the race when monit is used to monitor services which are also #8931

dchen1107 commented May 28, 2015

vmarmol May 28, 2015

dchen1107 May 28, 2015

roberthbailey commented May 28, 2015

vmarmol May 28, 2015

dchen1107 May 28, 2015

vmarmol commented May 28, 2015

Fix the race when monit is used to monitor services which are also #8931

Fix the race when monit is used to monitor services which are also #8931

Conversation

dchen1107 commented May 28, 2015

vmarmol May 28, 2015

Choose a reason for hiding this comment

dchen1107 May 28, 2015

Choose a reason for hiding this comment

roberthbailey commented May 28, 2015

vmarmol May 28, 2015

Choose a reason for hiding this comment

dchen1107 May 28, 2015

Choose a reason for hiding this comment

vmarmol commented May 28, 2015