Skip to content

Prefetch Ryuk image with retries#13639

Closed
findepi wants to merge 1 commit intotrinodb:masterfrom
findepi:findepi/prefetch-ryuk-image-with-retries-214c8d
Closed

Prefetch Ryuk image with retries#13639
findepi wants to merge 1 commit intotrinodb:masterfrom
findepi:findepi/prefetch-ryuk-image-with-retries-214c8d

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Aug 12, 2022

According to https://nineinchnick.github.io/trino-cicd/reports/flaky/,
HTTP 500 when getting testcontainers/ryuk is not uncommon source of
test flakiness. Let's try to improve the situation by fetching the image
before tests are started.

@findepi findepi added test no-release-notes This pull request does not require release notes entry labels Aug 12, 2022
@cla-bot cla-bot bot added the cla-signed label Aug 12, 2022
@findepi findepi force-pushed the findepi/prefetch-ryuk-image-with-retries-214c8d branch from 26b4844 to d79a5dc Compare August 12, 2022 11:21
@findepi findepi requested a review from nineinchnick August 12, 2022 11:21
According to https://nineinchnick.github.io/trino-cicd/reports/flaky/,
HTTP 500 when getting `testcontainers/ryuk` is not uncommon source of
test flakiness. Let's try to improve the situation by fetching the image
before tests are started.
@findepi findepi force-pushed the findepi/prefetch-ryuk-image-with-retries-214c8d branch from d79a5dc to b5f7dff Compare August 12, 2022 11:22
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@nineinchnick how i should add this to ci.yml?
where else i should add this?

PTs for sure, others?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IIRC @nineinchnick is working on a change that will extract common code to separate step extracted as separate file. Then it will be easy to add such changes

@@ -458,6 +458,9 @@ jobs:
run: |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change is not going to help:(

We blame ryuk because it is downloaded first. I disabled downloading ryuk in the past and it was still failing on other images.

If we could pre-download images before running tests then we would avoid flakiness of docker registries. Cc @ppalucha

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We blame ryuk because it is downloaded first.

you're right.

I disabled downloading ryuk in the past and it was still failing on other images.

Maybe if we retry long enough to download ryuk we cover for the short unavailability of the docker hub.
So it depends on the unavailability pattern (total vs % requests dropped).

Also, wonder whether testcontainers library does / can do retries for all images it downloads.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Also, wonder whether testcontainers library does / can do retries for all images it downloads.

testcontainers/testcontainers-java#5760

@findepi findepi closed this Aug 24, 2022
@findepi findepi deleted the findepi/prefetch-ryuk-image-with-retries-214c8d branch August 24, 2022 08:49
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Aug 24, 2022

Superseded by #13774

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed no-release-notes This pull request does not require release notes entry test

Development

Successfully merging this pull request may close these issues.

3 participants