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 current code, not published images, in ES integration tests #5709

Closed
yurishkuro opened this issue Jul 4, 2024 · 2 comments
Closed

Use current code, not published images, in ES integration tests #5709

yurishkuro opened this issue Jul 4, 2024 · 2 comments
Labels
good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@yurishkuro
Copy link
Member

indexCleanerImage = "jaegertracing/jaeger-es-index-cleaner:latest"
rolloverImage = "jaegertracing/jaeger-es-rollover:latest"

This test is invoked from scripts/es-integration-test.sh.

Here the test is using :latest build of the helper containers. This is what happens in the CI:

=== RUN   TestIndexCleaner_doNotFailOnEmptyStorage
Unable to find image 'jaegertracing/jaeger-es-index-cleaner:latest' locally
latest: Pulling from jaegertracing/jaeger-es-index-cleaner
Digest: sha256:9020812f290230b65e2ada3dcab193c419b7f8bb29435bab045f8802482d0004
Status: Downloaded newer image for jaegertracing/jaeger-es-index-cleaner:latest

In other words, instead of testing the current code (which could've been changed in the PR under test), we are using a published image built from the main branch.

We need to refactor this test. Since these helper containers are not used in any other CI workflows, it's good to test them in this workflow. We need to build the containers locally similar to this other script:

bash scripts/build-upload-a-docker-image.sh ${LOCAL_FLAG} -b -c jaeger-es-index-cleaner -d cmd/es-index-cleaner -p "${platforms}" -t release
bash scripts/build-upload-a-docker-image.sh ${LOCAL_FLAG} -b -c jaeger-es-rollover -d cmd/es-rollover -p "${platforms}" -t release

And then use a specific tag (not :latest) when executing the test.

@yurishkuro yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Jul 4, 2024
@hellspawn679
Copy link
Contributor

@yurishkuro, can you give a reference to how containers are built locally and tested in other scripts?

@yurishkuro
Copy link
Member Author

I already have a link in the description

yurishkuro pushed a commit that referenced this issue Jul 8, 2024
## Which problem is this PR solving?
- #5709

## Description of the changes
- added a function build_local_img

## How was this change tested?
- 

## 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: mehul gautam <mehulsharma4786@gamil.com>
Signed-off-by: mehul gautam  <mehulsharma4786@gmail.com>
Co-authored-by: mehul gautam <mehulsharma4786@gamil.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants