Repository navigation
Group clib, Java, Lucene, Rust, and Go CI jobs as matrix sub-jobs and split Java/Lucene build and test - #2761
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
946d5ee to
40ad88c
Compare
|
/ok to test 40ad88c |
|
/ok to test aa7c319 |
bdice
added
improvement
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe pull request adds reusable workflows for C builds and matrix jobs, then routes build and test workflows through them. Java and Lucene tests now use downloaded build artifacts. Build scripts update artifact naming and arguments, and standalone C tests derive artifact names from the CUDA version and host architecture. ChangesCI Workflow and Artifact Changes
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to This change reorganizes CI so that Java and Lucene are built once on amd64 and tested on every architecture, and it groups the other jobs as matrix sub-jobs. No concrete defects were identified, so it appears ready to merge once its CI checks pass. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
bdice
changed the title
| with: | ||
| build_type: ${{ inputs.build_type }} | ||
| matrix_name: conda-cpp-build | ||
| build: |
There was a problem hiding this comment.
This is the standalone C tarball build that used to be written out inline in both pr.yaml and build.yaml. The steps are unchanged. It runs directly on the host runner (./build.sh tarball on cpu16), not in a CI container, so it can't use custom-job.yaml like the other jobs. The workflow computes its own conda-cpp-build matrix, so each caller is a single job, and the matrix entries appear as sub-jobs under it. The branch/date/sha inputs and the rapids-github-info step come from the build.yaml copy; they're empty on PR runs.
| with: | ||
| build_type: ${{ inputs.build_type }} | ||
| matrix_name: conda-cpp-build | ||
| matrix_filter: 'map(. + {GPU: (if .ARCH == "amd64" then "rtxpro6000" else "l4" end), DRIVER: "latest"}) | ${{ inputs.matrix_filter }}' |
There was a problem hiding this comment.
The default runner for each entry is rtxpro6000 on amd64 and l4 otherwise, with the latest driver. These are the GPUs the old jobs used, so most callers don't set a matrix_filter. Callers that need only amd64 (Rust, Go, the Java/Lucene builds) filter with map(select(.ARCH == "amd64")).
| build_type: ${{ inputs.build_type }} | ||
| matrix_name: conda-cpp-build | ||
| matrix_filter: 'map(. + {GPU: (if .ARCH == "amd64" then "rtxpro6000" else "l4" end), DRIVER: "latest"}) | ${{ inputs.matrix_filter }}' | ||
| run: |
There was a problem hiding this comment.
The name gives readable check names, e.g. conda-java-tests / 12.9.2, 3.11, arm64, rockylinux8, l4, latest-driver / build. The trailing / build is custom-job's job ID. CPU jobs (node_type set) leave out the GPU and driver.
| strategy: | ||
| fail-fast: false | ||
| matrix: ${{ fromJSON(needs.compute-matrix.outputs.matrix) }} | ||
| uses: rapidsai/shared-workflows/.github/workflows/custom-job.yaml@main |
There was a problem hiding this comment.
Each matrix entry calls the shared custom-job.yaml instead of a copy of its steps, so changes in shared-workflows (action pins, proxy cache, telemetry and so on) reach these jobs automatically. Because the matrix lives in this wrapper, the inputs below can use ${{ matrix.* }} directly. custom-job reads secrets only through optional inputs (alternative-gh-token-secret-name, sccache-dist-token-secret-name) that we don't set, so callers don't need secrets: inherit.
| date: ${{ inputs.date }} | ||
| sha: ${{ inputs.sha }} | ||
| arch: ${{ matrix.ARCH }} | ||
| node_type: ${{ inputs.node_type || format('gpu-{0}-{1}-1', matrix.GPU, matrix.DRIVER) }} |
There was a problem hiding this comment.
GPU jobs get gpu-<GPU>-<DRIVER>-1 from the matrix entry. The Java/Lucene builds pass node_type: cpu8 instead. Without --run-java-tests, the Maven build passes -DskipTests, so it never needs a GPU; build.yaml already ran these builds on cpu8.
| # amd64; this verifies that the resulting jextract-generated Panama bindings also work | ||
| # correctly against a native libcuvs_c.so on this host's architecture. | ||
|
|
||
| CUVS_JAVA_ARTIFACT="cuvs-java-cuda${RAPIDS_CUDA_VERSION}" |
There was a problem hiding this comment.
This is the former ci/test_java_prebuilt.sh, renamed to fit the build_*.sh / test_*.sh convention. It tests a prebuilt artifact, like test_python.sh and test_cpp.sh. The old build-and-test test_java.sh is removed. The script now takes no arguments: the artifact name comes from RAPIDS_CUDA_VERSION, which the CI images set, and the dependency matrix uses $(arch), as build_java.sh does. The rename also means the existing !ci/test_java.sh changed-files exclusions in pr.yaml now cover the script that actually runs.
| # this doubles as a cross-arch check that the jextract-generated Panama bindings baked | ||
| # into the amd64 jar work unmodified against a native libcuvs_c.so on that architecture. | ||
|
|
||
| CUVS_JAVA_ARTIFACT="cuvs-java-cuda${RAPIDS_CUDA_VERSION}" |
There was a problem hiding this comment.
Same as test_java.sh: the former test_lucene_prebuilt.sh, with no arguments.
| # TODO: Remove this argument-handling when build and test workflows are separated, | ||
| # and test_java.sh no longer calls build_java.sh | ||
| # ref: https://github.com/nvidia/cuvs/issues/868 | ||
| EXTRA_BUILD_ARGS=("--build-java-examples") |
There was a problem hiding this comment.
Removes the --run-java-tests handling, as the TODO here asked (#868), because CI no longer builds and tests in one job. The build.sh --run-java-tests flag for local development is unchanged.
| echo "Error: name of the cuvs-java artifact is missing" >&2 | ||
| exit 1 | ||
| fi | ||
| CUVS_JAVA_ARTIFACT="cuvs-java-cuda${RAPIDS_CUDA_VERSION}" |
There was a problem hiding this comment.
With --run-java-tests gone, the script takes no arguments. The cuvs-java artifact name comes from RAPIDS_CUDA_VERSION, matching what matrix-job.yaml uploads.
| fi | ||
|
|
||
| payload_name="$1" | ||
| case "$(arch)" in |
There was a problem hiding this comment.
The clib tarball artifact is named libcuvs_c_<CUDA_VER>_<ARCH>.tar.gz, with amd64/arm64 from the matrix. None of the images has an environment variable with those spellings, so this maps $(arch) to them. That keeps the artifact name unchanged.
|
/merge |
3 participants
Restructures the clib, Java, Lucene, Rust, and Go CI jobs in
pr.yaml,build.yaml, andtest.yamlso each is a single job whose matrix entries appear as sub-jobs, following NVIDIA/cudf#24327. Previously each of these was a separate*-matrixjob plus a caller job that defined the matrix, so every matrix entry showed up as its own top-level check.Reusable workflows
.github/workflows/clib-build.yaml: the standalone C tarball build, previously written out inline in bothpr.yamlandbuild.yaml. It runs on the host runner, so it keeps its own steps..github/workflows/matrix-job.yaml: computes theconda-cpp-buildmatrix and callsrapidsai/shared-workflows/.github/workflows/custom-job.yaml@mainonce per entry, so the job steps stay in shared-workflows instead of being copied here. Inside the wrapper,custom-jobinputs use${{ matrix.* }}directly:rtxpro6000on amd64 andl4otherwise, with the latest driver. CPU jobs passnode_type, e.g. the Java/Lucene builds usecpu8.<container-image-repository>:26.12-cuda<CUDA_VER>-<LINUX_VER>-py<PY_VER>, with the repository defaulting torapidsai/ci-conda.ci/release/update-version.shstill bumps the tag.<artifact-name-prefix>-cuda<CUDA_VER>when a prefix is given.custom-jobreads secrets only through optional inputs we don't set, so callers no longer usesecrets: inherit.Java and Lucene: separate build and test
Previously
ci/test_java.sh/ci/test_lucene.shrebuilt and tested on amd64, and*-other-archjobs tested the amd64-built artifacts on arm64 (ci/test_*_prebuilt.sh). Now (closes the TODOs referencing #868):conda-java-build/conda-lucene-build(PR) andjava-build/lucene-build(build.yaml) build once on amd64 CPU runners, whichbuild.yamlalready used.conda-java-tests/conda-lucene-teststest those artifacts without recompiling, on every architecture, amd64 included. The published jar is the amd64 build, so this checks that its jextract-generated Panama bindings work everywhere.test.yaml, the nightly test jobs no longer build.rapids-download-from-githubfetches the artifacts from thebuild.yamlrun for the same commit, as the old other-arch jobs already did.ci/test_java_prebuilt.shandci/test_lucene_prebuilt.share renamed toci/test_java.shandci/test_lucene.sh, replacing the old build-and-test scripts. This also makes the existing!ci/test_java.sh/!ci/test_lucene.shchanged-files exclusions apply to the scripts that run.ci/build_java.sh/ci/build_lucene.shdrop--run-java-testshandling. Thebuild.sh --run-java-testsflag for local development is unchanged.*-other-archjobs'release/26.10pins and 26.10 container images are gone. All jobs use@mainand 26.12 images.Scripts no longer take matrix arguments
ci/test_java.sh,ci/test_lucene.sh,ci/build_lucene.sh, andci/test_standalone_c.shderive artifact names fromRAPIDS_CUDA_VERSION(and$(arch)for the clib tarball'samd64/arm64suffix). Artifact names are unchanged.Other cleanups
rocky8-clib-testsno longer passesdate: ${{ inputs.date }}_candsha: ${{ inputs.sha }}.pr.yamlhas no such inputs.build.yaml's Rust matrix was computed withbuild_type: pull-request. It now uses the run's build type. The matrices are identical today.nv-gha-runners/*actions (get-pr-infoinpr.yaml) are used at@main, as in shared-workflows..github/zizmor.ymlnow allows refs fornv-gha-runners/*, as it already did forrapidsai/shared-workflowsandrapidsai/shared-actions.PR check names
rocky8-clib-standalone-build (amd64, 3.11, 12.9.2, rockylinux8)rocky8-clib-standalone-build / 12.9.2, 3.11, amd64, rockylinux8rocky8-clib-tests (amd64, 3.11, 12.9.2, rockylinux8)rocky8-clib-tests / 12.9.2, 3.11, amd64, rockylinux8, rtxpro6000, latest-driver / buildconda-java-build-and-tests (amd64, 3.11, 12.9.2, rockylinux8)conda-java-build / 12.9.2, 3.11, amd64, rockylinux8 / buildconda-java-tests / 12.9.2, 3.11, amd64, rockylinux8, rtxpro6000, latest-driver / buildconda-java-tests-other-arch (arm64, 3.11, 12.9.2, rockylinux8)conda-java-tests / 12.9.2, 3.11, arm64, rockylinux8, l4, latest-driver / buildconda-lucene-build-and-tests (amd64, 3.11, 12.9.2, rockylinux8)conda-lucene-build / 12.9.2, 3.11, amd64, rockylinux8 / buildconda-lucene-tests / 12.9.2, 3.11, amd64, rockylinux8, rtxpro6000, latest-driver / buildconda-lucene-tests-other-arch (arm64, 3.11, 12.9.2, rockylinux8)conda-lucene-tests / 12.9.2, 3.11, arm64, rockylinux8, l4, latest-driver / buildrust-build (amd64, 3.11, 12.9.2, rockylinux8)rust-build / 12.9.2, 3.11, amd64, rockylinux8, rtxpro6000, latest-driver / buildgo-build (amd64, 3.11, 12.9.2, rockylinux8)go-build / 12.9.2, 3.11, amd64, rockylinux8, rtxpro6000, latest-driver / buildRows show CUDA 12.9.2; each job also has a CUDA 13.3.0 entry. The clib jobs also have arm64 entries, as before. The
*-matrixjobs are gone frompr-builder.Runners
The Java and Lucene PR builds move from RTX PRO 6000 to
cpu8runners, matchingbuild.yaml. All other jobs keep their previous runners: clib buildcpu16; clib, Java, and Lucene tests on RTX PRO 6000 (amd64) or L4 (arm64); Rust and Go on RTX PRO 6000. Rust and Go stay single GPU jobs: each takes ~4–7 minutes, mostly conda environment setup, so splitting build and test would repeat that setup without saving GPU time.