-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add check for containers ready #787
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Varun Gupta <[email protected]>
Signed-off-by: Varun Gupta <[email protected]>
@@ -39,7 +39,7 @@ func IsPodTerminating(pod *v1.Pod) bool { | |||
|
|||
// IsPodReady returns true if a pod is ready; false otherwise. | |||
func IsPodReady(pod *v1.Pod) bool { | |||
return IsPodReadyConditionTrue(pod.Status) | |||
return IsPodReadyConditionTrue(pod.Status) && IsContainerReadyConditionTrue(pod.Status) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's update the comment as well.
// GetPodReadyCondition extracts the pod ready condition from the given status and returns that. | ||
// Returns nil if the condition is not present. | ||
func GetPodReadyCondition(status v1.PodStatus) *v1.PodCondition { | ||
_, condition := GetPodCondition(&status, v1.PodReady) | ||
return condition | ||
} | ||
|
||
// GetContainersReadyCondition extracts the containers ready condition from the given status and returns that. | ||
// Returns nil if the condition is not present. | ||
func GetContainersReadyCondition(status v1.PodStatus) *v1.PodCondition { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we check all the containers ready?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it check the pod condition, still pod level. I notice it's probably my mistake and I forgot double check the target-pod. #781 so we do not have code issue and I think we do not need this PR. Pod will be ready only after all container become ready. I was talking out running
but not ready
in original issue.
status:
conditions:
- lastProbeTime: null
lastTransitionTime: "2025-03-04T03:48:14Z"
status: "True"
type: Initialized
- lastProbeTime: null
lastTransitionTime: "2025-03-04T03:51:56Z"
status: "True"
type: Ready
- lastProbeTime: null
lastTransitionTime: "2025-03-04T03:51:56Z"
status: "True"
type: ContainersReady
Pull Request Description
Along with pod ready condition check, add check for containers ready as well.
Related Issues
Resolves: #781
Important: Before submitting, please complete the description above and review the checklist below.
Contribution Guidelines (Expand for Details)
We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:
Pull Request Title Format
Your PR title should start with one of these prefixes to indicate the nature of the change:
[Bug]
: Corrections to existing functionality[CI]
: Changes to build process or CI pipeline[Docs]
: Updates or additions to documentation[API]
: Modifications to aibrix's API or interface[CLI]
: Changes or additions to the Command Line Interface[Misc]
: For changes not covered above (use sparingly)Note: For changes spanning multiple categories, use multiple prefixes in order of importance.
Submission Checklist
By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.