CI: fix for blossom-ci auto trigger without comment - #771
Merged
Merged
Conversation
Fix for allowing blossom ci to be triggered without a /build comment in original commit we forgot to add condition to auth stage Signed-off-by: Daniel Pressler <danielpr@nvidia.com>
yosefe
approved these changes
Sep 4, 2025
yosefe
pushed a commit
that referenced
this pull request
Sep 4, 2025
svc-nixl
referenced
this pull request
Sep 7, 2026
Drive the process-wide allocator through device allocation, host-to-device and device-to-host copies, memset, explicit ownership release, and mapped host memory access. Skip cleanly when no CUDA-capable GPU is available so CPU-only test hosts can still run the unit suite. Signed-off-by: Tomer Davidor <tdavidor@nvidia.com>
svc-nixl
referenced
this pull request
Sep 7, 2026
Drive the process-wide allocator through device allocation, host-to-device and device-to-host copies, memset, explicit ownership release, and mapped host memory access. Skip cleanly when no CUDA-capable GPU is available so CPU-only test hosts can still run the unit suite. Signed-off-by: Tomer Davidor <tdavidor@nvidia.com>
This was referenced Sep 8, 2026
This was referenced Sep 9, 2026
svc-nixl
referenced
this pull request
Sep 14, 2026
nixlBackendInitParams::customParams defaults to nullptr, and the base constructor dereferences it unconditionally. nixlAgent always fills the field, so this never fires in normal use; it fires immediately for a directly constructed engine, which is what a backend's own unit tests do. The crash lands in the base member-initializer list, before any derived constructor body runs, so a backend cannot guard against it on its own. The caller's fix -- pointing the field at an empty map -- is easy once you know it, and not discoverable from a declaration whose default is nullptr. This only stops the base class from crashing. It does not make a null customParams generally supported: gpunetio, obj and the Dell s3_accel engine dereference the pointer directly in their own constructors, and they require parameters, so what an absent-parameters backend should do there is a separate question. Left alone deliberately. Fixes #2245 Signed-off-by: Kyeongpyo Kim <hgichon@gmail.com>
svc-nixl
referenced
this pull request
Sep 14, 2026
nixlBackendInitParams::customParams defaults to nullptr, and the base constructor dereferences it unconditionally. nixlAgent always fills the field, so this never fires in normal use; it fires immediately for a directly constructed engine, which is what a backend's own unit tests do. The crash lands in the base member-initializer list, before any derived constructor body runs, so a backend cannot guard against it on its own. The caller's fix -- pointing the field at an empty map -- is easy once you know it, and not discoverable from a declaration whose default is nullptr. This only stops the base class from crashing. It does not make a null customParams generally supported: gpunetio, obj and the Dell s3_accel engine dereference the pointer directly in their own constructors, and they require parameters, so what an absent-parameters backend should do there is a separate question. Left alone deliberately. Fixes #2245 Signed-off-by: Kyeongpyo Kim <kpkim@gluesys.com>
This was referenced Sep 14, 2026
svc-nixl
referenced
this pull request
Sep 14, 2026
Remove the default rc_gda exclusion and the argument scan that supported it. Keep --disable-ll-nvlink on the conditional RDMA invocation while allowing UCX to select the transport for default runs.
svc-nixl
referenced
this pull request
Sep 14, 2026
Remove the default rc_gda exclusion and the argument scan that supported it. Keep --disable-ll-nvlink on the conditional RDMA invocation while allowing UCX to select the transport for default runs.
svc-nixl
referenced
this pull request
Sep 14, 2026
registerMemory() registered the destination MR with only FI_REMOTE_WRITE | FI_REMOTE_READ for all non-tcp/sockets providers. CXI only generates a target-side remote-write completion event when the destination MR key has events enabled (FI_RMA_EVENT). NIXL's libfabric backend protocol depends on that target-side completion (in combination with the SEND/RECV notification) to know a transfer has completed, so CXI destination MRs must be registered with FI_RMA_EVENT. Fix by adding a cxi-specific branch in registerMemory() that includes FI_RMA_EVENT in provider_access_flags. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved memory registration for CXI-based networking. * Preserved remote read and write access while enabling required remote memory events. * Other supported networking providers retain their existing memory access behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Ryan Hankins <ryan.hankins@hpe.com> Co-authored-by: Adit Ranadive <aranadive@nvidia.com>
This was referenced Sep 14, 2026
svc-nixl
referenced
this pull request
Sep 15, 2026
## What? `MpStoreTest.ForeignOwnedStoreIsIgnoredAndNotReapable` and `MpExporterTest.ForeignOwnedLockFileCannotSilenceTheRun` `chown()` a file to uid 65534 and then assert `prometheus_mp` ignores it, treating a `chown` that returns 0 as proof the owner changed. On the v1.5.0 RC0 environments it is not: the call succeeds and the file keeps its owner, so the product sees `st_uid == geteuid()`, takes the healthy path, and both tests fail on an environment property. They now read the owner back and skip when it did not change, reporting the effective uid, the post-`chown` `st_uid`, the filesystem type, the seccomp mode and the uid map. Test-only; where the handover works the tests run and assert exactly as before. ## Why? NVBug 6734875 / NIX-1806: both fail on all four executed RC0 environments, x86_64 and aarch64. The `st_uid` checks they cover are correct and unchanged -- the tests were asserting a precondition they never verified. Mutation-tested: a no-op `chown` makes both skip with the diagnostic, and bypassing the read-back reproduces the reported failures. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved diagnostics for multiprocess telemetry test environments, including filesystem, security, and ownership details. * Added shared handling for transferring file ownership during test setup. * Tests now provide clearer reasons when required permissions or ownership changes are unavailable and skip appropriately. * Consolidated ownership management across related telemetry tests, reducing duplicated setup logic and improving consistency. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
This was referenced Sep 15, 2026
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.
Fix for allowing blossom ci to be triggered without a /build comment in original commit we forgot to add condition to auth stage
What?
blossom ci trigger without /build comment
Why?
fixup for original chnage