DPF Service Template and Configuration Generator for Doca Weave - #4404
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
Summary by CodeRabbit
WalkthroughThe PR updates weave chart and image metadata, Helm values, DHCP networks, flow-controller underlay interfaces, and DaemonSet extended resources. It also exposes ChangesWeave service configuration
BF4 Astra flavor
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ServiceDefinition
participant WeaveServiceBuilders
participant HelmValues
participant DpuServiceConfiguration
ServiceDefinition->>WeaveServiceBuilders: provide weave metadata and service settings
WeaveServiceBuilders->>HelmValues: render nested images, DHCP networks, and underlay interfaces
WeaveServiceBuilders->>DpuServiceConfiguration: set DaemonSet bf_sf resources
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/api-core/src/dpf_services.rs (2)
88-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicated weave chart/image constants.
DOCA_WEAVE_DHCP_AGENT_SERVICE_HELM_NAME/_HELM_VERSION/_IMAGE_NAME/_IMAGE_TAGare byte-for-byte identical to theirDOCA_WEAVE_FLOW_CONTROLLER_SERVICE_*counterparts, since both services ship from the samedpf-weavechart/image. Maintaining four duplicate pairs risks one being bumped without the other during a future version update.♻️ Suggested consolidation
-pub const DOCA_WEAVE_DHCP_AGENT_SERVICE_HELM_NAME: &str = "dpf-weave"; -pub const DOCA_WEAVE_DHCP_AGENT_SERVICE_HELM_VERSION: &str = "v26.5.0-1f8f4e1e"; -pub const DOCA_WEAVE_DHCP_AGENT_SERVICE_IMAGE_NAME: &str = "weave-system"; -pub const DOCA_WEAVE_DHCP_AGENT_SERVICE_IMAGE_TAG: &str = "v26.5.0-f2c9f7c4-nightly"; - -/// Weave flow (ovs) controller service definitions. -pub const DOCA_WEAVE_FLOW_CONTROLLER_SERVICE_HELM_NAME: &str = "dpf-weave"; -pub const DOCA_WEAVE_FLOW_CONTROLLER_SERVICE_HELM_VERSION: &str = "v26.5.0-1f8f4e1e"; -pub const DOCA_WEAVE_FLOW_CONTROLLER_SERVICE_IMAGE_NAME: &str = "weave-system"; -pub const DOCA_WEAVE_FLOW_CONTROLLER_SERVICE_IMAGE_TAG: &str = "v26.5.0-f2c9f7c4-nightly"; +pub const DOCA_WEAVE_SERVICE_HELM_NAME: &str = "dpf-weave"; +pub const DOCA_WEAVE_SERVICE_HELM_VERSION: &str = "v26.5.0-1f8f4e1e"; +pub const DOCA_WEAVE_SERVICE_IMAGE_NAME: &str = "weave-system"; +pub const DOCA_WEAVE_SERVICE_IMAGE_TAG: &str = "v26.5.0-f2c9f7c4-nightly";Then reference the shared constants from both
default_doca_weave_dhcp_agent_serviceanddefault_doca_weave_flow_controller_service.🤖 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 `@crates/api-core/src/dpf_services.rs` around lines 88 - 97, Consolidate the identical DHCP agent and flow controller weave chart/image constants in dpf_services.rs into one shared set, then update default_doca_weave_dhcp_agent_service and default_doca_weave_flow_controller_service to reference those shared constants. Remove the duplicated DOCA_WEAVE_DHCP_AGENT_SERVICE_* or DOCA_WEAVE_FLOW_CONTROLLER_SERVICE_* definitions while preserving the existing values and service configuration behavior.
585-629: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDuplicated port-name data between DHCP networks and underlay ports.
WEAVE_DHCP_AGENT_NETWORKSandWEAVE_FLOW_CONTROLLER_UNDERLAY_PORTSboth enumerate the same 8 switch-port names (r0swpln0…r3swpln1), but in different orders and as two independently maintained arrays. If the port topology is ever updated, editing one list without the other would silently produce a mismatched DHCP/underlay configuration with no compile-time or test signal, since the two are not derived from a shared source.Consider deriving both from a single canonical
WEAVE_ASTRA_PORTSlist (or pairing PCI address directly with the port in one source array) so the DHCP networks and underlay interfaces can never drift apart.🤖 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 `@crates/api-core/src/dpf_services.rs` around lines 585 - 629, Consolidate the duplicated switch-port definitions used by weave_dhcp_agent_networks and weave_flow_controller_underlay_interfaces into one canonical WEAVE_ASTRA_PORTS source, pairing each port with its PCI address or otherwise deriving both configurations from the same entries. Update both helper functions to consume that shared source while preserving their existing generated field values and output behavior.
🤖 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.
Nitpick comments:
In `@crates/api-core/src/dpf_services.rs`:
- Around line 88-97: Consolidate the identical DHCP agent and flow controller
weave chart/image constants in dpf_services.rs into one shared set, then update
default_doca_weave_dhcp_agent_service and
default_doca_weave_flow_controller_service to reference those shared constants.
Remove the duplicated DOCA_WEAVE_DHCP_AGENT_SERVICE_* or
DOCA_WEAVE_FLOW_CONTROLLER_SERVICE_* definitions while preserving the existing
values and service configuration behavior.
- Around line 585-629: Consolidate the duplicated switch-port definitions used
by weave_dhcp_agent_networks and weave_flow_controller_underlay_interfaces into
one canonical WEAVE_ASTRA_PORTS source, pairing each port with its PCI address
or otherwise deriving both configurations from the same entries. Update both
helper functions to consume that shared source while preserving their existing
generated field values and output behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 96f539dd-cda6-4e2b-98b9-6f07970b14de
📒 Files selected for processing (5)
crates/api-core/src/dpf_services.rscrates/dpf/src/flavor.rscrates/dpf/src/lib.rscrates/dpf/src/sdk.rscrates/dpf/src/types.rs
Signed-off-by: aadvani <aadvani@nvidia.com>
9d2fbf5 to
5b5767a
Compare
|
🌿 Preview your docs: https://nvidia-preview-pull-request-4404.docs.buildwithfern.com/infra-controller |
…IA#4404) Add rust code generator for dpuserviceconfiguration and dpuservicetemplate files for doca-weave-dhcp-agent and doca-weave-flow-controller ## Related issues NVIDIA#3205 ## Type of Change - [x ] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [ ] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Breaking Changes - [ ] **This PR contains breaking changes** ## Testing - [x] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) ## Additional Notes [dpuserviceconfiguration-doca-weave-dhcp-agent.yaml](https://github.com/user-attachments/files/30560955/dpuserviceconfiguration-doca-weave-dhcp-agent.yaml) [dpuserviceconfiguration-doca-weave-flow-controller.yaml](https://github.com/user-attachments/files/30560962/dpuserviceconfiguration-doca-weave-flow-controller.yaml) [dpuservicetemplate-doca-weave-dhcp-agent.yaml](https://github.com/user-attachments/files/30560971/dpuservicetemplate-doca-weave-dhcp-agent.yaml) [dpuservicetemplate-doca-weave-flow-controller.yaml](https://github.com/user-attachments/files/30561054/dpuservicetemplate-doca-weave-flow-controller.yaml) Signed-off-by: aadvani <aadvani@nvidia.com> Signed-off-by: Alex Ball <aball@nvidia.com>
Add rust code generator for dpuserviceconfiguration and dpuservicetemplate files for doca-weave-dhcp-agent and doca-weave-flow-controller
Related issues
#3205
Type of Change
Breaking Changes
Testing
Additional Notes
dpuserviceconfiguration-doca-weave-dhcp-agent.yaml
dpuserviceconfiguration-doca-weave-flow-controller.yaml
dpuservicetemplate-doca-weave-dhcp-agent.yaml
dpuservicetemplate-doca-weave-flow-controller.yaml