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

Use docker compose for kafka integration tests #5500

Merged
merged 31 commits into from
May 31, 2024

Conversation

hellspawn679
Copy link
Contributor

@hellspawn679 hellspawn679 commented May 29, 2024

Which problem is this PR solving?

Part of #5485

Description of the changes

  • In script/kafka-integration-test.sh initialized kafka by docker compose
  • updated .github/workflow/ci-kafka.yml to take logs from docker compose
  • added comments in script/kafka-integration-test.sh and fixed a typo
  • add new locations to dependabot config to allow it to upgrade versions in the new docker compose files

How was this change tested?

  • CI

Checklist

Your Name and others added 18 commits May 27, 2024 03:24
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: mehul gautam  <mehulsharma4786@gmail.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
@hellspawn679 hellspawn679 requested a review from a team as a code owner May 29, 2024 20:01
@hellspawn679 hellspawn679 requested a review from joe-elliott May 29, 2024 20:01
@hellspawn679 hellspawn679 changed the title Ci updaing CI for kafka May 29, 2024
Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.18%. Comparing base (629c47e) to head (59e1434).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5500   +/-   ##
=======================================
  Coverage   96.18%   96.18%           
=======================================
  Files         327      327           
  Lines       16012    16012           
=======================================
  Hits        15401    15401           
  Misses        436      436           
  Partials      175      175           
Flag Coverage Δ
badger_v1 8.05% <ø> (ø)
badger_v2 1.93% <ø> (ø)
cassandra-3.x-v1 16.43% <ø> (ø)
cassandra-3.x-v2 1.85% <ø> (ø)
cassandra-4.x-v1 16.43% <ø> (ø)
cassandra-4.x-v2 1.85% <ø> (ø)
elasticsearch-7-v1 18.88% <ø> (?)
elasticsearch-8-v1 19.08% <ø> (?)
elasticsearch-8-v2 19.08% <ø> (?)
elasticsearch-8.x-v2 ?
grpc_v1 9.48% <ø> (ø)
grpc_v2 7.53% <ø> (ø)
kafka 9.78% <ø> (ø)
opensearch-1-v1 18.92% <ø> (?)
opensearch-1.x-v1 ?
opensearch-2-v1 18.94% <ø> (?)
opensearch-2-v2 18.92% <ø> (?)
opensearch-2.x-v1 ?
opensearch-2.x-v2 ?
unittests 93.87% <ø> (ø)

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.

please write informative PR title and description

scripts/kafka-integration-test.sh Outdated Show resolved Hide resolved
.github/dependabot.yml Outdated Show resolved Hide resolved
@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label May 30, 2024
Your Name added 3 commits May 30, 2024 11:39
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
Signed-off-by: Your Name <you@example.com>
@hellspawn679 hellspawn679 changed the title updaing CI for kafka Use docker compose for kafka integration tests May 30, 2024
@hellspawn679
Copy link
Contributor Author

please have a look at dependabot then I will add it for both elaticsearch/opensearch and kafka

@hellspawn679 hellspawn679 requested a review from yurishkuro May 30, 2024 16:46
.github/dependabot.yml Outdated Show resolved Hide resolved
Signed-off-by: Your Name <you@example.com>
@hellspawn679
Copy link
Contributor Author

@yurishkuro as you can see when the elasticsearch 8.x v2failed the output elastic container logs job was not complete to fix this we need to use
docker compose -f docker-compose/elasticsearch/v${{matrix.version.major}}.yml logs
in place of
docker log <GITHUB_OUTPUT>(Cid)
and we probably need to change
the jobs matrix
from {{matrix.version.major}}-->7.x to {{matrix.version.major}}-->7
and we won't be needing GITHUB_OUTPUT any more

@hellspawn679
Copy link
Contributor Author

Screenshot 2024-06-01 003835

Signed-off-by: Your Name <you@example.com>
@hellspawn679 hellspawn679 requested a review from yurishkuro May 31, 2024 19:26
interval: daily
allow:
- dependency-name: "docker.elastic.co/elasticsearch/elasticsearch"
update-types: ["version-update:semver-minor"]
Copy link
Member

Choose a reason for hiding this comment

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

neat, I didn't know this was possible, and it addresses one of my concerns in the ticket (to avoid it auto-upgrade 7x -> 8x)

@hellspawn679
Copy link
Contributor Author

@yurishkuro as you can see when the elasticsearch 8.x v2failed the output elastic container logs job was not complete to fix this we need to use docker compose -f docker-compose/elasticsearch/v${{matrix.version.major}}.yml logs in place of docker log <GITHUB_OUTPUT>(Cid) and we probably need to change the jobs matrix from {{matrix.version.major}}-->7.x to {{matrix.version.major}}-->7 and we won't be needing GITHUB_OUTPUT any more

@yurishkuro can you have a look at this should I include this issue in this pr only ?

Signed-off-by: Your Name <you@example.com>
@yurishkuro
Copy link
Member

can you have a look at this should I include this issue in this pr only ?

Yeah, you can add that here. I think it would be better to keep the encapsulation and not require the workflow to know the exact file name - similar to cid >> $GITHUB_OUTPUT, you can store the compose file name in the output and have the next step use that

Signed-off-by: Your Name <you@example.com>
@hellspawn679 hellspawn679 requested a review from yurishkuro May 31, 2024 20:24
Signed-off-by: Your Name <you@example.com>
@hellspawn679 hellspawn679 requested a review from yurishkuro May 31, 2024 21:24
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
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 enabled auto-merge (squash) May 31, 2024 21:57
@yurishkuro yurishkuro disabled auto-merge May 31, 2024 21:58
@yurishkuro yurishkuro enabled auto-merge (squash) May 31, 2024 21:59
@yurishkuro yurishkuro merged commit ba4da0d into jaegertracing:main May 31, 2024
40 checks passed
varshith257 pushed a commit to varshith257/jaeger that referenced this pull request Jun 2, 2024
## Which problem is this PR solving?
Part of jaegertracing#5485

## Description of the changes
* In `script/kafka-integration-test.sh` initialized kafka by docker
compose
* updated `.github/workflow/ci-kafka.yml` to take logs from docker
compose
* added comments in `script/kafka-integration-test.sh` and fixed a typo 
* add new locations to dependabot config to allow it to upgrade versions
in the new docker compose files

## How was this change tested?
- CI

## Checklist
- [ ] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [ ] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Your Name <you@example.com>
Signed-off-by: mehul gautam  <mehulsharma4786@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Your Name <you@example.com>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Vamshi Maskuri <gwcchintu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:ci Change related to continuous integration / testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants