Skip to content
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 reliable check for Kafka broker being ready #5154

Conversation

shashank-iitbhu
Copy link
Contributor

@shashank-iitbhu shashank-iitbhu commented Jan 29, 2024

Which problem is this PR solving?

Resolves #5148

Description of the changes

  • The previous approach used nc to check the availability of the Kafka port, which was not reliable, especially when the Kafka container was started asynchronously. The updated script now ensures a more reliable check for Kafka readiness by attempting to list topics using kafka-topics.sh within a loop. This modification ensures that the script waits until Kafka is fully initialized before proceeding with integration tests.

How was this change tested?

  • This modification ensures that the script waits until Kafka is fully initialized before proceeding with integration tests.
  • Ran bash scripts/kafka-integration-test.sh -k from the project root.

Checklist

Signed-off-by: Shashank Mittal <shashank.mittal.mec22@itbhu.ac.in>
@shashank-iitbhu shashank-iitbhu requested a review from a team as a code owner January 29, 2024 22:25
@yurishkuro yurishkuro changed the title Added reliable check for kafka broker running Add reliable check for Kafka broker being ready Jan 29, 2024
@yurishkuro yurishkuro added the changelog:test Change that's adding missing tests or correcting existing tests label Jan 29, 2024
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (befd5f4) 95.63% compared to head (72dd387) 95.62%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5154      +/-   ##
==========================================
- Coverage   95.63%   95.62%   -0.02%     
==========================================
  Files         324      324              
  Lines       18590    18590              
==========================================
- Hits        17778    17776       -2     
- Misses        651      653       +2     
  Partials      161      161              
Flag Coverage Δ
cassandra-3.x 25.58% <ø> (ø)
cassandra-4.x 25.58% <ø> (ø)
elasticsearch-5.x 19.86% <ø> (ø)
elasticsearch-6.x 19.85% <ø> (-0.02%) ⬇️
elasticsearch-7.x 19.98% <ø> (-0.02%) ⬇️
elasticsearch-8.x 20.08% <ø> (ø)
grpc-badger 19.48% <ø> (-0.02%) ⬇️
kafka 14.09% <ø> (ø)
opensearch-1.x 20.00% <ø> (ø)
opensearch-2.x 20.00% <ø> (ø)
unittests 93.32% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Thanks!

@yurishkuro yurishkuro merged commit ecf1dc0 into jaegertracing:main Jan 29, 2024
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:test Change that's adding missing tests or correcting existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fix] Ensure Kafka broker is started before running integration tests
2 participants