Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update toolkit overview: add components, remove diagram #157

Merged
merged 1 commit into from
Feb 19, 2025
Merged

Update toolkit overview: add components, remove diagram #157

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions container-toolkit/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,17 @@ Container Device Interface <cdi-support.md>
docker-specialized.md
```

The NVIDIA Container Toolkit enables users to build and run GPU-accelerated containers.
The toolkit includes a container runtime [library](https://github.com/NVIDIA/libnvidia-container)
and utilities to automatically configure containers to leverage NVIDIA GPUs.
The NVIDIA Container Toolkit is a collection of libraries and utilities enabling users to build and run GPU-accelerated containers. It currently includes:

```{image} https://cloud.githubusercontent.com/assets/3028125/12213714/5b208976-b632-11e5-8406-38d379ec46aa.png
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose removing the diagram. It might be a provocative change. My main argument is that the diagram does not help answering what the toolkit is and where it fits the big picture. In particular, it does not mention: the container toolkit, the runtime library, i.e. it does not really help clarify what the toolkit is and does. It however mentions a different toolkit, and that might be confusing.

That's the diagram:
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm ok with that. The image looks pretty, but doesn't add real value in this context.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to removing it if its not right/not adding value.

Is there a diagram that would make sense here? I'm not sure how this image got created, but if there is a diagram we'd like to see here, I can do more investigating into getting them created.

Copy link
Contributor Author

@jgehrcke jgehrcke Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a diagram that would make sense here?

I think it's ok for today to not investigate creating a new one. We will see. Glad we agree on the removal.

:width: 400
```
* The NVIDIA Container Runtime (`nvidia-container-runtime`)
* The NVIDIA Container Toolkit CLI (`nvidia-ctk`)
* The NVIDIA CDI Hooks (`nvidia-cdi-hook`)
* The NVIDIA Container Runtime Hook (`nvidia-container-runtime-hook`)
* The NVIDIA Container CLI (`nvidia-container-cli`)
* The NVIDIA Container Library (`libnvidia-container1`)
Copy link
Contributor Author

@jgehrcke jgehrcke Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can think about linking out to the github repos of those components (not sure how we feel about that from within the docs -- are most links in docs expected to be cross-references into other parts in docs?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about links to the GitHub repo(s) at this stage as that adds additional questions. For example, the first four components listed here are developed from the nvidia-container-toolkit repo and the last two from libnvidia-container. We are moving towards just needing a single one and maybe we can add the reference then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, it might be too much have the links here to each component repo at this point in the docs. If we wanted to include links to the repo here, it might make sense to do that in a table format. So we could include a "Repo link" column in the and visually show that some components are in the same repo.

In general, I'm pro adding links and cross-referencing things. No real restrictions on adding links to github repos or other docs sites (like the k8s.io site) to help explain concepts. As long as its adding value and not chaos. I guess the only note there is if we have docs/explanations in a repo that should live in the docs site, then we should add it to the docs site, instead of linking to the repo from the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the exhaustive feedback to both of you! I will leave this as-is.

No real restrictions on adding links to github repos or other docs sites (like the k8s.io site) to help explain concepts.

Thanks for confirming! :)


## License

The NVIDIA Container Toolkit (and all included components) is licensed under [Apache 2.0](https://www.apache.org/licenses/LICENSE-2.0) and
contributions are accepted with a DCO. See the [contributing](https://github.com/NVIDIA/nvidia-container-toolkit/blob/master/CONTRIBUTING.md) document for
contributions are accepted with a Developer Certificate of Origin (DCO). See the [contributing](https://github.com/NVIDIA/nvidia-container-toolkit/blob/master/CONTRIBUTING.md) document for
Copy link
Contributor Author

@jgehrcke jgehrcke Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit

more information.