[VL][CI] Add gpu image centos-9-jdk17-cuda13.1-cudf - #12690
Open
marin-ma wants to merge 2 commits into
Open
Conversation
marin-ma
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a new CentOS Stream 9 CUDA 13.1 cuDF GPU Docker image intended to restore missing dependencies (S3/GCS/ABFS) and moves GPU CI usage toward JDK 17.
Changes:
- Adds a new Dockerfile for
centos-9-jdk17-cuda13.1-cudfthat builds Velox/Gluten with cloud filesystem flags enabled. - Extends the GitHub Actions workflow to build and push the new Docker image tag.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf | Introduces a new CUDA 13.1 cuDF build image definition intended for JDK 17 + cloud filesystem deps. |
| .github/workflows/docker_image.yml | Adds a CI job to build/push the new Docker image tag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+18
to
+33
| FROM quay.io/centos/centos:stream9 | ||
| ENV CUDA_ARCHITECTURES=75 | ||
| ENV LD_LIBRARY_PATH=/opt/gluten/ep/build-velox/build/velox_ep/_build/release/_deps/curl-build/lib:$LD_LIBRARY_PATH | ||
| ENV CC=/opt/rh/gcc-toolset-14/root/bin/gcc \ | ||
| CXX=/opt/rh/gcc-toolset-14/root/bin/g++ | ||
|
|
||
| RUN dnf config-manager --add-repo "https://developer.download.nvidia.com/compute/cuda/repos/rhel9/x86_64/cuda-rhel9.repo"; \ | ||
| dnf update; \ | ||
| dnf install -y sudo patch maven perl git gcc-toolset-14 cuda-toolkit-13-1 && \ | ||
| dnf autoremove -y && dnf clean all; \ | ||
| git clone --depth=1 https://github.com/apache/incubator-gluten /opt/gluten && \ | ||
| cd /opt/gluten && \ | ||
| source /opt/rh/gcc-toolset-14/enable && \ | ||
| bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_abfs=ON --enable_gpu=ON && \ | ||
| rm -rf /opt/gluten && \ | ||
| rm -rf /root/.cache/ccache |
Comment on lines
+24
to
+27
| RUN dnf config-manager --add-repo "https://developer.download.nvidia.com/compute/cuda/repos/rhel9/x86_64/cuda-rhel9.repo"; \ | ||
| dnf update; \ | ||
| dnf install -y sudo patch maven perl git gcc-toolset-14 cuda-toolkit-13-1 && \ | ||
| dnf autoremove -y && dnf clean all; \ |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:37
- Cloning
apache/gluteninside the Docker build makes the image non-reproducible (depends on whatevermainis at build time) and adds an external network dependency during builds. Since the workflow already checks out the repo and builds withcontext: ., prefer using the build context (e.g., copy the local sources) or pass an explicit commit/ref as an ARG to ensure the image contents are deterministic.
git clone --depth=1 https://github.com/apache/gluten /opt/gluten && \
cd /opt/gluten && \
source /opt/rh/gcc-toolset-14/enable && \
bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_abfs=ON --enable_gpu=ON && \
rm -rf /opt/gluten && \
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:36
RUNdefaults to/bin/sh -c; relying onsourcebeing available is shell-dependent. To avoid build failures across base image changes, either use POSIX.instead ofsource, or switch the Dockerfile shell to bash for this layer (e.g., via aSHELLdirective orbash -lc).
source /opt/rh/gcc-toolset-14/enable && \
bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_abfs=ON --enable_gpu=ON && \
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:25
LD_LIBRARY_PATHis set to a path under/opt/gluten/..., but/opt/glutenis removed during the build. This leaves the image with an environment variable referencing a non-existent directory (and can make runtime debugging harder or mask missing libs). Either keep the referenced artifacts, move needed libraries into a stable location, or drop thisLD_LIBRARY_PATHentry.
ENV LD_LIBRARY_PATH=/opt/gluten/ep/build-velox/build/velox_ep/_build/release/_deps/curl-build/lib:$LD_LIBRARY_PATH
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:37
LD_LIBRARY_PATHis set to a path under/opt/gluten/..., but/opt/glutenis removed during the build. This leaves the image with an environment variable referencing a non-existent directory (and can make runtime debugging harder or mask missing libs). Either keep the referenced artifacts, move needed libraries into a stable location, or drop thisLD_LIBRARY_PATHentry.
rm -rf /opt/gluten && \
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:36
- Building tests and benchmarks (
--build_tests=ON --build_benchmarks=ON) during the Docker image build can significantly increase build time and resource usage in CI, especially if the goal is mainly to bake in dependencies. If they aren’t required for the image’s intended use, consider disabling them to reduce CI duration and risk of timeouts.
bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_abfs=ON --enable_gpu=ON && \
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:38
- Trailing whitespace at end of line. Consider trimming to keep diffs clean and avoid whitespace-only churn.
rm -rf /root/.cache/ccache
Comment on lines
+29
to
+31
| cd /opt/gluten && \ | ||
| source /opt/rh/gcc-toolset-14/enable && \ | ||
| bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_abfs=ON --enable_gpu=ON && \ |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Suppressed comments (2)
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:31
dnf config-managertypically comes fromdnf-plugins-coreon RHEL/CentOS 9 minimal images; if it’s not preinstalled inquay.io/centos/centos:stream9, this step will fail immediately. Installdnf-plugins-core(or use an alternative repo-add approach) before invokingdnf config-manager.
RUN dnf config-manager --add-repo "https://developer.download.nvidia.com/compute/cuda/repos/rhel9/x86_64/cuda-rhel9.repo" && \
dnf update -y && \
dnf install -y sudo patch java-${JAVA_VERSION}-openjdk-devel maven perl git gcc-toolset-14 cuda-toolkit-13-1 && \
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:33
- Cloning
apache/glutenwithout pinning a specific ref/commit makes the image build non-reproducible (the resulting image contents can change over time with the same Dockerfile). Consider accepting aGLUTEN_REF/GLUTEN_COMMITbuild arg and checking out that ref, so CI and downstream users can reliably rebuild identical images.
git clone --depth=1 https://github.com/apache/gluten /opt/gluten && \
Comment on lines
+35
to
+36
| source /opt/rh/gcc-toolset-14/enable && \ | ||
| bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_abfs=ON --enable_gpu=ON && \ |
| ENV JAVA_HOME=/usr/lib/jvm/java-${JAVA_VERSION}-openjdk | ||
| ENV PATH=$JAVA_HOME/bin:$PATH | ||
| ENV CUDA_ARCHITECTURES=75 | ||
| ENV LD_LIBRARY_PATH=/opt/gluten/ep/build-velox/build/velox_ep/_build/release/_deps/curl-build/lib:$LD_LIBRARY_PATH |
| cd /opt/gluten && \ | ||
| source /opt/rh/gcc-toolset-14/enable && \ | ||
| bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_abfs=ON --enable_gpu=ON && \ | ||
| rm -rf /opt/gluten && \ |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (3)
dev/builddeps-veloxbe.sh:324
ENABLE_HDFScurrently triggersinstall_adapters, butinstall_adapters(as defined in ep/build-velox/src/setup-rhel.sh) installs S3/GCS/ABFS adapters unconditionally and does not install anything HDFS-specific. This changes behavior vs. the prior per-flag installs: e.g.--enable_hdfs=ON(with S3/GCS/ABFS OFF) will now install all adapter deps unexpectedly, and HDFS still won’t get any dedicated deps.
if [[ "$ENABLE_S3" == "ON" || "$ENABLE_GCS" == "ON" || "$ENABLE_HDFS" == "ON" || "$ENABLE_ABFS" == "ON" ]]; then
install_adapters
fi
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:39
- PR description says the new image should be built with
--enable_s3=ON --enable_gcs=ON --enable_abfs=ON(no mention of HDFS), but this Dockerfile enables HDFS as well. If HDFS isn’t intended here, keeping it ON can unnecessarily expand the dependency surface and build time.
bash ./dev/buildbundle-veloxbe.sh get-velox && \
bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_hdfs=ON --enable_abfs=ON --enable_gpu=ON && \
.github/workflows/docker_image.yml:177
- This workflow adds/pushes the new
centos-9-jdk17-cuda13.1-cudfimage, but existing CI jobs and docs still referenceapache/gluten:centos-9-jdk8-cudf(e.g., velox_backend_x86.yml / velox_backend_cache.yml / VeloxGPU.md). If the intent is to “switch to jdk17 for the gpu build”, those consumers need to be updated in the same PR (or the PR description should clarify this is only introducing the new image without switching CI yet).
context: .
file: dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf
push: true
tags: ${{ env.DOCKERHUB_REPO }}:centos-9-jdk17-cuda13.1-cudf
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:38
- This new image runs
buildbundle-veloxbe.shand then deletes/opt/gluten, but it never installs the cuDF build artifacts into the image (unlikeDockerfile.centos-9-jdk17-cuda12.9-cudf, which doescmake --installfrom_deps/cudf-build). If/opt/glutenis removed, cuDF headers/libs built underep/.../_deps/cudf-buildwon’t be available for subsequent GPU builds using this image.
RUN git clone --depth=1 https://github.com/apache/gluten /opt/gluten && \
cd /opt/gluten && \
source /opt/rh/gcc-toolset-14/enable && \
bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_hdfs=ON --enable_abfs=ON --enable_gpu=ON && \
rm -rf /opt/gluten && \
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:37
- PR description says the new image is built with
--enable_s3=ON --enable_gcs=ON --enable_abfs=ON, but the Dockerfile also enables HDFS (--enable_hdfs=ON). If HDFS isn’t required for the GPU jobs you’re targeting, consider removing it (or update the PR description to match), since it can increase build time and dependency surface area.
bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_hdfs=ON --enable_abfs=ON --enable_gpu=ON && \
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (4)
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:38
- The image deletes /opt/gluten, but it never installs the built cuDF artifacts into the image (unlike the existing cuda12.9 Dockerfile which runs
cmake --installin the cudf-build directory). Afterrm -rf /opt/gluten, the cuDF build outputs will be gone, so downstream GPU jobs may not have the required cuDF libraries available.
RUN git clone --depth=1 https://github.com/apache/gluten /opt/gluten && \
cd /opt/gluten && \
source /opt/rh/gcc-toolset-14/enable && \
bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_hdfs=ON --enable_abfs=ON --enable_gpu=ON && \
rm -rf /opt/gluten && \
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:27
LD_LIBRARY_PATHis set to a path under/opt/gluten/..., but this Dockerfile removes/opt/glutenlater. Leaving a non-existent directory inLD_LIBRARY_PATHcan cause confusing runtime/linker behavior; if these libraries are needed, they should be installed to a stable location (e.g. /usr/local/lib) instead.
ENV CUDA_ARCHITECTURES=75
ENV LD_LIBRARY_PATH=/opt/gluten/ep/build-velox/build/velox_ep/_build/release/_deps/curl-build/lib:$LD_LIBRARY_PATH
ENV CC=/opt/rh/gcc-toolset-14/root/bin/gcc \
CXX=/opt/rh/gcc-toolset-14/root/bin/g++
dev/builddeps-veloxbe.sh:324
install_adapters(as defined in ep/build-velox/src/setup-rhel.sh) unconditionally installs S3+GCS+ABFS dependencies. Switching to it changes behavior vs the previous per-flag installs: enabling only one adapter (or just HDFS) will still install all adapter dependencies, increasing build time and introducing extra failure surface. It also drops theAZURE_SDK_DISABLE_AUTO_VCPKGexport that was previously applied for ABFS.
if [[ "$ENABLE_S3" == "ON" || "$ENABLE_GCS" == "ON" || "$ENABLE_HDFS" == "ON" || "$ENABLE_ABFS" == "ON" ]]; then
install_adapters
fi
dev/build-helper-functions.sh:238
- This adds an unconditional call to
scripts/setup-centos-adapters.shfor all CentOS dependency setups. That makes CentOS setup depend on an extra script existing in the Velox checkout and runs adapter installation even when adapters are disabled (setup_dependencies already gates adapter installation). This can break CentOS builds if the Velox branch doesn’t ship that script, and adds unnecessary work for non-adapter builds.
elif [[ "$LINUX_DISTRIBUTION" == "centos" ]]; then
scripts/setup-centos-adapters.sh
case "$LINUX_VERSION_ID" in
marin-ma
marked this pull request as draft
August 7, 2026 09:51
marin-ma
force-pushed
the
add-jdk17-cudf13-image
branch
from
August 10, 2026 10:54
30fc666 to
6157b13
Compare
marin-ma
force-pushed
the
add-jdk17-cudf13-image
branch
from
August 10, 2026 10:54
6157b13 to
2269adb
Compare
Member
|
should we also remove the old jdk8 based image? https://github.com/apache/gluten/blob/main/.github/workflows/docker_image.yml#L61 |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
dev/builddeps-veloxbe.sh:324
- This change removes the
AZURE_SDK_DISABLE_AUTO_VCPKG=ONexport that previously ran when--enable_abfs=ON. There are no remaining references in the repo, so ABFS dependency install may regress (Azure SDK’s build can re-enable vcpkg auto mode). Re-introduce the export when ABFS is enabled before running the adapter installer.
if [[ "$ENABLE_S3" == "ON" || "$ENABLE_GCS" == "ON" || "$ENABLE_HDFS" == "ON" || "$ENABLE_ABFS" == "ON" ]]; then
install_adapters
fi
| if [[ "$LINUX_DISTRIBUTION" == "ubuntu" || "$LINUX_DISTRIBUTION" == "debian" || "$LINUX_DISTRIBUTION" == "pop" ]]; then | ||
| scripts/setup-ubuntu.sh | ||
| elif [[ "$LINUX_DISTRIBUTION" == "centos" ]]; then | ||
| source scripts/setup-centos-adapters.sh |
Comment on lines
+39
to
+41
| RUN source /opt/rh/gcc-toolset-14/enable && \ | ||
| bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_hdfs=ON --enable_abfs=ON --enable_gpu=ON build_arrow && \ | ||
| rm -rf /opt/gluten /root/.cache/ccache |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (3)
dev/docker/cudf/Dockerfile.centos-9-jdk17-cuda13.1-cudf:41
- The
buildbundle-veloxbe.shinvocation appends a positionalbuild_arrowargument. Becausebuildbundle-veloxbe.shsourcesbuilddeps-veloxbe.sh, that extra argument is treated as a command override and can prevent the normal Velox/Gluten build flow from running. Also, this Dockerfile enables HDFS even though the PR description only mentions enabling S3/GCS/ABFS for missing dependencies.
RUN source /opt/rh/gcc-toolset-14/enable && \
bash ./dev/buildbundle-veloxbe.sh --run_setup_script=ON --build_arrow=ON --spark_version=3.5 --build_tests=ON --build_benchmarks=ON --enable_s3=ON --enable_gcs=ON --enable_hdfs=ON --enable_abfs=ON --enable_gpu=ON build_arrow && \
rm -rf /opt/gluten /root/.cache/ccache
dev/build-helper-functions.sh:238
setup_linuxsourcesscripts/setup-centos-adapters.sh, but that file does not exist in this repo (andget-velox.shdoesn’t create/copy it). This will make CentOS dependency setup fail at runtime with a "No such file" error.
elif [[ "$LINUX_DISTRIBUTION" == "centos" ]]; then
source scripts/setup-centos-adapters.sh
case "$LINUX_VERSION_ID" in
dev/builddeps-veloxbe.sh:324
setup_dependenciesnow callsinstall_adapterswhenever any of S3/GCS/HDFS/ABFS is enabled. This is a behavior change from the previous per-adapter installs and can also fail ifinstall_adaptersisn't defined in the sourced environment. It also drops the ABFS-specificAZURE_SDK_DISABLE_AUTO_VCPKG=ONexport that was previously set.
if [[ "$ENABLE_S3" == "ON" || "$ENABLE_GCS" == "ON" || "$ENABLE_HDFS" == "ON" || "$ENABLE_ABFS" == "ON" ]]; then
install_adapters
fi
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The jdk version in the gpu image
apache/gluten:centos-9-jdk8-cudfis now jdk17 after #11835, and some dependencies such as aws-sdk-cpp are missing in the image because it's no longer based on the velox image. Some gpu jobs that rely on theapache/gluten:centos-9-jdk8-cudfimage have failed due to this change.This pr adds a new gpu image
centos-9-jdk17-cuda13.1-cudfwith build flags--enable_s3=ON --enable_gcs=ON --enable_abfs=ONso the essential dependencies will be installed. Keeping the jdk8 version seems unnecessary. We can switch to jdk17 for the gpu build.