Restore NGC workflow for run_cloudxr_via_docker.sh#261
Merged
Conversation
📝 WalkthroughWalkthroughIntroduces a containerized CloudXR runtime service via Docker, including a multi-stage Dockerfile, Python-based runtime wrapper, initialization entrypoint script, docker-compose orchestration with OpenXR volume mounting, and updated build tooling to handle SDK procurement and service deployment. Changes
Sequence Diagram(s)sequenceDiagram
participant Host as Docker Host
participant Compose as Docker Compose
participant Build as Runtime Container
participant Entrypoint as entrypoint-ngc.sh
participant Python as main.py
participant Native as libcloudxr.so
Host->>Compose: docker-compose up cloudxr-runtime
Compose->>Build: Build multi-stage image
Note over Build: Stage 0: Extract SDK<br/>Stage 1: Setup runtime
Compose->>Build: Start container
Build->>Entrypoint: Execute entrypoint
Entrypoint->>Entrypoint: Create /openxr/run<br/>Copy artifacts
Entrypoint->>Python: Execute main.py
Python->>Native: ctypes.CDLL(libcloudxr.so)
Python->>Native: nv_cxr_service_create()
Python->>Python: Register SIGINT/SIGTERM
Python->>Native: nv_cxr_service_start()
Python->>Native: nv_cxr_service_join()
Note over Python: Awaiting shutdown signal
Python->>Native: nv_cxr_service_destroy()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
aristarkhovNV
force-pushed
the
aaristarkhov/ngc-run-via-docker
branch
from
87371ed to
7b4774f
Compare
aristarkhovNV
force-pushed
the
aaristarkhov/ngc-run-via-docker
branch
from
7b4774f to
5fca204
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deps/cloudxr/docker-compose.yaml`:
- Around line 5-29: The cloudxr-runtime service in the docker-compose config is
missing a restart policy so it won't auto-recover if it crashes; update the
cloudxr-runtime service definition (service name: cloudxr-runtime) to include a
restart policy consistent with the other services (e.g., add restart:
unless-stopped) directly under the service's top-level keys so the container
restarts on failure and recovers dependencies like wss-proxy.
- Around line 67-73: Add a clarifying comment to the docker-compose volume
definition for openxr-volume explaining that the host path referenced by
${CXR_HOST_VOLUME_PATH} must be created beforehand by the setup script;
specifically mention the setup_cloudxr_env.sh step (it runs mkdir -p
"$CXR_HOST_VOLUME_PATH") and that run_cloudxr_via_docker.sh sources that script
before docker-compose up so users should run the provided startup flow rather
than launching docker-compose directly.
In `@deps/cloudxr/Dockerfile.runtime-ngc`:
- Around line 30-41: The apt-get install line in the RUN instruction (the block
installing libatomic1, libbsd0, libegl1, libgl1, libglx0, libvulkan1, libx11-6,
libxext6, python3) is unpinned which triggers DL3008 and causes non-reproducible
images; either pin critical packages by specifying explicit versions (use
package=version in the RUN apt-get install invocation after apt-get update) or
add a clear comment above this RUN explaining you intentionally left packages
unpinned for maintenance reasons and documenting that reproducibility is not
required, then update CI/docs accordingly to reflect the chosen strategy.
- Around line 49-50: The Dockerfile RUN line uses chmod 777 on /openxr and
/openxr/run which is overly permissive; change this to more restrictive
permissions (e.g., 755 or 775) and ensure the runtime user can write by setting
ownership to the target UID/GID (use the build/run environment variables
${CXR_UID:-1000} and ${CXR_GID:-1000}) instead of giving world-writable
access—update the RUN command that creates /openxr and /openxr/run to chown
those directories to ${CXR_UID:-1000}:${CXR_GID:-1000} and apply chmod 755 or
775 as appropriate so the non-root container user has the needed access without
777.
In `@deps/cloudxr/runtime/main.py`:
- Around line 7-8: Wrap the libc load and service creation with explicit error
handling: catch exceptions raised by ctypes.CDLL("libcloudxr.so") (the lib
symbol) and raise/log a clearer diagnostic that includes the original exception
and a hint about container library paths; after calling
lib.nv_cxr_service_create(...) check the returned status and the svc
(ctypes.c_void_p instance) for a non-zero/NULL handle, and if creation failed
raise/log a descriptive error and abort further calls that assume a valid svc
handle. Ensure you reference and validate the lib.nv_cxr_service_create call and
the svc variable so subsequent calls never run on an invalid handle.
- Around line 11-12: The stop() handler currently calls
lib.nv_cxr_service_stop(svc) unconditionally; add a state guard like
state["service_created"] before invoking lib.nv_cxr_service_stop to avoid
stopping a non-created or already-destroyed service, and ensure you set
state["service_created"] = True right after successful lib.nv_cxr_service_create
(and clear it when destroying) so stop() checks the same flag before calling
lib.nv_cxr_service_stop(svc).
- Around line 15-20: The native calls nv_cxr_service_create,
nv_cxr_service_start, nv_cxr_service_join, and nv_cxr_service_destroy currently
ignore return values; update the code to capture each call's return code and
validate it (e.g., rc = lib.nv_cxr_service_create(ctypes.byref(svc))) then on
non-success log an error including the return code and perform appropriate error
handling (raise an exception or exit/cleanup the svc) to avoid proceeding after
a failed create/start/join/destroy; ensure svc is only used or destroyed if
create succeeded and that all failures produce clear logs mentioning the
function name (nv_cxr_service_create/start/join/destroy) and the rc.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4de4e169-1893-4386-849f-28c4a489cf46
📒 Files selected for processing (5)
deps/cloudxr/Dockerfile.runtime-ngcdeps/cloudxr/docker-compose.yamldeps/cloudxr/runtime/entrypoint-ngc.shdeps/cloudxr/runtime/main.pyscripts/run_cloudxr_via_docker.sh
There was a problem hiding this comment.
Pull request overview
Restores the CloudXR “NGC runtime SDK + Docker” workflow for run_cloudxr_via_docker.sh by switching the dev compose setup to build/run a minimal runtime image directly from the downloaded CloudXR Runtime SDK.
Changes:
- Add runtime SDK download step to the Docker launch script.
- Move the CloudXR runtime service definition into
deps/cloudxr/docker-compose.yaml(no longer requiringdocker-compose.runtime.yamlfor this workflow). - Introduce an NGC-focused runtime image (
Dockerfile.runtime-ngc) and entrypoint + minimal Python runtime launcher.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/run_cloudxr_via_docker.sh | Ensures runtime SDK is present before composing services. |
| deps/cloudxr/runtime/main.py | Adds a minimal ctypes-based CloudXR runtime launcher. |
| deps/cloudxr/runtime/entrypoint-ngc.sh | Prepares /openxr runtime artifacts and gates startup on EULA acceptance. |
| deps/cloudxr/docker-compose.yaml | Adds cloudxr-runtime service and shared bind-mounted volume configuration. |
| deps/cloudxr/Dockerfile.runtime-ngc | Builds a minimal runtime image from the downloaded SDK tarball. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
life1ess
approved these changes
Mar 13, 2026
jiwenc-nv
pushed a commit
that referenced
this pull request
Mar 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.
Summary by CodeRabbit