Skip to content

refactor(mnnvl): remove auto-MNNVL mutating webhook, switch to opt-in#559

Merged
shmuel-runai merged 4 commits into
ai-dynamo:mainfrom
shmuel-runai:grove-417/main-webhook-optin
Apr 30, 2026
Merged

refactor(mnnvl): remove auto-MNNVL mutating webhook, switch to opt-in#559
shmuel-runai merged 4 commits into
ai-dynamo:mainfrom
shmuel-runai:grove-417/main-webhook-optin

Conversation

@shmuel-runai

Copy link
Copy Markdown
Contributor

/kind feature

Summary

  • Remove MutateAutoMNNVL from the defaulting webhook — MNNVL is now purely opt-in via explicit grove.io/auto-mnnvl annotation
  • Remove networkConfig from the defaulting handler since it's no longer needed
  • Add validation rejecting MNNVL annotations on non-GPU PodCliques
  • Update E2E tests to explicitly opt-in to MNNVL via annotations

Key changes

  • webhook.go: Delete MutateAutoMNNVL, add non-GPU PCLQ validation in validatePodCliqueTemplateSpecOnCreate
  • defaulting/handler.go: Remove networkConfig field and MNNVL mutation from Default()
  • register.go: Simplify NewHandler(mgr) call (no longer needs network config)
  • E2E tests: Use buildGPUPCSWithMNNVL helper to explicitly set annotations; rewrite testPCSGetsAutoAnnotation to verify opt-in semantics

Which issue(s) this PR fixes

Ref #417

Test plan

  • Unit tests for ResolveGroupName in helpers_test.go
  • TestValidatePCSOnCreate_NonGPUCliqueWithMNNVL — validates rejection of MNNVL on non-GPU cliques
  • TestMNNVL_WebhookPipeline_LegacyPCSUpdate — simplified to single case (no mutation on update)
  • E2E tests updated to opt-in model
  • go build ./... and go test pass

Made with Cursor

Made with Cursor

Remove the MutateAutoMNNVL defaulting webhook that automatically injected
the grove.io/auto-mnnvl annotation onto GPU PodCliqueSets. MNNVL is now
purely opt-in: users must explicitly set the annotation on their PCS.

Add validation rejecting MNNVL annotations on non-GPU PodCliques.
Update E2E tests to explicitly opt-in to MNNVL via annotations.

Made-with: Cursor
@copy-pr-bot

copy-pr-bot Bot commented Apr 28, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@shmuel-runai shmuel-runai self-assigned this Apr 28, 2026
Comment thread operator/internal/mnnvl/webhook.go Outdated
Comment thread operator/internal/mnnvl/webhook.go Outdated
Comment thread operator/e2e/tests/auto-mnnvl/supported_and_enabled_test.go Outdated
Made-with: Cursor
shayasoolin
shayasoolin previously approved these changes Apr 29, 2026
Comment thread operator/internal/mnnvl/computedomain/computedomain.go Outdated
Comment thread operator/e2e/tests/auto-mnnvl/testutils.go
Comment thread operator/e2e/tests/auto-mnnvl/testutils.go
Made-with: Cursor
@shmuel-runai shmuel-runai merged commit d1852ce into ai-dynamo:main Apr 30, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants