Skip to content

fix(pod): respect ContainersReady status in waitForPodReady#245

Closed
FourFriends wants to merge 9 commits intokestra-io:mainfrom
FourFriends:feat-1218-pod-ready
Closed

fix(pod): respect ContainersReady status in waitForPodReady#245
FourFriends wants to merge 9 commits intokestra-io:mainfrom
FourFriends:feat-1218-pod-ready

Conversation

@FourFriends
Copy link
Copy Markdown

Summary

This PR updates PodService.waitForPodReady to make the readiness check more strict.

Details

Previously, the method only checked for the presence of a ContainersReady condition or a PodCompleted reason, without verifying the condition status. As a result, Pods with ContainersReady status False or Unknown could be treated as ready.

Now, waitForPodReady only considers a Pod as ready when:

  • it has a ContainersReady condition with status True, or
  • it has a condition with reason PodCompleted.

This makes the Pod readiness handling more accurate and aligned with Kubernetes semantics.

resolve this issue #244

@github-project-automation github-project-automation Bot moved this to To review in Pull Requests Dec 18, 2025
@FourFriends FourFriends changed the title Ensure waitForPodReady only treats Pods as ready when ContainersReady… fix(pod): respect ContainersReady status in waitForPodReady Dec 18, 2025
@FourFriends
Copy link
Copy Markdown
Author

@fdelbrayelle can you help review this pr, thanks?

Copy link
Copy Markdown
Member

@fdelbrayelle fdelbrayelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution @FourFriends! Could you write a unit test who check this new business rule please 🙏 ? Thanks!

@fdelbrayelle fdelbrayelle added area/plugin Plugin-related issue or feature request kind/external Pull requests raised by community contributors labels Dec 19, 2025
@FourFriends
Copy link
Copy Markdown
Author

@fdelbrayelle Hi

I've added unit tests for waitForPodReady to verify the new business rule that ContainersReady status must be "True". The tests cover:

  • ContainersReady=True returns ready ✅
  • ContainersReady=False/Unknown/empty does not return ready ✅
  • PodCompleted and Failed phase still work (preserved behavior) ✅

Tests use Mockito and don't require a real K8s cluster. Thanks! 🙏

@fdelbrayelle
Copy link
Copy Markdown
Member

Hi @FourFriends 👋
Thank you. We'd prefer you to add real tests without mocks in this test class. You can test it even locally by running .github/setup-unit.sh which runs a K8s cluster thanks to Kind for you.

@Malaydewangan09
Copy link
Copy Markdown
Member

Hey @FourFriends 👋, are there any updates on this?

@FourFriends
Copy link
Copy Markdown
Author

@Malaydewangan09 I've been quite busy lately, but I will add the integration tests soon. thanks

@MilosPaunovic
Copy link
Copy Markdown
Member

Hey @FourFriends, are there any updates on this?

@Malaydewangan09
Copy link
Copy Markdown
Member

Closing in favour of #266

@FourFriends
Copy link
Copy Markdown
Author

thanks, too busy last month, thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/plugin Plugin-related issue or feature request kind/external Pull requests raised by community contributors

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants