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

Fixed do_kafka_test.sh script #28

Merged
merged 31 commits into from
Sep 27, 2023
Merged

Conversation

dmccoystephenson
Copy link
Member

@dmccoystephenson dmccoystephenson commented May 10, 2023

Changes

The do_kafka_test.sh was created six years ago, but it was discovered last year that it was not functioning properly due to inconsistencies in container name references and issues with dependent scripts.

The script has been refactored and thoroughly tested to ensure its functionality when executed in WSL.

Additional Changes

  • The dockerfile.testing file has been added to the project for testing purposes. This includes a python installation.
  • The majority of the code has been moved into methods in do_kafka_test.sh
  • Color has been added to the do_kafka_test.sh, do_bsm_test.sh and do_tim_test.sh scripts.
  • The standalone.sh and standalone-multi.sh scripts have been refactored.
  • Descriptions as comments have been added to a number of scripts.
  • The DOCKER_HOST_IP environment variable is now required to be set for the do_kafka_test.sh script to run.
  • The docker-compose-kafka.yml file has been added for testing purposes.

Testing

These changes have been verified to be working when the script is run locally in WSL.

Copy link

@drewjj drewjj left a comment

Choose a reason for hiding this comment

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

This looks good to me

Dockerfile-nsv Outdated Show resolved Hide resolved
Copy link

@payneBrandon payneBrandon left a comment

Choose a reason for hiding this comment

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

Looking good, just a couple comments

@payneBrandon
Copy link

@dmccoystephenson the do_kafka_test.sh script seems to be failing on all the tests for me

Copy link

@payneBrandon payneBrandon left a comment

Choose a reason for hiding this comment

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

Tests currently failing - maybe I missed a step somewhere?

@dmccoystephenson
Copy link
Member Author

dmccoystephenson commented Jun 22, 2023

Tests currently failing - maybe I missed a step somewhere?

The tests fail sometimes when data cannot be verified to be consumed from the kafka cluster. I have found that the tests are more likely to pass after a fresh restart of the host computer. It's possible that some background processing with docker is causing the issue (which would explain why a restart would help). I'll see if I can reproduce the fail case and identify what the problem is.

@dmccoystephenson
Copy link
Member Author

Tests pass for me. I am running do_kafka_test.sh in WSL with no other active containers present at the time of execution.

I will continue to experiment until the tests fail and then investigate further.

@dmccoystephenson dmccoystephenson marked this pull request as draft June 23, 2023 17:43
@dmccoystephenson dmccoystephenson marked this pull request as ready for review June 23, 2023 19:05
@dmccoystephenson
Copy link
Member Author

Compilation was failing when using Ubuntu 20.04 as the base image. After switching back to using Ubuntu 18.04, the tests are passing again.

@dmccoystephenson dmccoystephenson marked this pull request as draft June 23, 2023 20:10
@dmccoystephenson dmccoystephenson marked this pull request as ready for review June 23, 2023 20:16
@dmccoystephenson dmccoystephenson requested a review from drewjj June 23, 2023 21:09
@dmccoystephenson
Copy link
Member Author

These changes have been re-deployed to our dev cluster and data has been verified to still be flowing.

Copy link

@drewjj drewjj left a comment

Choose a reason for hiding this comment

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

Looks like the tests are not able to consume any of the produced messages. Not sure if this is a timing issue or something else. Definitely needs some more investigation.

@dmccoystephenson dmccoystephenson marked this pull request as draft June 23, 2023 22:58
dmccoystephenson pushed a commit that referenced this pull request Jul 11, 2023
Sync Master branch to develop
… PPM logs is above 100 instead of waiting a fixed amount of time for PPM readiness.
@dmccoystephenson
Copy link
Member Author

I suspect that the do_kafka_test.sh script was failing sometimes because data got produced before the PPM was ready to consume it. I have modified the standalone test scripts to wait until the PPM logs reach 100 lines instead of waiting a fixed 20 seconds for PPM readiness.

Please let me know if you run into more issues @drewjj

@dmccoystephenson dmccoystephenson requested a review from drewjj July 11, 2023 20:53
@dmccoystephenson dmccoystephenson marked this pull request as ready for review July 11, 2023 20:54
Copy link

@drewjj drewjj left a comment

Choose a reason for hiding this comment

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

I am still seeing my tests fail. I am running it in Ubuntu, I have the proper IP information setup due to the script successfully creating the messages but the test seems to fail at reading them for its test case. Unsure what to do here.
image

@dmccoystephenson dmccoystephenson requested a review from drewjj July 14, 2023 20:25
Copy link

@drewjj drewjj left a comment

Choose a reason for hiding this comment

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

The test script works great now. I hadn't fully updated my local repository before.

Copy link
Member Author

@dmccoystephenson dmccoystephenson left a comment

Choose a reason for hiding this comment

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

Self-review

README.md Outdated Show resolved Hide resolved
do_kafka_test.sh Outdated Show resolved Hide resolved
do_kafka_test.sh Outdated Show resolved Hide resolved
@payneBrandon payneBrandon merged commit bec5045 into develop Sep 27, 2023
@payneBrandon payneBrandon deleted the fixing-kafka-test-script branch September 27, 2023 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants