Auto-MNNVL: add MNNVL configuration and startup validation#346
Conversation
Ref: GREP-270 Add MNNVL configuration support to the Grove operator. This includes configuration, startup validation, and Helm chart integration. Changes: - Add MNNVLConfiguration struct with Enabled field (default: false) and to the OperatorConfiguration - Add validateMNNVLPrerequisites() to check for ComputeDomain CRD at startup - Update Helm chart values.yaml and _helpers.tpl with mnnvl.enabled - Add unit tests for config parsing and CRD detection logic When MNNVL is enabled but the ComputeDomain CRD is not installed, the operator exits with a non-zero exit code. Tested - UT
shayasoolin
left a comment
There was a problem hiding this comment.
LGTM, I'd consider just one thing:
Instead of "MNNVL MNNVLConfiguration" residing right inside OperatorConfiguration, maybe we should have a sub section called "NetworkAcceleration" with MNNVL as one of its field. In the end MNNVL is NVIDIA's accelerator and we better leave room for other technologies without having to break the API or having too many fields in the config struct's root level.
|
@unmarshall can you please review this as well. |
I think @shayasoolin makes a great point, I would say we should shift to this kind of approach before we merge. |
1d022e1
1d022e1 to
449c4fb
Compare
gflarity
left a comment
There was a problem hiding this comment.
PTAL at the mismatch, otherwise LGTM.
/kind feature
Ref: GREP-270
Add MNNVL configuration support to the Grove operator. This includes configuration, startup validation, and Helm chart integration.
Changes:
When MNNVL is enabled, but the ComputeDomain CRD is not installed, the operator exits with a non-zero exit code.
Tested