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 health port for jaeger container startup check. #2426

Merged
merged 1 commit into from
Jan 5, 2021

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Jan 5, 2021

Also migrate images to the ghcr ones I had set up but forgot to apply the change for.

Fixes #1871 (hopefully, will reopen if this doesn't actually fix it)

@@ -67,7 +67,7 @@
public static GenericContainer<?> collectorContainer =
new GenericContainer<>(
DockerImageName.parse(
"open-telemetry-docker-dev.bintray.io/java-test-containers:otel-collector-dev"))
"ghcr.io/open-telemetry/java-test-containers:otel-collector-dev"))
Copy link
Member

Choose a reason for hiding this comment

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

This change seems unrelated to the PR purpose. What is the reason for using a different registry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a long time ever since starting to use bintray for agent smoke tests, we found the push reliability to be close to 0. GHCR has yet to fail :)

I had already updated our mirror workflow

#2161

But forgot to update the tests after that run once.

It's indeed not strongly related to the purpose but touches the same lines mostly, please let me be a bit lazy with it still being a small PR :D

@breedx-splk
Copy link
Contributor

@anuraaga Thanks for the quick response to my findings with this quick fix! 🏆

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

Thanks! 🤞 this fixes it!

@jkwatson jkwatson merged commit 4916931 into open-telemetry:master Jan 5, 2021
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.

Jaeger (and presumably similar) testcontainers test flaky
4 participants