Nvidia hardware profile - #1382
Conversation
There was a problem hiding this comment.
Pull request overview
Adds NVIDIA (Spectrum-based) Cumulus switch profiles and related schema/validation adjustments so profiles can be validated and port breakout naming can be translated correctly (helping prevent typos in profile definitions).
Changes:
- Added NVIDIA Spectrum-2/3 silicon identifiers and two new NVIDIA switch profiles (SN3700C, SN4600) to the default Cumulus catalog.
- Updated SwitchProfile validation and API↔NOS port mapping to better handle Cumulus (MLX/VX) breakout naming.
- Extended CRDs/docs (and tests) with a new
nosFamilyfield and added a Cumulus-focused port-mapping test case.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/ctrl/switchprofile/silicons.go | Adds Spectrum 2/3 silicon constants for NVIDIA profiles. |
| pkg/ctrl/switchprofile/p_nv_msn4600.go | Introduces NVIDIA SN4600 SwitchProfile definition (ports, breakouts, pipelines). |
| pkg/ctrl/switchprofile/p_nv_msn3700c.go | Introduces NVIDIA SN3700C SwitchProfile definition (ports, breakouts). |
| pkg/ctrl/switchprofile/catalog.go | Registers the new NVIDIA profiles in the default Cumulus profile list. |
| pkg/ctrl/switchprofile/README.md | Adds guidance for gathering platform info and validating breakouts on Cumulus. |
| docs/api.md | Documents the new nosFamily field on port profiles. |
| config/crd/bases/wiring.githedgehog.com_switchprofiles.yaml | Adds nosFamily to SwitchProfile CRD schema. |
| config/crd/bases/agent.githedgehog.com_agents.yaml | Adds nosFamily to Agent CRD schema (embedded SwitchProfile schema). |
| api/wiring/v1beta1/switchprofile_types_test.go | Adds/updates tests for Cumulus port breakout translation behavior. |
| api/wiring/v1beta1/switchprofile_types.go | Adds nosFamily to API type, adjusts validation rules, and updates breakout mapping for Cumulus. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| nosNameBaseStr, cut := strings.CutPrefix(port.BaseNOSName, DataPortNOSNamePrefix) | ||
| if !cut { | ||
| return nil, errors.Errorf("port %q base NOS name %q is invalid (no expected prefix)", portName, port.NOSName) | ||
| } | ||
| nosNameBase, err := strconv.Atoi(nosNameBaseStr) | ||
| if err != nil { | ||
| return nil, errors.Errorf("port %q NOS name %q breakout mode %q offset %q is invalid (not a number)", portName, port.NOSName, swBreakout, offsetStr) | ||
| return nil, errors.Errorf("port %q base NOS name %q is invalid (suffix isn't a number)", portName, port.NOSName) | ||
| } |
There was a problem hiding this comment.
These errors are reporting the wrong value: they say “base NOS name %q” but format port.NOSName. This makes debugging invalid BaseNOSName values hard. Use port.BaseNOSName in both messages so the user sees the actual base name that failed parsing.
1b50e3c to
0b1ab4e
Compare
Frostman
left a comment
There was a problem hiding this comment.
Didn't yet had a look at the code, just cosmetics first
9ad497e to
28d4660
Compare
6479a45 to
2c6adb5
Compare
|
🚀 Temp artifacts published: |
2c6adb5 to
936eff4
Compare
|
🚀 Temp artifacts published: |
|
🚀 Temp artifacts published: |
6155e34 to
3e64862
Compare
|
🚀 Temp artifacts published: |
|
🚀 Temp artifacts published: |
950c5db to
38a163f
Compare
|
🚀 Temp artifacts published: |
84e5f67 to
4737ddb
Compare
|
🚀 Temp artifacts published: |
|
🚀 Temp artifacts published: |
|
🚀 Temp artifacts published: |
2608591 to
a02d1f2
Compare
|
🚀 Temp artifacts published: |
a02d1f2 to
8c272be
Compare
|
🚀 Temp artifacts published: |
8c272be to
00410c0
Compare
|
🚀 Temp artifacts published: |
00410c0 to
a7b5365
Compare
|
🚀 Temp artifacts published: |
a7b5365 to
6cea609
Compare
|
🚀 Temp artifacts published: |
6cea609 to
3729d46
Compare
|
🚀 Temp artifacts published: |
3729d46 to
c1403ae
Compare
|
🚀 Temp artifacts published: |
c1403ae to
f80edbc
Compare
|
🚀 Temp artifacts published: |
Signed-off-by: Logan Blyth <logan@githedgehog.com>
Signed-off-by: Logan Blyth <logan@githedgehog.com>
Signed-off-by: Logan Blyth <logan@githedgehog.com>
Signed-off-by: Logan Blyth <logan@githedgehog.com>
Signed-off-by: Logan Blyth <logan@githedgehog.com>
f80edbc to
fe9b992
Compare
|
🚀 Temp artifacts published: |
closes #1347
In conversation the goal of the validation is to make sure that we don't commit typos in the profile.