-
Notifications
You must be signed in to change notification settings - Fork 198
Update dockerfile dependencies #4470
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |
| # plus mysqld-debug, then route only the final foreground server to mysqld-debug. The stock | ||
| # entrypoint's config checks, datadir initialization, and temporary init server keep using the | ||
| # release binary because the debug binary can trip InnoDB assertions on that path. | ||
| ARG MYSQL_VERSION=8.0.46 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug (build-breaking): Bumping to 8.4.9 here will break the Docker build. Lines 26-28 (not shown in diff) hardcode the yum repo to mysql-8.0-community. When the base image is 8.4, line 23 extracts an 8.4.x RPM version, then lines 33-41 try to install those packages from the 8.0 repo -- which does not carry them. The repo config (lines 26-28) needs to be updated to mysql-8.4-lts-community. |
||
| ARG MYSQL_VERSION=8.4.9@sha256:c36050afdca850f23cef85703f84c7531a5ae155a11b5ee1c60acb09937c4084 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incomplete version bump: The ARG default is overridden by the bake file (docker-bake.hcl line 2) and workflow (mysql-debug-docker.yml lines 9, 24), both still at 8.0.46. The e2e test (clickhouse_mysql_test.go:1491) also references mysql-debug:8.0.46. This Dockerfile change is a no-op in CI. To complete the bump, also update: (1) docker-bake.hcl line 2, (2) mysql-debug-docker.yml lines 9 and 24, (3) clickhouse_mysql_test.go line 1491. Also update the yum repo URL on line 28 from mysql-8.0-community to mysql-8.4-lts-community. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High severity -- inconsistent version bump with latent breakage Renovate updated only the Dockerfile ARG default from 8.0.46 to 8.4.9, but three other files still hardcode 8.0.46:
Since CI passes the bake/workflow default as a build arg, the actual Docker build still uses mysql:8.0.46 -- so this Dockerfile change is effectively a no-op today. However, it creates a confusing inconsistency. More importantly, if someone later aligns the other files to 8.4, two things break:
Recommendation: Either revert the MySQL portion of this PR (keep the Dockerfile at 8.0.46 to match the other files), or commit to a full 8.4 migration that also updates the bake file, workflow, test references, repo URL, and validates e2e test compatibility. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug (build-breaking): Bumping to MySQL 8.4.9 will break the Docker build. Lines 26-28 below hardcode the yum repo to MySQL 8.4 RPMs are published under a different repository path (e.g. The fix should update the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Build breakage: The MySQL version jumps from 8.0 to 8.4, but the yum repo URL on line 29 is still hardcoded to When building with the new default The repo URL needs to change to (Verified that Additionally, the CI workflow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: hardcoded MySQL 8.0 yum repo will break with 8.4 base image Lines 26-28 define a yum repo pointing to With the base image now The repo name and URL need to be updated to match MySQL 8.4, e.g. Additionally, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug (build-breaking): The base image is bumped from MySQL 8.0.46 to 8.4.9, but lines 26-28 below still hardcode the MySQL 8.0 yum repo: This will break the Docker build. Line 23 extracts the RPM version from Fix: Update lines 26-28 to point at the MySQL 8.4 LTS repo: Also note that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High severity — build will break when this default is actually used. Bumping the base image to
Additionally, MySQL 8.4 has notable behavioral changes relevant to CDC usage:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: MySQL 8.0 to 8.4 creates build-breaking inconsistency This bumps the Dockerfile default from 8.0.46 to 8.4.9, but several related files were not updated:
If the intent is to stay on MySQL 8.0 for the debug image (which seems likely given the 8.0-specific repo configuration), this Dockerfile change should be reverted. If upgrading to 8.4, several additional changes are needed:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug (build-breaking): The MySQL version jumps from 8.0 to 8.4, but the yum repo configured below (line 29) is still hardcoded to The build extracts the RPM version from the 8.4.9 base image The repo URL needs to be updated to the MySQL 8.4 LTS repo (likely Additionally, MySQL 8.4 has significant breaking changes vs 8.0 worth verifying for e2e tests:
The PeerDB Go CDC connector code is already 8.4-compatible ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug (build-breaking): The yum repo configured on lines 26-32 still points to the MySQL 8.0 community repo ( The build extracts the RPM version from The repo config needs to be updated to point to the 8.4 repo:
(Or better yet, derive the repo path from the major.minor version dynamically so future bumps do not hit this again.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: version bump is a no-op. This ARG default is always overridden at build time. See the PR comment for details on which files need matching updates. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: yum repo URL at line 28 still points to mysql-8.0-community after this bump to MySQL 8.4 With this change, mysql_rpm_version (line 23) will resolve to an 8.4.x RPM version string. But the yum repo configured at line 28 points to mysql-8.0-community, which only hosts 8.0.x packages. The microdnf install at lines 34-41 will fail because packages like mysql-community-server-debug-8.4.9 don't exist in the 8.0 repo. The repo section (lines 26-31) needs to be updated for 8.4. The MySQL 8.4 LTS repo path is mysql-8.4-lts-community:
Additionally: MySQL 8.0 to 8.4 is a significant version jump with CDC-relevant changes:
The PeerDB connector code already handles 8.4 properly (version-aware SQL for SHOW BINARY LOG STATUS, SHOW REPLICA STATUS, binlog_expire_logs_seconds), so the main concerns are: (1) this Dockerfile build breakage, and (2) potential test auth issues from mysql_native_password being off by default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MySQL 8.0 to 8.4 bump is inconsistent with surrounding files and will break standalone builds. This ARG default was bumped to 8.4.9, but three other files still reference 8.0.46:
Because docker-bake.hcl overrides MYSQL_VERSION at build time, CI still builds with 8.0.46 -- this Dockerfile change is effectively a no-op in the pipeline. Worse, if someone builds the Dockerfile standalone (using this new default), it will fail: the yum repo URL on line 28 is hardcoded to mysql-8.0-community, which does not carry 8.4 packages. The RPM version extracted from the 8.4 base image will not be found in that repo. If an upgrade to MySQL 8.4 is intended, all four files need updating and the yum repo URL needs to change to the 8.4-lts-community path. PeerDB Go code already handles 8.4 (SHOW BINARY LOG STATUS, SHOW REPLICA STATUS, binlog_expire_logs_seconds), so the application side is ready -- it is only the build infrastructure that needs alignment. If the intent is to stay on 8.0 for now, this Dockerfile change should be reverted or Renovate should be configured to pin MySQL to 8.0.x. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Build-breaking: This bumps the base image to MySQL 8.4.9, but the yum repo URL on line 29 is still hardcoded to mysql-8.0-community. The build extracts the RPM version from the 8.4.9 base image (line 23), then tries to install mysql-community-server at that version from the 8.0 repo -- but 8.4.x packages do not exist there. The correct repo for MySQL 8.4 is mysql-8.4-community. The repo name (line 27) and section header (line 26) also need updating. Additionally, docker-bake.hcl line 2 still defaults to 8.0.46, and clickhouse_mysql_test.go line 1738 still references the 8.0.46 image tag. |
||
| FROM mysql:${MYSQL_VERSION} | ||
| ARG MYSQL_VERSION | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| # syntax=docker/dockerfile:1.24@sha256:87999aa3d42bdc6bea60565083ee17e86d1f3339802f543c0d03998580f9cb89 | ||
| # syntax=docker/dockerfile:1.25@sha256:0adf442eae370b6087e08edc7c50b552d80ddf261576f4ebd6421006b2461f12 | ||
|
|
||
| FROM golang:1.26-alpine@sha256:f1ddd9fe14fffc091dd98cb4bfa999f32c5fc77d2f2305ea9f0e2595c5437c14 AS builder | ||
| FROM golang:1.26-alpine@sha256:3ad57304ad93bbec8548a0437ad9e06a455660655d9af011d58b993f6f615648 AS builder | ||
| # Allow build flags to be passed in at build time, for example debug flags | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. golang digest + Alpine 3.23->3.24: Low risk. Golang is same version (1.26), new digest only. Alpine 3.24 has no breaking changes for PeerDB packages (ca-certificates, geos, openssl, postgresql-client, curl). Looks good. |
||
| ARG DEBUG_BUILD | ||
| ENV DEBUG_BUILD=${DEBUG_BUILD} | ||
|
|
@@ -30,7 +30,7 @@ RUN --mount=type=cache,target="/root/.cache/go-build" if [[ "$DEBUG_BUILD" = "1" | |
| go install github.com/go-delve/delve/cmd/dlv@latest; \ | ||
| fi | ||
|
|
||
| FROM alpine:3.23@sha256:5b10f432ef3da1b8d4c7eb6c487f2f5a8f096bc91145e68878dd4a5019afde11 AS flow-base | ||
| FROM alpine:3.24@sha256:28bd5fe8b56d1bd048e5babf5b10710ebe0bae67db86916198a6eec434943f8b AS flow-base | ||
| ENV TZ=UTC | ||
| ADD --checksum=sha256:e5bb2084ccf45087bda1c9bffdea0eb15ee67f0b91646106e466714f9de3c7e3 https://truststore.pki.rds.amazonaws.com/global/global-bundle.pem /usr/local/share/ca-certificates/global-aws-rds-bundle.pem | ||
| RUN apk add --no-cache ca-certificates geos && \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,7 +41,7 @@ RUN --mount=type=cache,target=/usr/local/cargo/registry \ | |
| mkdir -p /root/target && \ | ||
| cp target/${BUILD_MODE}/peerdb-server /root/target/ | ||
|
|
||
| FROM alpine:3.23@sha256:5b10f432ef3da1b8d4c7eb6c487f2f5a8f096bc91145e68878dd4a5019afde11 | ||
| FROM alpine:3.24@sha256:28bd5fe8b56d1bd048e5babf5b10710ebe0bae67db86916198a6eec434943f8b | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alpine 3.23->3.24: Low risk. Same assessment as flow.Dockerfile -- no breaking changes for packages used here (ca-certificates, postgresql-client, curl, iputils). Looks good. |
||
| ENV TZ=UTC | ||
| RUN apk add --no-cache ca-certificates postgresql-client curl iputils && \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug (build-breaking): Alpine 3.24 removed the unversioned This |
||
| adduser -s /bin/sh -D peerdb && \ | ||
|
|
||
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.
Build-breaking: The yum repo URL on lines 26-28 is hardcoded to
mysql-8.0-community:But the base image is now
mysql:8.4.9. When the Dockerfile queries the RPM version from the 8.4.9 base image (line 23) and then tries to install packages at that version from the 8.0 repo, it will fail — themysql-8.0-communityrepo only contains 8.0.x packages (up to 8.0.45).The repo needs to be updated to
mysql-8.4-community(confirmed it exists athttps://repo.mysql.com/yum/mysql-8.4-community/el/9/x86_64/with debug packages through 8.4.8).Suggested fix:
Additionally, the 8.4.9 debug RPM may not yet be published in the repo (latest available is 8.4.8 as of this review). You may want to pin to
8.4.8until 8.4.9 packages land, or verify the 8.4.9 RPMs are available before merging.