Skip to content

feat(signal): add grouped certification controller - #949

Closed
Kevin Cho (chokevin) wants to merge 2 commits into
Azure:mainfrom
chokevin:chokevin-upstream-dapple-certification
Closed

Kevin Cho (chokevin) wants to merge 2 commits into
Azure:mainfrom
chokevin:chokevin-upstream-dapple-certification

Conversation

@chokevin

@chokevin Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Project Signal had no upstream control plane for admitting approved Nodes to operator-defined certification workflows, and the original implementation stopped at one participant. This change adds the generic Kubernetes controller and now admits bounded Node groups atomically, while intentionally leaving scripts, execution images, WorkflowTemplates, backend installation, and backend runner permissions to downstream operators.

  • Adds CertificationProfile and CertificationRun APIs with immutable execution inputs and generated CRDs.
  • Selects up to 64 participants deterministically, reserves the complete group with rollback on partial failure, and recovers all group reservations after a missing run.
  • Binds every participant by ordered index, Node name, immutable UID, capacity, and allocatable-resource snapshot.
  • Passes both the compatibility Node list and a structured participants-json contract to the operator-supplied Argo WorkflowTemplate.
  • Requires successful grouped runs to return bounded backend resource identities covering every participant. Missing, malformed, duplicate, oversized, or incomplete backend evidence remains Inconclusive.
  • Preserves retries, cancellation, terminal evidence retention, cleanup, recurring policy, and monotonic Node projection across every participant.
  • Accounts the complete selected group against recurring failure-domain budgets before admission and after a new run is created.
  • Installs only the controller ServiceAccount/RBAC and Deployment. It does not grant permissions for MPIJobs, TrainJobs, or another workload backend.

The grouped contract treats participants as one certification boundary, not automatically as one distributed process group. Downstream analysis found that 13 of 15 current checks are independent per-Node diagnostics; the two MPI checks require a local OpenMPI launcher/worker pair on each tested Node rather than a cross-Node MPI world.

Read-only Dapple discovery confirmed that kubeflow.org/v2beta1 MPIJob and its Kueue integration are installed and operational, while the Trainer v2 TrainJob CRD/controller are absent. The downstream integration therefore targets MPIJob now. TrainJob remains a future adapter option behind the same upstream interface after a separately validated Trainer installation and MPI TrainingRuntime.

No certification scripts, runner build recipe, bundled CertificationProfiles, WorkflowTemplates, hardware thresholds, captured hardware output, or environment-specific identifiers are published in this PR.

Verification

  • Passed: go test ./deploy/signal ./internal/signal ./cmd/project-signal-controller
  • Passed: go build ./cmd/project-signal-controller
  • Passed: golangci-lint run ./api/signal/v1alpha1 ./internal/signal ./cmd/project-signal-controller with 0 issues
  • Passed: make signal-manifests
  • Passed: git diff --check
  • Not run: live grouped Dapple execution; it remains gated on the downstream MPIJob WorkflowTemplate/renderer and a bounded validation approval.

The upstream surface contains 26 changed files, 7,068 additions, and 9 deletions across two commits.

Risk

The main compatibility surface is the WorkflowTemplate interface. Existing single-participant templates can continue using participants; grouped templates must consume participants-json and publish complete backend-resources identity coverage before a Pass is accepted. Rollback is the previous controller commit plus CRD version.

Review guide

  1. Start with api/signal/v1alpha1/ for participant bounds, immutable snapshots, and backend resource identity status.
  2. Review internal/signal/controller.go for deterministic selection, atomic reservations, orphan recovery, Workflow inputs, and fail-closed grouped completion.
  3. Review internal/signal/policy.go for whole-group failure-domain accounting.
  4. Review internal/signal/controller_test.go and internal/signal/policy_test.go for multi-Node admission, partial rollback, restart recovery, backend coverage, and policy edge cases.
  5. Review deploy/signal/README.md for the public operator contract and MPIJob/TrainJob boundary.
  6. Review deploy/signal/manifest_test.go for the upstream publication boundary.

Scope boundary

This is generic controller infrastructure, not a published certification program. Operators remain responsible for reviewing and supplying scripts, images, policies, workload adapters, queue bindings, backend RBAC, cleanup behavior, and qualification evidence.

AI assistance: GitHub Copilot assisted with implementation, tests, investigation, documentation, and PR preparation. The author verified the claims and final text.

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

2 similar comments
@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin
Kevin Cho (chokevin) marked this pull request as ready for review October 20, 2026 02:41
@chokevin
Kevin Cho (chokevin) requested a review from a team October 20, 2026 02:41
@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

7 similar comments
@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Redacted: pre-publication environment-specific validation details were removed because they are outside the scope of this upstream controller PR.

@chokevin
Kevin Cho (chokevin) marked this pull request as draft October 20, 2026 20:37
@chokevin Kevin Cho (chokevin) changed the title feat(signal): add recoverable Dapple certification gate feat(signal): integrate Project Signal certification suite Oct 20, 2026
@chokevin Kevin Cho (chokevin) changed the title feat(signal): integrate Project Signal certification suite feat(signal): add generic certification controller Oct 20, 2026
@chokevin
Kevin Cho (chokevin) force-pushed the chokevin-upstream-dapple-certification branch from 484e5c2 to 7b88e4e Compare October 20, 2026 21:37
@chokevin Kevin Cho (chokevin) changed the title feat(signal): add generic certification controller feat(signal): add grouped certification controller Oct 20, 2026
@chokevin

Copy link
Copy Markdown
Contributor Author

Evidence: the 11-second recording shows the exact grouped-participant contract additions, then runs the focused multi-Node admission, rollback, backend identity, and recurring failure-domain tests.

signal-grouped-evidence.mp4

@chokevin

Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

The generic upstream participant backend adapter is now available as draft #963: #963

It is intentionally opened against main, so its interim diff includes #949 until that parent merges. The PR identifies the dependency and adapter commit explicitly; after #949 merges, GitHub will reduce the diff to the adapter layer. It adds optional Pod/MPIJob execution, WorkflowTemplate/RBAC wiring, UID-backed backend identity collection, fail-closed result aggregation, cleanup, tests, and a container definition while excluding certification scripts and Dapple-specific configuration.

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.

1 participant