feat: support modal - #1
Conversation
Signed-off-by: kerthcet <kerthcet@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces the foundational control-plane pieces for Nebula’s provider-based GPU workload orchestration, including a Modal provider adapter, Pod admission gating (via mutating webhook), placement/health controllers, and deployment tooling/docs to run the manager in-cluster without cert-manager.
Changes:
- Adds provider abstractions + registry, shared error classification, and a Modal provider implementation backed by a catalog-driven rate card.
- Implements Pod opt-in admission (scheduling gate + toleration) and controllers for Pod placement and NodePool health/status.
- Adds deployment scripts, webhook TLS bootstrapping (self-signed), catalogs/config samples, and CI workflows (lint/unit/e2e).
Reviewed changes
Copilot reviewed 101 out of 104 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/utils/utils.go | Adds e2e helper utilities for running commands, installing cert-manager/prom-operator, etc. |
| test/e2e/e2e_suite_test.go | Adds Ginkgo e2e suite bootstrap (build/load image, cert-manager handling). |
| README.md | Updates repo name/description. |
| PROJECT | Adds Kubebuilder project config. |
| pkg/vnode/status.go | Adds virtual-node Pod status projection helpers. |
| pkg/vnode/handler_test.go | Adds vnode handler unit tests. |
| pkg/vnode/doc.go | Documents vnode package ownership model. |
| pkg/version/version.go | Adds build-stamped version helper. |
| pkg/util/claim.go | Adds shared deterministic claim naming helper. |
| pkg/util/accelerator.go | Adds GPU request parsing (type label + nvidia.com/gpu count). |
| pkg/util/accelerator_test.go | Adds tests for accelerator request parsing. |
| pkg/provider/registry.go | Adds process-wide provider registry. |
| pkg/provider/provider.go | Defines provider interface, types, and capabilities model. |
| pkg/provider/modal/client.go | Implements Modal SDK-backed client seam. |
| pkg/provider/errors.go | Adds shared provision error sentinels + block-scope classification. |
| pkg/provider/errors_test.go | Tests shared error classification behavior. |
| pkg/provider/catalog/data/modal.csv | Adds Modal price/availability CSV catalog. |
| pkg/provider/catalog/catalog.go | Adds embedded/override catalog loader and CSV parsing. |
| pkg/provider/catalog/catalog_test.go | Tests catalog loading/overrides and Base behavior. |
| pkg/provider/catalog/base.go | Adds reusable catalog-backed adapter base methods. |
| OWNERS | Sets approver/reviewer. |
| internal/webhook/v1/webhook_suite_test.go | Adds envtest-based webhook suite with skip behavior. |
| internal/webhook/v1/pod_webhook.go | Adds mutating webhook to inject scheduling gate + toleration. |
| internal/webhook/v1/pod_webhook_test.go | Adds unit tests for pod webhook mutations/idempotency. |
| internal/controller/suite_test.go | Adds envtest suite bootstrap for controllers (skippable). |
| internal/controller/pod_placement_controller.go | Adds Pod placement reconciler (gate → claim → select provider → ungate). |
| internal/controller/nodepool_validation_test.go | Adds envtest CEL validation coverage for NodePool spec. |
| internal/controller/nodepool_controller.go | Adds NodePool reconciler (policy validation + placed counts). |
| internal/controller/nodepool_controller_test.go | Adds fake-client unit tests for NodePool reconciler logic. |
| hack/gen-webhook-cert.sh | Adds self-signed webhook cert + caBundle injection script. |
| hack/deploy.sh | Adds deploy-all script (build/publish, secrets, deploy, CA injection). |
| hack/boilerplate.go.txt | Adds license boilerplate template. |
| go.mod | Defines module + dependencies (Modal SDK, VK, controller-runtime, etc.). |
| docs/deploy.md | Adds deployment guide covering secrets, TLS, and deploy-all flow. |
| Dockerfile | Adds version-stamped manager build and distroless runtime image. |
| config/webhook/service.yaml | Adds webhook Service. |
| config/webhook/selector_patch.yaml | Narrows webhook scope via object/namespace selectors. |
| config/webhook/manifests.yaml | Adds MutatingWebhookConfiguration manifest. |
| config/webhook/kustomizeconfig.yaml | Adds kustomize name/namespace reference config for webhooks. |
| config/webhook/kustomization.yaml | Kustomizes webhook resources + selector patch. |
| config/samples/nebula_v1alpha1_nodepool.yaml | Adds NodePool sample manifest. |
| config/samples/deployment.yaml | Adds opted-in GPU workload Deployment sample. |
| config/rbac/service_account.yaml | Adds manager ServiceAccount. |
| config/rbac/role.yaml | Adds manager ClusterRole permissions. |
| config/rbac/role_binding.yaml | Adds manager ClusterRoleBinding. |
| config/rbac/nodepool_viewer_role.yaml | Adds NodePool viewer helper role. |
| config/rbac/nodepool_editor_role.yaml | Adds NodePool editor helper role. |
| config/rbac/nodepool_admin_role.yaml | Adds NodePool admin helper role. |
| config/rbac/nodeclaim_viewer_role.yaml | Adds NodeClaim viewer helper role. |
| config/rbac/nodeclaim_editor_role.yaml | Adds NodeClaim editor helper role. |
| config/rbac/nodeclaim_admin_role.yaml | Adds NodeClaim admin helper role. |
| config/rbac/metrics_reader_role.yaml | Adds metrics reader ClusterRole. |
| config/rbac/metrics_auth_role.yaml | Adds metrics auth ClusterRole (tokenreviews/SARs). |
| config/rbac/metrics_auth_role_binding.yaml | Binds metrics auth role to manager SA. |
| config/rbac/leader_election_role.yaml | Adds leader-election Role. |
| config/rbac/leader_election_role_binding.yaml | Adds leader-election RoleBinding. |
| config/rbac/kustomization.yaml | Aggregates RBAC resources. |
| config/prometheus/monitor.yaml | Adds ServiceMonitor for controller metrics. |
| config/prometheus/monitor_tls_patch.yaml | Adds TLS patch for ServiceMonitor when using cert-manager. |
| config/prometheus/kustomization.yaml | Kustomization for prometheus monitor (patch commented). |
| config/network-policy/kustomization.yaml | Aggregates network policy resources. |
| config/network-policy/allow-webhook-traffic.yaml | NetworkPolicy for webhook ingress. |
| config/network-policy/allow-metrics-traffic.yaml | NetworkPolicy for metrics ingress. |
| config/manager/modal-credentials.example.yaml | Example Modal credentials Secret manifest. |
| config/manager/manager.yaml | Manager Deployment (catalog mount, optional provider secrets, etc.). |
| config/manager/kustomization.yaml | Kustomize manager resources + image override. |
| config/default/metrics_service.yaml | Metrics Service manifest. |
| config/default/manager_webhook_patch.yaml | Patch to mount webhook cert + args/ports. |
| config/default/manager_metrics_patch.yaml | Patch to bind metrics HTTPS port. |
| config/default/kustomization.yaml | Default deployment composition + patches (webhook, catalog, etc.). |
| config/default/cert_metrics_manager_patch.yaml | Patch for cert-manager-backed metrics cert mounting. |
| config/crd/kustomizeconfig.yaml | Kustomize name/namespace ref config for CRD conversion webhooks. |
| config/crd/kustomization.yaml | CRD kustomization (bases + scaffold hooks). |
| config/crd/bases/nebula.inftyai.com_nodepools.yaml | NodePool CRD definition. |
| config/crd/bases/nebula.inftyai.com_nodeclaims.yaml | NodeClaim CRD definition. |
| config/certmanager/kustomizeconfig.yaml | Kustomize nameRef config for cert-manager resources. |
| config/certmanager/kustomization.yaml | Cert-manager component resources. |
| config/certmanager/issuer.yaml | Selfsigned Issuer manifest. |
| config/certmanager/certificate-webhook.yaml | Webhook Certificate manifest. |
| config/certmanager/certificate-metrics.yaml | Metrics Certificate manifest. |
| config/catalog/kustomization.yaml | Generates catalog ConfigMap from checked-in CSVs. |
| api/v1alpha1/zz_generated.deepcopy.go | Generated deepcopy code for API types. |
| api/v1alpha1/nodepool_types.go | Adds NodePool API type and validation markers. |
| api/v1alpha1/nodeclaim_types.go | Adds NodeClaim API type. |
| api/v1alpha1/groupversion_info.go | Adds API group version + shared label/annotation keys. |
| .golangci.yml | Adds golangci-lint configuration. |
| .gitignore | Ignores local env/bin/devcontainer, etc. |
| .github/workflows/test.yml | Adds unit test workflow. |
| .github/workflows/test-e2e.yml | Adds kind-backed e2e workflow. |
| .github/workflows/lint.yml | Adds golangci-lint workflow. |
| .env.example | Adds secrets-only env template for deploy tooling. |
Files not reviewed (1)
- api/v1alpha1/zz_generated.deepcopy.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 101 out of 104 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- api/v1alpha1/zz_generated.deepcopy.go: Generated file
Comments suppressed due to low confidence (6)
test/utils/utils.go:51
- Run() ignores GetProjectDir() errors and also calls os.Chdir(), which mutates global process state and can introduce test flakiness when tests run concurrently. Since exec.Cmd already supports per-command working directories via cmd.Dir, handle the GetProjectDir error and remove the global chdir.
test/utils/utils.go:217 - Typo/grammar in error message: "to be uncomment" should be "to uncomment" (or "to be uncommented").
internal/controller/pod_placement_controller.go:23 - This file imports fmt only to build the claim name. Since util.ClaimName is the shared single source of truth (used by the vnode handler and NodeClaim controller), switch to util.ClaimName here and drop fmt to avoid the naming convention drifting.
internal/controller/pod_placement_controller.go:184 - claimNameForPod duplicates the ClaimName convention already centralized in util.ClaimName (and used by pkg/vnode/handler.go + internal/controller/nodeclaim_controller.go). Using a local fmt.Sprintf risks future drift and mismatched ids across components.
internal/webhook/v1/pod_webhook.go:128 - hasProviderToleration currently returns true for any toleration with the right key/effect, even if it only matches a specific provider value (Operator=Equal). Since the placement controller may choose any provider and overwrites the provider nodeSelector, this can incorrectly skip injecting the key-only Exists toleration and leave the Pod unschedulable on the chosen virtual node.
pkg/provider/modal/client.go:183 - sdkClient.ListSandboxes claims it filters Nebula-owned sandboxes, but it currently returns all sandboxes under the App. That can surface instances with empty ClaimTagKey/ClaimName into the control plane and also contradicts the Client contract in modal.go. Filter the returned list to only sandboxes tagged with ClaimTagKey (and update the comment accordingly).
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
|
/lgtm |
|
Sorry, @kerthcet is not authorized to approve/unapprove this PR. |
What this PR does / why we need it
Which issue(s) this PR fixes
Fixes #
Special notes for your reviewer
Does this PR introduce a user-facing change?