-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix build on sles with new docker version #36707
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
Conversation
|
Pinging @elastic/es-core-infra |
jasontedor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple of comments.
| } | ||
|
|
||
| protected static void checkDockerVersionRecent(String dockerVersion) { | ||
| final Matcher matcher = dockerVersion =~ /Docker version (\d+\.\d+)\.\d+(?:-ce)?, build [0-9a-f]{7,}/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a SHA-1 hash, it can go up to 40 characters, so can we restrict this to {7,40}?
| } | ||
|
|
||
| @Test(expected = GradleException.class) | ||
| public void checkDockerVersionRecentOl() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the name of this test: checkDockerVersionRecentOl. I have a question about the name of the tests in general. How about testPassingDockerVersions and testFailingDockerVersions?
jasontedor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
@elasticmachine run gradle tests 1 |
|
@elasticmachine run gradle build tests 1 |
1 similar comment
|
@elasticmachine run gradle build tests 1 |
|
@elasticmachine run gradle build tests 1 |
|
I think this should be backported to 6.x and 6.6 too, as the same problem is failing some builds on those branches? Or is there a reason why the fix would need to be different on these branches? |
The build hash for some Docker versions is longer than 7 characters. Closes #36414
The build hash for some Docker versions is longer than 7 characters. Closes #36414
The build hash for this version is longer,
Closes #36414