Add pod readiness check #5595
Add pod readiness check #5595
Conversation
|
Can one of the admins verify this patch? |
| // Check "ready" conditions | ||
| for (PodCondition condition : conditions) { | ||
| if ("ready".equalsIgnoreCase(condition.getType())) { | ||
| if (!"true".equalsIgnoreCase(condition.getStatus())) { |
There was a problem hiding this comment.
Can just return Boolean.parseBoolean(condition.getStatus())?
|
LGTM with one minor tweak. Thanks! |
|
Thanks for taking a look so quickly! I took a look at your suggestion. The reason it is the way it is, is because there may be a chance that there are multiple "Ready" and/or multiple "PodConditions" and I need to go through all of them without breaking out unless it finds one which is false. If I were to just return with the parseBoolean method, it may return a false positive as there may be other "PodConditions" which are not "Ready." For example, say there's two "Ready" "PodConitions" in the list: one false and one true. If the parseBoolean method found the true one first, this method would break out of the loop and return true when in reality, there is still one PodCondition that is false. Or perhaps I misunderstood what you wanted changed. Let me know, thanks! |
|
I didn't know that you could have multiple ready conditions. Can you link to where that's either documented or show where it happens in kubernetes code base? |
|
I was thinking it was possible to have multiple readinessProbes if not now, in the future. However, it looks like if you try to do multiple readinessProbes, it will complain about the configuration, so it seems you can indeed only have one readinessProbe. I'll go ahead and make this update. |
|
The update has been made. |
|
Brilliant thanks. LGTM |
|
OK to test |
|
Merged by PR CI build. Thanks! |
Previously, this was not checking for pod readiness, only that the pod was running. This takes care of that.