OSAC-497: Add Netris CaaS networking documentation#28
Conversation
Document how network traffic flows through the OSAC infrastructure when using Netris as the network backend. Covers the physical underlay (spine-leaf topology, switches, softgates), the logical overlay (VPCs, NAT rules, MetalLB), and how egress/ingress traffic is routed for both the management and hosted clusters.
WalkthroughA new documentation file Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
features/netris-caas-networking.md (1)
27-27: ⚡ Quick winConsider using the default Mermaid renderer for broader platform compatibility.
The
'elk'layout engine is not available by default in all Mermaid providers, including GitHub's renderer. ELK is delivered as a separate package (@mermaid-js/layout-elk) and must be explicitly registered—it is not guaranteed to work "out of the box" across different documentation platforms. Switching to the default renderer ensures the diagram renders correctly everywhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/netris-caas-networking.md` at line 27, The Mermaid init block currently forces the ELK layout engine via "defaultRenderer: 'elk'" which may not be available on all platforms; update the init configuration (the line containing %%{init: {'theme': 'dark', 'themeVariables': {'lineColor': '#ff9933'}, 'flowchart': {'defaultRenderer': 'elk'}}}%%) to omit the defaultRenderer setting (or change it to the built-in default) so the diagram uses Mermaid's standard renderer for broader compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@features/netris-caas-networking.md`:
- Line 77: The sentence contains the incorrect plural "leafs" in the phrase
"switches form a full mesh between all leafs"; update that phrase to use the
correct plural "leaves" (i.e., "switches form a full mesh between all leaves")
to fix the grammar in the document.
- Around line 230-235: The fenced code block containing the ASCII connection
flow ("Ansible controller -> SSH to bastion -> SSH to server mgmt NIC ->
nmstatectl apply...") lacks a language specifier and triggers markdownlint
MD040; fix it by changing the opening backticks to include the language `text`
(i.e., replace ``` with ```text) so the block reads as a text-coded fence and
preserves the ASCII art formatting.
---
Nitpick comments:
In `@features/netris-caas-networking.md`:
- Line 27: The Mermaid init block currently forces the ELK layout engine via
"defaultRenderer: 'elk'" which may not be available on all platforms; update the
init configuration (the line containing %%{init: {'theme': 'dark',
'themeVariables': {'lineColor': '#ff9933'}, 'flowchart': {'defaultRenderer':
'elk'}}}%%) to omit the defaultRenderer setting (or change it to the built-in
default) so the diagram uses Mermaid's standard renderer for broader
compatibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3a82374-b9c6-4375-95b8-3baf8685547c
📒 Files selected for processing (1)
features/netris-caas-networking.md
| ``` | ||
|
|
||
| Each server has two VPC NICs bonded across both leaf switches. Spine | ||
| switches form a full mesh between all leafs. SoftGates peer with the |
There was a problem hiding this comment.
Fix plural form of "leaf".
The standard plural of "leaf" is "leaves", not "leafs". This was flagged by the grammar checker.
📝 Proposed fix
-switches form a full mesh between all leafs. SoftGates peer with the
+switches form a full mesh between all leaves. SoftGates peer with theAs per coding guidelines, the static analysis tool LanguageTool flagged this as a spelling/grammar error.
📝 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.
| switches form a full mesh between all leafs. SoftGates peer with the | |
| switches form a full mesh between all leaves. SoftGates peer with the |
🧰 Tools
🪛 LanguageTool
[grammar] ~77-~77: Ensure spelling is correct
Context: ...e switches form a full mesh between all leafs. SoftGates peer with the upstream route...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/netris-caas-networking.md` at line 77, The sentence contains the
incorrect plural "leafs" in the phrase "switches form a full mesh between all
leafs"; update that phrase to use the correct plural "leaves" (i.e., "switches
form a full mesh between all leaves") to fix the grammar in the document.
| ``` | ||
| Ansible controller | ||
| -> SSH to bastion (management network) | ||
| -> SSH to server mgmt NIC (eno1, DHCP IP) | ||
| -> nmstatectl apply: configures VPC NICs with static IPs and routes | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to code block.
The fenced code block should specify a language for proper syntax highlighting and to satisfy markdown linting rules. Since this is ASCII art showing a connection flow, use text as the language specifier.
📝 Proposed fix
-```
+```text
Ansible controller
-> SSH to bastion (management network)
-> SSH to server mgmt NIC (eno1, DHCP IP)As per coding guidelines, markdownlint flagged this as MD040 (fenced-code-language).
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 230-230: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@features/netris-caas-networking.md` around lines 230 - 235, The fenced code
block containing the ASCII connection flow ("Ansible controller -> SSH to
bastion -> SSH to server mgmt NIC -> nmstatectl apply...") lacks a language
specifier and triggers markdownlint MD040; fix it by changing the opening
backticks to include the language `text` (i.e., replace ``` with ```text) so the
block reads as a text-coded fence and preserves the ASCII art formatting.
|
cc @AlonaKaplan |
|
/lgtm |
mhrivnak
left a comment
There was a problem hiding this comment.
This is a good doc. Thanks for creating it.
| ```mermaid | ||
| %%{init: {'theme': 'dark', 'themeVariables': {'lineColor': '#ff9933'}, 'flowchart': {'defaultRenderer': 'elk'}}}%% | ||
| graph TB | ||
| internet((Internet)) |
There was a problem hiding this comment.
Just looking at the diagram, it seems unexpected that worker nodes communicate with their hosted control plane through the "Internet".
There was a problem hiding this comment.
via the internet here means they need to go through the gateway, its is not actually leaving to the internet. I agree it has pros and cons, this was the initial implementation which was consistent with what was done in MOC
There was a problem hiding this comment.
WDYT about updating the diagram to clarify? I think a lot of readers could get the wrong impression that we are sending traffic to the internet.
| ``` | ||
| Ansible controller | ||
| -> SSH to bastion (management network) | ||
| -> SSH to server mgmt NIC (eno1, DHCP IP) |
There was a problem hiding this comment.
Is this mgmt NIC the BMC or equivalent? I'm trying to understand how such an ssh session is getting access to the node's OS in order to run nmstatectl there.
There was a problem hiding this comment.
this is NIC which assigned IP via mgmt DHCP. The relevant keys are provided as secrets.
There was a problem hiding this comment.
Is this a NIC that's part of the normal system, and managed by the host OS? Or is this NIC part of the BMC that would often be accessed by redfish?
| Ansible controller | ||
| -> SSH to bastion (management network) | ||
| -> SSH to server mgmt NIC (eno1, DHCP IP) | ||
| -> nmstatectl apply: configures VPC NICs with static IPs and routes |
There was a problem hiding this comment.
Is it possible / reasonable to use the nmstate operator instead?
There was a problem hiding this comment.
I am not sue you can use nmstate operator for non OCP nodes
Summary