Recovery: approvals and reset job creation - #110
Merged
Conversation
The recovery controller runs a hardware reset by creating a one-shot Job on the node hosting the broken GPU. Add the template it starts from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Tuomas Katila <tuomas.katila@intel.com>
tkatila
force-pushed
the
recovery-approvals
branch
from
September 3, 2026 11:12
e0c26e0 to
48f59de
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The re-approval path can consume a one-shot approval even when no recovery Job actually starts, which can incorrectly force re-approval and diverge from the intended consumption semantics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR advances the GPU recovery workflow by adding approval handling and reset Job creation/monitoring to the GPURecoveryPlan controller, plus lifecycle safeguards (finalizer) so in-flight recovery Jobs aren’t orphaned during plan deletion.
Changes:
- Add approval matching (including node label selectors) and event override handling, and create/reset recovery Jobs for approved events.
- Track Job outcomes to update event state, retain Jobs for diagnostics, and clean up Jobs on plan deletion via a new finalizer.
- Add a reset Job template (embedded deployment asset), expand RBAC, and significantly extend envtest/unit coverage around approvals/finalizers/job handling.
File summaries
| File | Description |
|---|---|
| internal/controller/gpurecoveryplan_helpers.go | Adds shared helpers for approvals (node label cache), Job terminal detection, status message capping, and plan/event state utilities. |
| internal/controller/gpurecoveryplan_controller.go | Implements finalizer handling, approval processing, recovery Job creation, Job status sync, and spec/status persistence ordering. |
| internal/controller/gpurecoveryplan_controller_test.go | Adds extensive tests for finalizers, approval matching/consumption, overrides, job retention, and persistence behavior. |
| internal/controller/gpurecoveryplan_const.go | Introduces constants for the plan finalizer and recovery Job labels. |
| config/rbac/role.yaml | Updates RBAC to allow updating/patching GPURecoveryPlans and managing finalizers. |
| charts/gpu-base-operator/templates/role.yaml | Mirrors RBAC updates in the Helm chart role template. |
| config/deployments/xpum/xpum-reset-job.yaml | Adds the reset Job YAML template used by the controller for reset actions. |
| config/deployments/deployments.go | Embeds and exposes the reset Job template via XpuManagerResetJob(). |
| config/deployments/deployments_test.go | Adds basic template existence checks and validates resetter container requirements. |
| .trivyignore.yaml | Extends ignore list to cover the new reset Job template where relevant. |
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This adds the two phases that get it out — matching it against spec.approvals, and running the reset Job the approval authorises. Approvals come in two shapes. An eventId approval names one event; a selector approval is standing policy, matched against the event's recovery type, its node name and its node's labels. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Tuomas Katila <tuomas.katila@intel.com>
tkatila
force-pushed
the
recovery-approvals
branch
from
September 3, 2026 11:46
48f59de to
52a05ca
Compare
pfl
approved these changes
Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is missing the reflash job creation. It's coming in a later PR.
While reviewing the PR content, I made some code restructuring which sadly makes the diff a bit confusing.