-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat: add mysql 8.4 docker images #17529
feat: add mysql 8.4 docker images #17529
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
ac13068
to
4a50e03
Compare
4a50e03
to
9dd14c1
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17529 +/- ##
=======================================
Coverage 67.68% 67.69%
=======================================
Files 1584 1584
Lines 254630 254720 +90
=======================================
+ Hits 172356 172422 +66
- Misses 82274 82298 +24 ☔ View full report in Codecov by Sentry. |
9dd14c1
to
3d9e219
Compare
3d9e219
to
54712fa
Compare
3e1d7f9
to
24589d6
Compare
30db9c3
to
f6a5cf8
Compare
Signed-off-by: Leopold Jacquot <leopold.jacquot@infomaniak.com>
f6a5cf8
to
4f7dfd1
Compare
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.
One question for @frouioui, otherwise looks good to me.
@@ -162,3 +162,4 @@ List of changes between bootstrap image versions. | |||
## [41] - 2025-01-15 | |||
### Changes | |||
- Update base image to bookworm | |||
- Add MySQL84 image |
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.
@frouioui Do we need to bump this to 42 now that we add this new image?
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.
For me we don't as we add a new image and we are not editing a new one, but it's only my opinion.
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.
Yeah I don't think we should bump it to 42. We will just build the new (mysql84) tag for 41.
I have built and pushed the new |
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.
The docker build looks good to me:
root@vitess-testing:~/vitess# make docker_lite_mysql84
Building vitess/lite:mysql84
...
...
=> => exporting layers 6.1s
=> => writing image sha256:34c5e32d982a5165e23d775284b1f89118244d522f8e9f49b701465849a60781 0.0s
=> => naming to docker.io/vitess/lite:mysql84 0.0s
root@vitess-testing:~/vitess#
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.
Looks good to me! Thanks a bunch for working on this @L3o-pold ❤️
Before merging, quick question, were you able to verify that the new image fixes the bug you described in #17519? Edit: It should be okay, but may as well double check.
|
I don't, we rollbacked to 8.0 as 8.4 is experimental in Vitess |
It looks like this has the wrong version of Golang, causing builds to fail. See #17601. |
Signed-off-by: Leopold Jacquot <leopold.jacquot@infomaniak.com> Signed-off-by: garfthoffman <109185460+garfthoffman@users.noreply.github.com>
Description
Add MySQL8.4 docker images
Related Issue(s)
Fixes #17519
Checklist
Deployment Notes