Conversation
WalkthroughA new documentation file is added describing the DRA Attributes Downward API. The document outlines how DRA device attributes and networking information are exposed to containers via mounted JSON files, including node- and container-level paths, JSON schema structure, implementation workflow, lifecycle, and future considerations. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pull Request Test Coverage Report for Build 20714636028Details
💛 - Coveralls |
Signed-off-by: Or Shoval <oshoval@redhat.com>
|
cc @SchSeba Thanks for the co-op on this |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @enhancements/DownwardAPI.md:
- Around line 19-20: The second fenced code block containing the line
"/etc/dra/<claimName>.<requestName>.json" is missing a language specifier;
update that fence to include a language (e.g., replace the opening ``` with
```text) so the block becomes a ```text fenced block to satisfy markdown
linting.
- Around line 13-15: Update the fenced code block that contains the path
"/etc/dra/<pod_uid>/<claimName>.<requestName>.json" to include a language
specifier (e.g., add "text" after the opening backticks) so the block becomes a
proper markdown code fence with language declared.
🧹 Nitpick comments (1)
enhancements/DownwardAPI.md (1)
1-124: Documentation is clear and well-structured overall.The file effectively explains the downward API design, includes a practical JSON schema example, and references the upstream KEP. Consider enhancing it with:
- Error handling: What happens if files aren't available or fail to mount? Should applications gracefully handle missing files?
- File permissions: What are the expected ownership and permissions for these files?
- Application consumption: A brief example showing how an application would parse and use these files.
- Security considerations: Any sensitive data concerns with exposing device attributes and networking information?
These additions would make the documentation more actionable for developers implementing against this API.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
enhancements/DownwardAPI.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
enhancements/DownwardAPI.md
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Unit Tests
- GitHub Check: Build Container
- GitHub Check: Lint
| ``` | ||
| /etc/dra/<pod_uid>/<claimName>.<requestName>.json | ||
| ``` |
There was a problem hiding this comment.
Add language specification to fenced code block.
The first code block lacks a language specifier, which is flagged by markdown linting rules.
🔎 Proposed fix
-```
+```text
/etc/dra/<pod_uid>/<claimName>.<requestName>.json
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @enhancements/DownwardAPI.md around lines 13-15, Update the fenced code block
that contains the path "/etc/dra/<pod_uid>/<claimName>.<requestName>.json" to
include a language specifier (e.g., add "text" after the opening backticks) so
the block becomes a proper markdown code fence with language declared.
| ``` | ||
| /etc/dra/<claimName>.<requestName>.json |
There was a problem hiding this comment.
Add language specification to fenced code block.
The second code block lacks a language specifier, which is flagged by markdown linting rules.
🔎 Proposed fix
-```
+```text
/etc/dra/<claimName>.<requestName>.json
-```
+```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| /etc/dra/<claimName>.<requestName>.json |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @enhancements/DownwardAPI.md around lines 19-20, The second fenced code block
containing the line "/etc/dra/<claimName>.<requestName>.json" is missing a
language specifier; update that fence to include a language (e.g., replace the
opening ``` with ```text) so the block becomes a ```text fenced block to satisfy
markdown linting.
| "lastTransitionTime": "2024-01-15T10:00:00Z" | ||
| } | ||
| ], | ||
| "data": { |
There was a problem hiding this comment.
do we need to add here type ? for example network 0x as we discussed for SR-IOV ?
(btw not sure today Kubevirt is protected in this aspect of device mixing)
| /etc/dra/<claimName>.<requestName>.json | ||
| ``` | ||
|
|
||
| Each file corresponds to a unique `(claimName, requestName)` tuple, supporting scenarios with multiple resource claims per pod. |
There was a problem hiding this comment.
note that the example below doesn't adhere to it, so we need to raise it on the KEP or align here,
since as we spoke each clainName, requestName tuple can belong to a different driver, so we need a separate file per tuple or at least per driver ?
(while the example belows shows one file for all the requests)
|
@oshoval @SchSeba PTAL at the latest commit: kubernetes/enhancements@9ef91a9 Based on @SchSeba's inputs on data arriving later then DRA hook, I added some changes to the design, summary of it is here:
The directory structure combined with generationNumber allows the application to:
|
|
Also to be completely transparent, I need to spend more time in modeling this data: https://github.com/k8snetworkplumbingwg/dra-driver-sriov/pull/62/files#diff-e1130a488ec53121cb29a9b728dd15975a800ed1aab63867239265f2710e5ae7R41, its WIP. The plan is to get it into good enough state and get api-review so we can maintain this longterm, so please expect some changes here. |
|
Thanks @alaypatel07 Note: the above is not related to the fact NRI is not blocking and has 2 seconds timeout, for this Sebastian and Antonio discussed Machine config solution to increase the time. |
|
cc @SchSeba @aojea @alaypatel07 @EdDev beside the PR, please see also Even if we dont need #62 (comment) |
|
i wonder if we should just wait for KEP 5606 instead develop the downwardAPI by the SR-IOV driver meanwhile, moreover @alaypatel07 |
|
Hi @SchSeba Thanks ! |
3 participants
This enhancement provides containers with access to Dynamic Resource Allocation (DRA)
device attributes and networking information through files mounted into the container filesystem.
This allows applications to discover allocated SR-IOV VF attributes,
networking configuration, and CNI results without querying the Kubernetes API.
See kubernetes/enhancements#5606
Until it is part of K8s, implement it by the driver.
Thanks @SchSeba for the solution and co-op
Summary by CodeRabbit
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.