Repository navigation
CI: Jenkins integration testing (do not merge) - #12094
Alexey-Rivkin wants to merge 155 commits into
Conversation
Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Gtest failures when running in k8s env, as unlimites max_threads cause resource exhaustion. Setting the CPU affinity will limit max_threads to 2 dynamically. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Run RoCE first, then IB. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
RoCE on Blossom hits a crash in rdma_get_cm_event during test_ucp_sockaddr.ep_query that doesn't reproduce on Azure RoCE. Land IB (CX8) coverage first; RoCE will return in a separate PR after the rdmacm path is triaged. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
Run RoCE first, then IB. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
PR openucx#11161 introduced an adaptive NPROC cap in common.sh to avoid OOM on k8s nodes where `nproc` reports the full host. Two Jenkins-invoked spots still called `nproc` directly; switch them to ${NPROC} so they honor the cap. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
The OSS dispatcher already aborts the prior dispatcher build for the same PR, but the 8 leaf jobs it spawned via `build job: ..., wait: true` keep running until they finish naturally. That ties up build executors every time someone pushes a new commit to a PR. Port the pattern NIXL uses: scrape each stale dispatcher's console log for `Starting building: <name> #<num>` lines to find its children, kill the leaves first so the dispatcher's wait unblocks, then kill the dispatcher. Use `doKill()` instead of `doStop()` for a hard stop, and retry up to 3 times with a 5s gap so builds caught mid-startup still get torn down. Failures inside the abort block are still swallowed and logged - we never want stale cleanup to break a fresh build.
Lets the stale-build cleanup in Jenkinsfile.dispatcher be tested on the live dispatcher before it merges. Revert to master once validated.
|
🤖 Starting review — findings will be posted here when done. |
|
🤖 CI Triage Agent — TL;DR: Build #48 was manually cancelled by a user ~24 s into the "Checkout source code" stage — there is no product or test failure to fix; just re-run the build. Full analysisSummary: The only stage in the pipeline, "Checkout source code", ended in state Root cause: Not a code defect. The whole-build log shows the explicit marker Implicated commit: unknown — commit File: unknown — no UCX source file was reached (failure occurred in Suggested fix: No code change required. Re-trigger Related: PR #12094 (branch
|
|
🤖 CI Triage Agent — TL;DR: The build didn't fail on a code or test problem — it was manually aborted by the user (arivkin@nvidia.com) ~14 seconds into Full analysisSummary: Root cause: Human cancellation, not a product or CI-script defect. The whole-build log contains, at 09:25:02:
Timestamp analysis rules out a hang or timeout: the stage started at 09:24:49/09:24:58 and the interrupt arrived at 09:25:02 — ~4–14 s of execution with continuous, gap-free output (module loads → The Implicated commit: none — [REDACTED:Hex High Entropy String] (branch File: no source file at fault; abort surfaced at Suggested fix:
Related: PR #12094 (openucx/ucx, the "do not merge" Jenkins integration branch under test); umbrella drafts referenced by it: #11173, #11471, #11469, #11491. Last functional changes to the Coverity matrix: [REDACTED:Hex High Entropy String] "CI: reduce CPU requests in all CI pipelines (#11494)" (NirWolfer) and a346463 "CI: Jenkins/Gitlab integration - phase 1 (#11113)" (Alexey Rivkin) — neither is implicated here.
|
|
🤖 CI Triage Agent — TL;DR: This build wasn't a real test/code failure — it was manually aborted by a human ("Aborted by Alexey Rivkin") ~13 s into the Full analysisSummary: Root cause: Manual pipeline cancellation, not a product or test defect. Evidence from the whole-build log:
This is not a timeout or hang. Stage durations were 16 s and 25 s, log lines are continuous with sub-second gaps (largest gap < 1 s) right up to the kill, and the kill message names a human actor rather than a time limit. The Two real (but non-fatal, pre-existing) defects are visible in the same log and are worth separate fixes:
Implicated commit: None for the abort (operator action, not a commit). For the File: Abort: no source file — Suggested fix:
Related: PR #12094 ("CI: Jenkins integration testing (do not merge)") — the test-only branch under validation; rocm.m4 change PR #11236. |
| - git: | ||
| url: "https://github.com/openucx/ucx.git" | ||
| branches: [ 'master' ] | ||
| branches: [ 'origin/pr/12094/head' ] |
There was a problem hiding this comment.
blocker: The dispatcher job's pipeline SCM branch is pinned to this PR's head instead of master. This file is "applied by DevOps to Jenkins", so merging it would make the production dispatcher permanently load Jenkinsfile.dispatcher from PR 12094.
| branches: [ 'origin/pr/12094/head' ] | |
| branches: [ 'master' ] |
Leftover from testing — the dispatcher must load the Jenkinsfile from master.
| run: | | ||
| ./contrib/test_jenkins.sh | ||
| # Limit tests to 2 CPUs to prevent max_threads exhaustion in k8s (similar to Azure CPU affinity) | ||
| allowed_cpus=$(awk '/^Cpus_allowed_list:/ {print $2}' /proc/self/status) |
There was a problem hiding this comment.
Pinning derives CPUs from a host-wide mask: unless the kubelet runs the static CPU manager policy, Cpus_allowed_list in a pod is the full host range (e.g. 0-127), so first_chunk is the whole range and every one of the 8 worker pods (2 images × 4 workers) pins itself to the same CPUs 0-1 and contends for them. Can we offset the slice by ${worker}, or rely on the cgroup CPU quota instead?
| arch: x86_64, | ||
| tolerations: "[{key: 'node.kubernetes.io/unschedulable', operator: 'Exists', effect: 'NoSchedule'}]", | ||
| annotations: [{ key: "k8s.v1.cni.cncf.io/networks", value: "sriov-cx7-p2" }], | ||
| limits: "{memory: 16Gi, cpu: 10000m, nvidia.com/sriov_cx7_p2: 1}", |
There was a problem hiding this comment.
Requests 10 CPUs but uses 2: limits/requests ask for cpu: 10000m per pod while the step pins all work (including make -j) to at most 2 CPUs, over-reserving the node by 8 CPUs per pod. Can we lower the request so the nodes are not over-reserved?
| RUN_TESTS: "yes" | ||
| TEST_PERF: "0" | ||
| ASAN_CHECK: "no" | ||
| VALGRIND_CHECK: "no" |
There was a problem hiding this comment.
ASAN_CHECK removed without explanation. contrib/test_jenkins.sh documents ASAN_CHECK as a job input and branches on it; it still works because the script does not use set -u, but every other matrix (test_gpu_matrix.yaml, test_dl_matrix.yaml) sets it explicitly. Why remove it?
| if (!alive) break | ||
| alive.each { target -> | ||
| echo "${attempt > 1 ? 'Retry ' + attempt + ': ' : ''}Killing ${target.fullDisplayName}" | ||
| target.doKill() |
There was a problem hiding this comment.
doKill() skips the leaf job's finally: Jenkinsfile.github updates the GitHub commit status and uploads logs in a finally block. A hard doKill() bypasses it, so the killed leaf jobs leave their per-job status stuck at "CI started". Please call doStop() first and use doKill() only if the build is still alive.
| // Collect leaf builds from each stale dispatcher's console log | ||
| def leafBuilds = [] | ||
| staleBuilds.each { b -> | ||
| b.getLog(1000).each { line -> |
There was a problem hiding this comment.
Leaf builds found by scraping console text: Run.getLog(1000) returns only the tail of the log, while the Starting building: lines are printed at the very beginning of the dispatcher run, so a verbose run silently finds no leaf builds. Why not find leaf builds by their githubData/VARIABLE_FROM_POST parameter, like the dispatcher matching above, instead of parsing the console log?
| # Each thread requires 5MB. Cap threads number by total available shared memory. | ||
| max_threads=$(df /dev/shm | awk '/shm/ {printf "%d", $4 / 5000}') | ||
| num_threads=$(($max_threads < $(nproc) ? $max_threads : $(nproc))) | ||
| num_threads=$(($max_threads < ${NPROC} ? $max_threads : ${NPROC})) |
There was a problem hiding this comment.
NPROC is larger than the pinned CPU count: with the new taskset, nproc returns 2 inside the step, while NPROC from buildlib/tools/common.sh is memory-derived (16 for the 16Gi limit). So for this job the change raises num_threads from 2 to 16, which works against the stated goal of bounding max_threads. Is raising the thread count intended?
| timeout: "${STEP_TIMEOUT_MINUTES}" | ||
| run: | | ||
| ./contrib/test_jenkins.sh | ||
| # Limit tests to 2 CPUs to prevent max_threads exhaustion in k8s (similar to Azure CPU affinity) |
There was a problem hiding this comment.
minor: This comment and the one on line 62 state the same thing; one is enough.
|
Per |
|
🤖 CI Triage Agent — TL;DR: Build #15 was not a real codestyle failure — the Full analysisSummary: Root cause: The stage log shows normal, expected progress: Secondary (cosmetic, not the failure): Implicated commit: bfc4886 "CI: Kill leaf jobs when aborting stale PR dispatchers" (Alexey Rivkin), on top of feec90b "CI: Extract dispatcher pipeline into Jenkinsfile" (Noam Tsemah) File: Suggested fix:
Related: #11491 (CI: kill leaf jobs when aborting stale PR dispatchers), #11796 (CI: Replace Blue Ocean with pipeline-overview URL), PR #12094 (this build's PR) |
|
🤖 CI Triage Agent — TL;DR: Build #20 wasn't a product/test failure — it was externally aborted ("Aborted by Alexey Rivkin") ~7 s into the Full analysisSummary: Root cause: Two interacting infrastructure issues, no UCX code defect:
Secondary, non-fatal noise worth cleaning up: Implicated commit: File: Suggested fix:
Related: PR #12094 (CI: Jenkins integration testing — do not merge), PR #11491 (CI: kill leaf jobs when aborting stale PR dispatchers), PR #11796 (CI: Replace Blue Ocean with pipeline-overview URL)
|
Holding Run objects or a JsonSlurper across the retry sleep breaks the pipeline state checkpoint, so targets are kept as job name and build number pairs, as in NIXL. Leaf jobs live in the UCX-OSS folder and are logged as 'UCX-OSS » <job>', which the NIXL regex does not match, so the display path is mapped back to the full job name.
|
🤖 Starting review — findings will be posted here when done. |
| - git: | ||
| url: "https://github.com/openucx/ucx.git" | ||
| branches: [ 'master' ] | ||
| branches: [ 'origin/pr/12094/head' ] |
There was a problem hiding this comment.
blocker — dispatcher SCM pinned to this PR's branch. This is a debug pin that must be reverted before merge; the file is applied by DevOps to Jenkins, so the dispatcher would permanently load Jenkinsfile.dispatcher from PR #12094 instead of master. Suggested: branches: [ 'master' ]
| if (!alive) break | ||
| alive.each { t -> | ||
| echo "${attempt > 1 ? 'Retry ' + attempt + ': ' : ''}Killing ${t[0]} #${t[1]}" | ||
| Jenkins.instance.getItemByFullName(t[0])?.getBuildByNumber(t[1])?.doKill() |
There was a problem hiding this comment.
doKill() skips pipeline cleanup. doKill() is Jenkins' hard kill and does not run post/finally sections. Two concrete consequences:
pipeline_stopintest_gpu_matrix.yamlandtest_dl_matrix.yamlreleases the Slurm allocations (SLURM_JOB_TIMEOUT: '02:45:00'/'03:20:00'); hard-killing the leaf leaks the allocation and the/mnt/pvc/job_id_*file until the Slurm timeout expires, holdingfunk/gb200nvl72_cinodes.- Hard-killing the stale dispatcher skips its
catchblock, soupdateCommitStatus(... FAILURE)never runs and the PR keeps a stale "Blossom CI started" pending status.
Can we doStop() on the first attempts and only doKill() on the last one?
| // path is mapped back to the "UCX-OSS/<job>" full name. | ||
| def targets = [] | ||
| staleBuilds.each { b -> | ||
| b.getLog(1000).each { line -> |
There was a problem hiding this comment.
Leaf discovery via console-log scraping is fragile. Run.getLog(1000) returns only the tail of the log, so the Starting building: lines (emitted at the very beginning of the parallel fanout) can fall out of the window and the leaves will silently survive. The match also depends on the " » " display-name format. Instead of parsing the console log, can we locate the leaf builds via their UpstreamCause (upstream project + build number)?
| # Each thread requires 5MB. Cap threads number by total available shared memory. | ||
| max_threads=$(df /dev/shm | awk '/shm/ {printf "%d", $4 / 5000}') | ||
| num_threads=$(($max_threads < $(nproc) ? $max_threads : $(nproc))) | ||
| num_threads=$(($max_threads < ${NPROC} ? $max_threads : ${NPROC})) |
There was a problem hiding this comment.
NPROC ignores the CPU affinity mask. NPROC from buildlib/tools/common.sh is derived from the cgroup memory limit (16Gi → 16), not from the affinity mask. With the new taskset -c 0-1 wrapper, /dev/shm in a pod is typically 64MB → max_threads ≈ 13, so test_init_mt now runs ~13 threads on 2 CPUs, whereas plain nproc (affinity-aware) returned 2. This works against the stated goal of the pinning in test_matrix.yaml. Pls cap by the number of allowed CPUs.
| fi | ||
|
|
||
| echo "Running test_jenkins.sh with CPU affinity: ${cpu_set} (allowed: ${allowed_cpus})" | ||
| taskset -c "${cpu_set}" ./contrib/test_jenkins.sh |
There was a problem hiding this comment.
Whole script pinned to 2 CPUs while the pod requests 10. taskset wraps all of test_jenkins.sh, including build devel, $MAKEP, and the MPI/release builds, so every build in the job is confined to 2 cores even though requests/limits ask for cpu: 10000m. This is the likely reason timeout_minutes had to go 90 → 240. Why not bound the thread counts in the tests instead and let the build use the requested CPUs?
| ./contrib/test_jenkins.sh | ||
| # Limit tests to 2 CPUs to prevent max_threads exhaustion in k8s (similar to Azure CPU affinity) | ||
| allowed_cpus=$(awk '/^Cpus_allowed_list:/ {print $2}' /proc/self/status) | ||
| first_chunk=${allowed_cpus%%,*} |
There was a problem hiding this comment.
minor — only the first CPU chunk is considered. With the k8s static CPU manager, Cpus_allowed_list is commonly a non-contiguous sibling pair (e.g. 3,67). first_chunk is then 3 with no -, so cpu_set becomes a single CPU, and slice_affinity 0/slice_affinity 1 in test_jenkins.sh return the same core for both client and server. Maybe count CPUs across all chunks instead of only the first one.
Every row ran build_mode=short, so long-mode checks (clang, gcc, no_devx, installed headers, soname) never ran, and without /hpc/local the CUDA, fuse and doxygen steps were skipped. Set the Azure build mode per image, mount /hpc/local, and add the rows Azure has and Jenkins lacked: centos7 with IB, rhel8.2 with GCC 15.2, the ZE null-driver build and ICC/PGI.
With the 64M container /dev/shm, mm iface chunk allocation fails (uct_mem.c:482 assertion). A RAM-backed /dev/shm fixes that but counts against the pod limit, and 16Gi pods get SIGKILLed mid-run, so pods get 32Gi. test_uct_peer_failure_multiple needs more contexts than a VF BAR allows and is excluded until the VF BAR is raised. /hpc/local provides the MPI modules for the MPI tests.
|
🤖 Starting review — findings will be posted here when done. |
Checks whether Blossom pods can pull UCX builder images through the Artifactory Harbor remote.
|
🤖 CI Triage Agent — TL;DR: Build #18 never got to compile — its Full analysisSummary: Root cause: Evidence chain from the logs:
Secondarily, the dispatcher's stale-build killer should have removed this zombie leaf: This is a CI-infrastructure/race failure, not a UCX code defect. Implicated commit: File: Suggested fix:
Related: PR #12094 — "CI: Jenkins integration testing (do not merge)" (#12094)
|
| TEST_PERF: "0" | ||
| ASAN_CHECK: "no" | ||
| # MPI test_memhooks loops on a debug.c assert with the lab HPC-X (Open MPI 5). | ||
| RUN_MPI_TESTS: "no" |
There was a problem hiding this comment.
all four gtest matrices (test, gpu, dl, bf) set RUN_MPI_TESTS=no, so run_mpi_tests - and the make installcheck inside it - never runs in the new CI. can we keep it enabled on at least one row? the /hpc/local mount above is commented as being there for the MPI tests.
| refspec: "+refs/heads/*:refs/remotes/origin/* +refs/pull/*:refs/remotes/origin/pr/*" | ||
| browser: githubweb | ||
| browser-url: "https://github.com/openucx/ucx" | ||
| wipe-workspace: false |
There was a problem hiding this comment.
wipe-workspace: false (leaf template): JJB defaults wipe-workspace to true, so this keeps the previous build's tree in the workspace. steps that create fresh dirs then fail: mkdir build in test_bindings_matrix.yaml and git_clone_with_retry into ${WORKSPACE}/ucx_<tag> in test_wirecompat_matrix.yaml. why is it needed?
| if IFS= read -r -n 1 _; then | ||
| echo "ERROR: step output passed ${ci_output_cap} bytes" >&${ci_stdout} | ||
| fi) 2>&1 | ||
| ci_output_pid=$! |
There was a problem hiding this comment.
minor: bash sets $! for a process substitution only since 5.1, so ci_output_pid is empty on the centos7 / ubuntu18.04 images used by test_cuda_matrix.yaml and test_wirecompat_gpu_matrix.yaml; the EXIT loop breaks on the first kill -0, so the last lines it is meant to save are still lost there.
| run: run | ||
| args: | ||
| jobIdFile: "${JOB_ID_FILE_ROOT}/job_id_gpuwc_${BUILD_NUMBER}.txt" | ||
| testScript: >- |
There was a problem hiding this comment.
this is the same legacy-tag loop as test_wirecompat_matrix.yaml, rewritten as one line (and the non-GPU copy still carries dead != v1.14.0 checks). can we move the loop into a buildlib/tools/ script and call it from both? root AGENTS.md asks not to duplicate implementation patterns.
| args: | ||
| jobIdFile: "${JOB_ID_FILE_ROOT}/job_id_jucx_${BUILD_NUMBER}.txt" | ||
| # Skip testMmapFile: it assumes 4K pages, GB200 uses 64K | ||
| testScript: &jucx_test 'set -xeE; source .ci/scripts/cap_output.sh; export JAVA_HOME=/usr/lib/jvm/$jucx_jdk; export PATH=/usr/lib/jvm/$jucx_jdk/bin:$(printenv PATH); ulimit -l unlimited; unset UCX_NET_DEVICES; source buildlib/az-helpers.sh; ifaces=$(get_rdma_interfaces); if [ -z "$ifaces" ]; then echo ERROR: No active RDMA interfaces on machine; exit 1; fi; jport=$((20000 + RANDOM % 10000)); export JUCX_TEST_PORT=$jport; export UCX_HANDLE_ERRORS=bt; jtests=$(cd bindings/java/src/test/java/org/openucx/jucx; ls Ucp*Test.java | grep -v UcpMemoryTest | sed s/.java// | paste -sd,); make -C bindings/java/src/main/native test MVN="mvn -Dtest=$jtests"; make -C bindings/java/src/main/native test MVN="mvn -Dtest=UcpMemoryTest#testGetRkey+testRemoteKeyUnpack+testExportedMkey"; make -C bindings/java/src/main/native package; ipv4_found=0; for iface in $ifaces; do sip=$(get_ip $iface); if [ -z "$sip" ]; then continue; fi; echo Running standalone benchmark on $iface:$jport; java -XX:ErrorFile=hs_err_%p.log -XX:OnError="cat hs_err_%p.log" -cp "bindings/java/resources/:bindings/java/src/main/native/build-java/*" org.openucx.jucx.examples.UcxReadBWBenchmarkReceiver s=$sip p=$jport t=1000000 & sleep 10; java -XX:ErrorFile=hs_err_%p.log -XX:OnError="cat hs_err_%p.log" -cp "bindings/java/resources/:bindings/java/src/main/native/build-java/*" org.openucx.jucx.examples.UcxReadBWBenchmarkSender s=$sip p=$jport t=1000000; wait; ipv4_found=1; done; if [ $ipv4_found -eq 0 ]; then echo ERROR: No IPv4 address on any of $ifaces; exit 1; fi' |
There was a problem hiding this comment.
the JUCX build+test body now exists in three slightly different copies (here, test_bindings_matrix.yaml, test_bindings_gpu_matrix.yaml) on top of buildlib/jucx/jucx-test.yml. can we extract one script under buildlib/jucx/ that all rows call?
|
Existing threads supported with +1 (not re-opened): the Residual coverage gaps (unverifiable from the repo, need CI run links): Per-docker key propagation as step env ( Review mode: REQUEST_CHANGES, carried by the still-open branch-pin blocker; findings 1–5 above are non-blocking. |
The GB200 filter hid five product failures (cuda-async and cuda-managed rendezvous, the new GPU-NIC RMA test and a tag fallback timeout), so the job could pass while UCX is broken on GB200. Run the full gtest set so these failures stay visible until UCX fixes them. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
|
🤖 Starting review — findings will be posted here when done. |
| if IFS= read -r -n 1 _; then | ||
| echo "ERROR: step output passed ${ci_output_cap} bytes" >&${ci_stdout} | ||
| fi) 2>&1 | ||
| ci_output_pid=$! |
There was a problem hiding this comment.
$! is only set for process substitutions in bash >= 5.1, so ci_output_pid is empty on the centos7/centos8 images that source this (test_wirecompat_gpu, test_cuda, test_gpu asan) and the drain loop below breaks immediately. Can we have the reader publish its own $BASHPID instead, so the tail of a step log is not lost?
| args: | ||
| jobIdFile: "${JOB_ID_FILE_ROOT}/job_id_jucx_${BUILD_NUMBER}.txt" | ||
| # Skip testMmapFile: it assumes 4K pages, GB200 uses 64K | ||
| testScript: &jucx_test 'set -xeE; source .ci/scripts/cap_output.sh; export JAVA_HOME=/usr/lib/jvm/$jucx_jdk; export PATH=/usr/lib/jvm/$jucx_jdk/bin:$(printenv PATH); ulimit -l unlimited; unset UCX_NET_DEVICES; source buildlib/az-helpers.sh; ifaces=$(get_rdma_interfaces); if [ -z "$ifaces" ]; then echo ERROR: No active RDMA interfaces on machine; exit 1; fi; jport=$((20000 + RANDOM % 10000)); export JUCX_TEST_PORT=$jport; export UCX_HANDLE_ERRORS=bt; jtests=$(cd bindings/java/src/test/java/org/openucx/jucx; ls Ucp*Test.java | grep -v UcpMemoryTest | sed s/.java// | paste -sd,); make -C bindings/java/src/main/native test MVN="mvn -Dtest=$jtests"; make -C bindings/java/src/main/native test MVN="mvn -Dtest=UcpMemoryTest#testGetRkey+testRemoteKeyUnpack+testExportedMkey"; make -C bindings/java/src/main/native package; ipv4_found=0; for iface in $ifaces; do sip=$(get_ip $iface); if [ -z "$sip" ]; then continue; fi; echo Running standalone benchmark on $iface:$jport; java -XX:ErrorFile=hs_err_%p.log -XX:OnError="cat hs_err_%p.log" -cp "bindings/java/resources/:bindings/java/src/main/native/build-java/*" org.openucx.jucx.examples.UcxReadBWBenchmarkReceiver s=$sip p=$jport t=1000000 & sleep 10; java -XX:ErrorFile=hs_err_%p.log -XX:OnError="cat hs_err_%p.log" -cp "bindings/java/resources/:bindings/java/src/main/native/build-java/*" org.openucx.jucx.examples.UcxReadBWBenchmarkSender s=$sip p=$jport t=1000000; wait; ipv4_found=1; done; if [ $ipv4_found -eq 0 ]; then echo ERROR: No IPv4 address on any of $ifaces; exit 1; fi' |
There was a problem hiding this comment.
Bare wait can block on the output reader: pls capture the receiver pid and use wait $java_pid here, as test_bindings_matrix.yaml does (see test_bindings_matrix.yaml:205-209) — with cap_output.sh sourced, a bare wait can also wait for its process substitution, which only exits at script end (dl-base is Ubuntu 24.04 / bash 5.2).
|
|
||
| # all are running mpi tests | ||
| run_mpi_tests | ||
| if [ "${RUN_MPI_TESTS:-yes}" == "yes" ] |
There was a problem hiding this comment.
minor: new env var is undocumented — pls add RUN_MPI_TESTS to the env var list in the header comment, next to RUN_TESTS/TEST_PERF.
| branches: [ 'master' ] | ||
| shallow-clone: true | ||
| depth: 100 | ||
| branches: [ 'origin/pr/12094/head' ] |
There was a problem hiding this comment.
Already reported (support the existing thread rather than re-opening) — blocker, still open: the dispatcher SCM is still pinned to branches: [ 'origin/pr/12094/head' ] and drops shallow-clone/depth while adding wipe-workspace: false. The base file uses master with shallow-clone: true, depth: 100. This is debug scaffolding that must be reverted before merge.
|
Step timeout was cut 360 → 210 but this matrix sets neither
Already reported (existing thread): PR size (~750 added lines) exceeds the 500-added-line hard limit in Already reported (existing thread): Already reported (existing thread): Already reported (existing thread): Already reported (existing thread): Already reported (existing thread): Already reported (existing thread): aggregate SR-IOV VF demand and concurrent funk-node demand. Residual coverage gaps (need CI run links; all previously raised): per-docker key propagation as step env ( |
Coverity 2019.12 misses the security checkers needed for current CWE coverage. Move the Jenkins Coverity job to the lab's tools/cov-2026.03 module at high aggressiveness, in the rhel9.0 builder image because the new analyzer needs glibc 2.18+, and generate a compiler configuration per job because the new installation ships none. coverity.sh takes the module, aggressiveness and configure step from the environment, so Azure keeps running 2019.12 unchanged. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
|
🤖 Starting review — findings will be posted here when done. |
| # Install scctl for slurm client management (x86_64 only, not available for ARM64) | ||
| RUN if [ "$(uname -m)" = "x86_64" ]; then \ | ||
| curl -s https://urm.nvidia.com/artifactory/sw-swx-lab-infra-generic/scctl/get-app.sh | bash -s -- --output-dir /usr/local/bin || echo "INFO: scctl installation failed, continuing..."; \ | ||
| curl -fsS https://artifactory.nvidia.com/artifactory/sw-swx-lab-infra-generic-local/scctl/get-app.sh | bash -s -- --output-dir /usr/local/bin; \ |
There was a problem hiding this comment.
Dropping the || echo ... continuing was meant to make this fail loud, but a pipeline's exit status is bash's, and the default RUN shell has no pipefail — so a 404/401 from curl still leaves the image without scctl and the RUN succeeds.
Suggested change:
curl -fsS https://artifactory.nvidia.com/artifactory/sw-swx-lab-infra-generic-local/scctl/get-app.sh -o /tmp/get-app.sh && \
bash /tmp/get-app.sh --output-dir /usr/local/bin && rm /tmp/get-app.sh; \
| refspec: "+refs/heads/*:refs/remotes/origin/* +refs/pull/*:refs/remotes/origin/pr/*" | ||
| browser: githubweb | ||
| browser-url: "https://github.com/openucx/ucx" | ||
| wipe-workspace: false |
There was a problem hiding this comment.
why wipe-workspace: false? (same at line 105) JJB defaults it to true, and steps added here fail on a reused workspace: mkdir build in test_bindings_matrix.yaml:97 and git_clone_with_retry into ${WORKSPACE}/ucx_${ucx_tag} in test_wirecompat_matrix.yaml:78 (plain git clone into an existing non-empty dir). Removing shallow-clone looks justified — test_bindings_matrix.yaml:162 needs git describe --tags — but this one looks like debug leftover.
| def leafBranches = { jobs -> | ||
| jobs.collectEntries { branchName, jobName -> | ||
| [(branchName): { | ||
| catchError(buildResult: 'SUCCESS', stageResult: 'FAILURE', catchInterruptions: false) { |
There was a problem hiding this comment.
catchError here also swallows a dispatch failure, e.g. a leaf name not yet registered in JJB: the branch turns red but the dispatcher still reports SUCCESS and the leaf posts no GitHub status at all (Jenkinsfile.github:24), so the PR looks green with a check silently missing. The basic-compile branch guards exactly this with the gate?.result re-check — can we do the same for the leaves?
| if IFS= read -r -n 1 _; then | ||
| echo "ERROR: step output passed ${ci_output_cap} bytes" >&${ci_stdout} | ||
| fi) 2>&1 | ||
| ci_output_pid=$! |
There was a problem hiding this comment.
minor: $! is only set for a process substitution by recent bash, so on the older images (the centos7 nvidia/cuda rows in test_cuda, centos7-mofed5.4-cuda11 in test_wirecompat_gpu, centos8-...-cuda11_asan in test_gpu) ci_output_pid stays empty, kill -0 '' fails and the drain loop breaks immediately — dropping the last lines exactly when a step fails. can we have the subshell write its $$ to a temp file instead?
| run: | | ||
| source .ci/scripts/cap_output.sh | ||
| set -eEx -o pipefail | ||
| hcas=($(ibv_devinfo | awk '/hca_id:/ {h=$2} /port:/ {p=$2} /state:/ {s=$2} |
There was a problem hiding this comment.
buildlib/tools/test_mad.sh already has detect_active_ib_hca/set_vars doing this ibv_devinfo | awk and the ibstat LID/GUID parsing, and the Build step above already sources that file. can we add the two-port variant there and keep the YAML to a one-line call? 20 lines of test logic in YAML is hard to reproduce by hand when it breaks.
| args: | ||
| jobIdFile: "${JOB_ID_FILE_ROOT}/job_id_jucx_${BUILD_NUMBER}.txt" | ||
| # Skip testMmapFile: it assumes 4K pages, GB200 uses 64K | ||
| testScript: &jucx_test 'set -xeE; source .ci/scripts/cap_output.sh; export JAVA_HOME=/usr/lib/jvm/$jucx_jdk; export PATH=/usr/lib/jvm/$jucx_jdk/bin:$(printenv PATH); ulimit -l unlimited; unset UCX_NET_DEVICES; source buildlib/az-helpers.sh; ifaces=$(get_rdma_interfaces); if [ -z "$ifaces" ]; then echo ERROR: No active RDMA interfaces on machine; exit 1; fi; jport=$((20000 + RANDOM % 10000)); export JUCX_TEST_PORT=$jport; export UCX_HANDLE_ERRORS=bt; jtests=$(cd bindings/java/src/test/java/org/openucx/jucx; ls Ucp*Test.java | grep -v UcpMemoryTest | sed s/.java// | paste -sd,); make -C bindings/java/src/main/native test MVN="mvn -Dtest=$jtests"; make -C bindings/java/src/main/native test MVN="mvn -Dtest=UcpMemoryTest#testGetRkey+testRemoteKeyUnpack+testExportedMkey"; make -C bindings/java/src/main/native package; ipv4_found=0; for iface in $ifaces; do sip=$(get_ip $iface); if [ -z "$sip" ]; then continue; fi; echo Running standalone benchmark on $iface:$jport; java -XX:ErrorFile=hs_err_%p.log -XX:OnError="cat hs_err_%p.log" -cp "bindings/java/resources/:bindings/java/src/main/native/build-java/*" org.openucx.jucx.examples.UcxReadBWBenchmarkReceiver s=$sip p=$jport t=1000000 & sleep 10; java -XX:ErrorFile=hs_err_%p.log -XX:OnError="cat hs_err_%p.log" -cp "bindings/java/resources/:bindings/java/src/main/native/build-java/*" org.openucx.jucx.examples.UcxReadBWBenchmarkSender s=$sip p=$jport t=1000000; wait; ipv4_found=1; done; if [ $ipv4_found -eq 0 ]; then echo ERROR: No IPv4 address on any of $ifaces; exit 1; fi' |
There was a problem hiding this comment.
this is the third copy of the JUCX flow in this PR (test_bindings_matrix.yaml, test_bindings_gpu_matrix.yaml, here), each written differently, on top of buildlib/jucx/jucx-test.yml. can we extract it into a buildlib/tools/test_jucx.sh that all three call? a single-line rendering of that whole flow is not maintainable.
| BUILD_ID: "${BUILD_NUMBER}" | ||
| # Module name of the lab modulefiles tree. | ||
| COV_MODULE: "tools/cov-2026.03" | ||
| COV_AGGRESSIVENESS: "high" |
There was a problem hiding this comment.
why high? run_coverity fails on any reported defect (coverity.sh:115-117), so raising aggressiveness together with the 2019.12 → 2026.03 bump is likely to keep the coverity leaf red until all the new findings are triaged.
| jobs: | ||
| - "{jjb_proj}" | ||
|
|
||
| - project: |
There was a problem hiding this comment.
Existing thread (+1, still true): ucx-test-namespace-gpu-oss is registered here (lines 242-248) but absent from the dispatcher leafJobs.
|
Existing thread (+1, still true): the Existing thread (+1, still true): PR size is over the 500-added-line limit.
Existing thread (+1, still true): Existing thread (+1, still true): step timeouts exceed
Existing thread (+1, still true): Existing thread (+1, still true): Existing thread (+1, still true): Existing thread (+1, still true): aggregate SR-IOV VF / funk-node demand. Residual coverage gaps — unverifiable from the repository, still needing CI run links: per-docker key propagation as step env ( |
With Coverity 2026 the devel capture alone took 21 minutes with 4 jobs, so cov-analyze was killed at the 30 minute step timeout. Build and analyze with all 6 pod CPUs and raise the step timeout to 75 minutes, within the 90 minute job timeout. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
|
🤖 Starting review — findings will be posted here when done. |
| # Install scctl for slurm client management (x86_64 only, not available for ARM64) | ||
| RUN if [ "$(uname -m)" = "x86_64" ]; then \ | ||
| curl -s https://urm.nvidia.com/artifactory/sw-swx-lab-infra-generic/scctl/get-app.sh | bash -s -- --output-dir /usr/local/bin || echo "INFO: scctl installation failed, continuing..."; \ | ||
| curl -fsS https://artifactory.nvidia.com/artifactory/sw-swx-lab-infra-generic-local/scctl/get-app.sh | bash -s -- --output-dir /usr/local/bin; \ |
There was a problem hiding this comment.
The || echo "...continuing..." was dropped to make a failed scctl install fatal, but the pipe still hides it: RUN uses /bin/sh (dash, no pipefail), so the exit status is bash's, not curl's. On a 404/auth error curl -fsS writes nothing to stdout, bash -s -- reads an empty script and exits 0, and the image builds without scctl — surfacing much later as scctl: command not found in the allocation step of test_gpu/test_cuda/test_bindings_gpu/test_ucxx_gpu.
Suggestion:
curl -fsS https://artifactory.nvidia.com/artifactory/sw-swx-lab-infra-generic-local/scctl/get-app.sh -o /tmp/get-app.sh && bash /tmp/get-app.sh --output-dir /usr/local/bin; \
|
|
||
| az_init_modules | ||
| modules_for_coverity | ||
| cov-analyze --ident |
There was a problem hiding this comment.
minor: cov-analyze --ident is unconditional, and COV_MODULE still defaults to tools/cov-2019.12, so this diagnostic also runs on the Azure job (buildlib/pr/coverity.yml:23). The script is #!/bin/bash -eEl, so a version that rejects the flag would abort Coverity on Azure for a line with no functional purpose.
Suggestion:
cov-analyze --ident || true
|
|
||
| parallel(leafBranches(leafJobs)) | ||
|
|
||
| githubHelper.updateCommitStatus(pipelineOverviewUrl, "UCX CI ended", GitHubCommitState.SUCCESS) |
There was a problem hiding this comment.
minor: Both the success and failure paths now post the same description, "UCX CI ended", so the text next to the GitHub check no longer says whether CI passed (only the state icon differs). Can we keep distinct wording, e.g. "UCX CI passed" / "UCX CI failed"? (also applies at line 158)
| branches: [ 'master' ] | ||
| shallow-clone: true | ||
| depth: 100 | ||
| branches: [ 'origin/pr/12094/head' ] |
There was a problem hiding this comment.
Already reported (+1 on existing thread): branch pin to origin/pr/12094/head — base uses branches: [ 'master' ]; also drops shallow-clone/depth: 100. Still the blocker carrying REQUEST_CHANGES.
| arch: x86_64, | ||
| # posix without proc link needs the host (default) PID namespace; | ||
| # ci-demo has no hostPID option, so it rides on the raw tolerations. | ||
| tolerations: "[]\n hostPID: true" |
There was a problem hiding this comment.
Already reported (+1 on existing thread): hostPID/hostIPC injected through the raw tolerations string (also test_wirecompat_matrix.yaml:28).
|
Already reported (+1 on existing thread): PR size is ~750 added lines vs the 500 hard limit in Already reported (+1 on existing thread):
Already reported (+1 on existing thread): Already reported (+1 on existing thread): Already reported (+1 on existing thread):
Already reported (+1 on existing thread): Already reported (+1 on existing thread): aggregate SR-IOV VF demand and concurrent funk-node demand. Already reported (+1 on existing thread): scope of the ucxx Residual coverage gaps (need CI run links; all previously raised): per-docker key propagation as step env (
Same class as the timeout findings, so folding it in rather than opening a thread: |
The PGI compiler (nvc 21.2) fails the devel build when a type is
typedef'd twice ("typedef name has already been declared"), which breaks
the EFA transport build. ROCm IPC and ZE IPC headers repeat the same
typedefs, so keep a single one for each type.
Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
(cherry picked from commit 7066531)
rocm_ipc_md.h forward-declares uct_rocm_ipc_cache_t but never uses it, and every user of the type includes rocm_ipc_cache.h. Removing the forward declaration leaves a single typedef on the struct definition, same as the ZE IPC change, instead of splitting the type across two headers. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com> (cherry picked from commit 1cc5bc9)
|
🤖 Starting review — findings will be posted here when done. |
| kubernetes: | ||
| cloud: il-ipp-blossom-prod-nbu-swx-ucx | ||
| namespace: nbu-swx-ucx | ||
| limits: "{memory: 10Gi, cpu: 6000m}" |
There was a problem hiding this comment.
The pod requests cpu: 6000m, but the step pins the entire run (build included) to 2 CPUs with taskset -c "$((slot * 2))-$((slot * 2 + 1))". Four of the six reserved cores stay idle, and a 16-core BF3 then fits only 2 pods while batchSize: 4 expects 4 at once. Can we make the request match the pinned set, or widen the taskset to 3 CPUs?
| source .ci/scripts/cap_output.sh | ||
| # Host network and CPUs are shared, so each pod gets its own ports and 2 CPUs. | ||
| slot=$((SLOT_BASE + worker)) | ||
| export EXECUTOR_NUMBER=${slot} |
There was a problem hiding this comment.
slot comes only from SLOT_BASE + worker, so two concurrent builds of this job (the job template sets concurrent: true) pick the same slots. With hostNetwork: true their EXECUTOR_NUMBER-derived port ranges (UCX_TCP_PORT_RANGE, server_port_min in common.sh) collide whenever the pods land on the same BF3 node. Azure avoided this with AZP_AGENT_ID, which buildlib/pr/tests.yml:94 documents as "uniq for one host". How is cross-build slot uniqueness handled here?
| refspec: "+refs/heads/*:refs/remotes/origin/* +refs/pull/*:refs/remotes/origin/pr/*" | ||
| browser: githubweb | ||
| browser-url: "https://github.com/openucx/ucx" | ||
| wipe-workspace: false |
There was a problem hiding this comment.
Are wipe-workspace: false and dropping shallow-clone/depth: 100 intended, or part of the same debug change as the origin/pr/12094/head pin above? Without the wipe, untracked leftovers from a previous build persist in the leaf-job workspace.
| # Each thread requires 5MB. Cap threads number by total available shared memory. | ||
| max_threads=$(df /dev/shm | awk '/shm/ {printf "%d", $4 / 5000}') | ||
| num_threads=$(($max_threads < $(nproc) ? $max_threads : $(nproc))) | ||
| num_threads=$(($max_threads < ${NPROC} ? $max_threads : ${NPROC})) |
There was a problem hiding this comment.
| num_threads=$(($max_threads < ${NPROC} ? $max_threads : ${NPROC})) | |
| num_threads=$(($max_threads < $(nproc) ? $max_threads : $(nproc))) |
NPROC from common.sh:38 is nproc --all outside containers, which ignores CPU affinity, whereas $(nproc) respected it. Enroot/pyxis sets neither /.dockerenv nor KUBERNETES_SERVICE_HOST, so on the DL cluster this reports 144 instead of the ~24 CPUs the Slurm cpuset grants, and the new BF3 rows run under taskset -c with 2 CPUs.
|
Already reported — still true in this revision (support the existing threads, do not re-open): the Residual coverage gaps (need CI run links): per-docker key propagation as step env ( |
The dispatcher job loaded its pipeline from the integration PR branch, so every /build ran that branch's basic compile gate and its full job list. PRs that do not carry those matrix files failed on missing conf_file. Loading from master runs the job list each PR is expected to pass. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
|
🤖 Starting review — findings will be posted here when done. |
| BUILD_ID: "${BUILD_NUMBER}" | ||
| # Module name of the lab modulefiles tree. | ||
| COV_MODULE: "tools/cov-2026.03" | ||
| COV_AGGRESSIVENESS: "high" |
There was a problem hiding this comment.
why high here? run_coverity fails the job on any non-zero defect count, so raising the aggressiveness level on top of the Coverity 2019→2026 jump will likely surface pre-existing findings unrelated to the PR. can we land the version bump first with the default level?
|
|
||
| az_init_modules | ||
| modules_for_coverity | ||
| cov-analyze --ident |
There was a problem hiding this comment.
minor: this looks like a leftover probe (cov-analyze --ident) — pls drop it or guard it; the script runs with -e and Azure still loads tools/cov-2019.12 here, so an unsupported option would fail the Coverity stage there too.
| if IFS= read -r -n 1 _; then | ||
| echo "ERROR: step output passed ${ci_output_cap} bytes" >&${ci_stdout} | ||
| fi) 2>&1 | ||
| ci_output_pid=$! |
There was a problem hiding this comment.
minor: $! is only set by a process substitution from bash 5.1 on, so on the CentOS 7 / UBI 8 images (bash 4.x) ci_output_pid stays empty, kill -0 fails and the drain loop breaks immediately — the last lines of the step can still be lost.
| def leafBranches = { jobs -> | ||
| jobs.collectEntries { branchName, jobName -> | ||
| [(branchName): { | ||
| catchError(buildResult: 'SUCCESS', stageResult: 'FAILURE', catchInterruptions: false) { |
There was a problem hiding this comment.
with buildResult: 'SUCCESS' on every leaf branch the dispatcher build stays SUCCESS, so this sets the commit status to SUCCESS even when e.g. ucx-test-oss failed. is a green aggregate status intended, given the per-leaf statuses are then the only real signal?
A busy GPU partition makes salloc give up after the 1h --immediate window and fail the PR with "Unable to allocate resources: Connection timed out", although the job would have run once a node freed up. Queue for up to 4h instead and raise each job timeout by the same 3h, since the allocation step has no step timeout and only the job timeout bounds the wait. The CUDA memory hooks rows also pull their CUDA image in a step of its own, so a slow image import no longer eats the test step's timeout. Signed-off-by: Alexey Rivkin <arivkin@nvidia.com>
|
🤖 Starting review — findings will be posted here when done. |
| RUNNING_IN_AZURE: "no" | ||
| BUILD_REPOSITORY_URI: "https://github.com/openucx/ucx.git" | ||
| # v1.14.0 off: its server cannot reconfigure the ep the PR client answers. | ||
| UCX_LEGACY_TAGS: "v1.15.x v1.16.x v1.17.x v1.18.x" |
There was a problem hiding this comment.
UCX_LEGACY_TAGS drops v1.14.0, but the two != "v1.14.0" guards below were kept, so they are now dead. On Azure (wire_compat.yml) only stages 1 and 3 are skipped for v1.14.0 while ucp_hello_world (stages 2 and 4) still runs against it — can we add v1.14.0 back to the list so that coverage is kept? The GPU variant (test_wirecompat_gpu_matrix.yaml) dropped it too, and has no guards at all.
| BUILD_ID: "${BUILD_NUMBER}" | ||
| # Module name of the lab modulefiles tree. | ||
| COV_MODULE: "tools/cov-2026.03" | ||
| COV_AGGRESSIVENESS: "high" |
There was a problem hiding this comment.
why high aggressiveness? coverity.sh fails the job whenever nerrors > 0 and has no suppression path, so together with the 2019.12 -> 2026.03 upgrade this gate now has to be clean at the most false-positive-prone level.
| if IFS= read -r -n 1 _; then | ||
| echo "ERROR: step output passed ${ci_output_cap} bytes" >&${ci_stdout} | ||
| fi) 2>&1 | ||
| ci_output_pid=$! |
There was a problem hiding this comment.
minor: $! is not set by process substitution on bash 4.x, which the centos7 CUDA, centos8 asan and rockylinux8 wheel images that source this still run — ci_output_pid stays empty there, kill -0 ${ci_output_pid} fails and the drain loop breaks immediately, losing exactly the trailing lines the trap was added to preserve. Can we use a short fixed sleep instead, so the tail of the log is not lost on those images?
| jobs: | ||
| - "{jjb_proj}" | ||
|
|
||
| - project: |
There was a problem hiding this comment.
+1 (existing thread, still true): ucx-test-namespace-gpu-oss is registered here but absent from the dispatcher's leafJobs — 22 leaf projects defined, 21 dispatched; namespace-gpu is the only dead one, making test_namespace_gpu_matrix.yaml's 154 lines unreachable.
|
|
||
| # Loads swx-jenkins-lib once before the parallel rows; /tmp has no job id files. | ||
| pipeline_start: | ||
| containerSelector: "{name: 'build_helper_ucx'}" |
There was a problem hiding this comment.
Residual coverage gap (needs CI run links): unanchored pipeline_start/pipeline_stop container selectors here and at line 181.
| } | ||
|
|
||
| if (staleBuilds) { | ||
| echo "PR #${currentPrNumber}: found ${staleBuilds.size()} stale dispatcher(s) to abort" |
There was a problem hiding this comment.
Residual coverage gap (needs CI run links): CPS-safety of the rewritten dispatcher abort loop — staleBuilds still holds WorkflowRun objects live across this echo.
|
+1 (existing thread, still true): PR size is over the 500-added-line hard limit.
+1 (existing thread, still true): +1 (existing thread, still true): step timeout sums exceed +1 (existing thread, still true):
+1 (existing thread, still true):
+1 (existing thread, still true): coverity timeout of 90 vs a 75-minute step. +1 (existing thread, still true): +1 (existing thread, still true): aggregate SR-IOV VF and funk-node demand. +1 (existing thread, still true): scope of the Residual coverage gap (needs CI run links): per-docker key propagation as step env — Residual coverage gap (needs CI run links): regex Residual coverage gap (needs CI run links): the
Residual coverage gap (needs CI run links): duplicate
Residual coverage gap (needs CI run links): Residual coverage gap (needs CI run links): availability of Residual coverage gap (needs CI run links):
Residual coverage gap (needs CI run links): Residual coverage gap (needs CI run links): |
What?
Test-only branch that combines the open Jenkins CI drafts (#11173, #11471, #11469, #11491) on top of master.
Why?
Validate all Jenkins (Blossom) jobs together as part of the Azure to Jenkins migration. Jenkins only fetches openucx refs, so the branch needs a PR. Not for merge: each draft lands on its own.
How?