I think this is it. I want to run tests on real cluster, but I believe it's ready for review.

@lavalamp @davidopp

looks OK to me.

Ping me when you get tests working. :)

@lavalamp tests are passing now
(I didn't review the PR, just saw your comment)

I guess that @lavalamp meant the tests I mentioned in previous comment. There's no e2e tests for this and I didn't have time to write those. I'll hand test it today and add tests sometime next week.

@lavalamp I run manual tests and it seems to work now (I had a bug - now I skip master node when looking for Ready Nodes).

Can you exercise your bugfix in the unit test? then LGTM.

There is a unit test for this - the last case I added checks exactly this.

@k8s-bot test this issue: #IGNORE
yesterday's jenkins problems

Sorry, maybe I'm blind, but I can't see a test case with a node having a name ending in "-master"?

Oh, sorry - the master thing. I miss-read your comment.

@lavalamp PTAL

LGTM!

It's pretty high in the queue, but we might need to merge it by hand if it won't make it until tomorrow.

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

GCE e2e build/test passed for commit 6d27009.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue

This looks like it might also resolve the scenario described in #19124. Will it be backported to 1.2 as well?

@pesho No. But 1.3 should be out relatively soon (less than a month).

I am seeing this error in the kubelet logs:

E0930 18:21:12.386245    2565 kubelet.go:2913] Error updating node status, will retry: failed to get node address from cloud provider: RequestError: send request failed
caused by: Get http://169.254.169.254/latest/meta-data/local-ipv4: dial tcp 169.254.169.254:80: getsockopt: no route to host

All pods in the node were rescheduled to other nodes. Is this how this is supposed to behave? Also, did it reschedule without doing few retries at the least?

@manojlds - yes it is is as expected, and NC does a number of retries before it evicts Pods (by default 30 times, but it depends on how often NodeStatus is reported, and what are the settings for PodEvictionGracePeriod). From what you wrote there were other Nodes that were serving, so the behavior seems to be correct, or did you expect something else?

This log line suggests that the cloud provider became unreachable from some of your Nodes. @yujuhong

@gmarek - thanks. This is the only instance of the error I could see in the log, so not sure if it did retry before actually evicting the pods. The behaviour seems fine, only doubt/question is on the retry.

Even if the kubelet does not retry it's fine, as NC requires a lot of failed NodeStatus updates to actually start doing something.

For kubelet-side of this @yujuhong should be able to answer/point you to someone who can.

Kubelet should retry up to 5 times by default. On top of that, kubelet tries to update the node status every 10s (or a customized period passed by flags/configs), and node controller only reacts in O(min). One failed attempt should not cause any serious reactions.

@yujuhong thanks. Will take a look at the flags, and see how best I can use them in my cluster.