Scheduler ignores nodes that are in a bad state #7668
Conversation
|
/sub |
| // We currently only use a conditionType of "Ready". If the kubelet doesn't | ||
| // periodically report the status of a node, the nodecontroller sets its | ||
| // ConditionStatus to "Unknown". If the kubelet thinks a node is unhealthy | ||
| // it can (in theory) set it its ConditionStatus to "False". |
|
|
||
| // We currently only use a conditionType of "Ready". If the kubelet doesn't | ||
| // periodically report the status of a node, the nodecontroller sets its | ||
| // ConditionStatus to "Unknown". If the kubelet thinks a node is unhealthy |
There was a problem hiding this comment.
The nodecontroller sets it to unknown when the kubelet hasn't updated it in so long, the kubelet sets it to something sensible usually.
|
LGTM sans nits-- can you add a test in test/integration/ somewhere? |
|
Doing, wanted to check if this was ok |
bprashanth
force-pushed
the
scheduling_minions
branch
from
e06a218
to
76b54ef
Compare
|
Sorry for the delay, surfacing failures due to watch timeout in the integration tests was a slight pain but I didn't want to checkin something that flaked without reason on shippable given the weird n/w latencies we've observed in the past. PTAL, running e2e. |
bprashanth
changed the title
|
E2e passed |
| // Wait till the passFunc confirms that the object it expects to see is in the store. | ||
| // Used to observe reflected events. | ||
| func waitForReflection(s cache.Store, key string, passFunc func(n interface{}) bool) error { | ||
| return wait.Poll(time.Second, time.Second*20, func() (bool, error) { |
There was a problem hiding this comment.
An entire second between checks in a test? Is it really that slow?
There was a problem hiding this comment.
I can bring it down, copied the polling interval from the scheduler test but it probably isn't. Most cases on localhost it's done on the first poll.
|
Small suggestions. Basically fine. |
bprashanth
force-pushed
the
scheduling_minions
branch
from
9c224e6
to
d93f74d
Compare
|
Addressed comments, PTAL |
|
LGTM |
lavalamp
added
the
lgtm
bprashanth
force-pushed
the
scheduling_minions
branch
from
d93f74d
to
51365a4
Compare
|
Thought it's unlikely i just realize the node controller can hit the pod eviction timeout and delete the pod we're expecting to see scheduled before we see it, because we poll. Hrm. The right way to solve this would be to watch for events. @lavalamp wdyt about doing that in this pr vs a follow up? Edit: the comment above was about the integration test not the actual code change. |
|
Filed #7874 for shippable |
|
You're worried about the test accidentally passing because of that? You could disable the node controller for the test, or switch to events like you say. |
bprashanth
force-pushed
the
scheduling_minions
branch
from
51365a4
to
4b0607c
Compare
|
False alarm, I got confused with the other integration suite, this one doesn't start a nodecontroller to begin with. So I guess this is good to go when things pass, I only made the name of the nod/pod more unique to help with debuggin in my latest upload. |
|
@lavalamp can this be merged ? |
Scheduler ignores nodes that are in a bad state
|
Yeah I was just waiting for it to turn green. |
The scheduler should ignore unhealthy nodes.
ref #7222, #7561