chore(rbac): add events:create and jobs:patch/get permissions for EvalHub - #842
Conversation
…lHub ServiceAccount Add two new ClusterRoles for the EvalHub ServiceAccount: - evalhub-events: grants events create, so the EvalHub server can emit Kubernetes Events against backing Job resources on evaluation lifecycle transitions (EvaluationStarted, EvaluationCompleted, EvaluationFailed, EvaluationThresholdViolated) - evalhub-jobs-writer extended with get and patch verbs, so the server can read-modify-patch the trustyai.opendatahub.io/evaluation-phase label on running Jobs Both new ClusterRoleBindings are added to the OPA policy allowlist. No functional code changes; this is the RBAC prerequisite for RHAI-277.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change adds EvalHub event RBAC resources, registers them in Kustomize, updates RBAC policy allowlists, and permits the jobs writer role to get and patch batch jobs. ChangesEvalHub RBAC
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/components/evalhub/rbac/evalhub_events_role.yaml`:
- Around line 15-17: Add the patch verb to the core events rule in the EvalHub
events RBAC configuration, preserving the existing create permission so
EventRecorder can update existing event objects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c40c7e65-db5a-4f7c-b9ce-d7cb7c7c9ef0
📒 Files selected for processing (5)
config/components/evalhub/kustomization.yamlconfig/components/evalhub/rbac/evalhub_events_binding.yamlconfig/components/evalhub/rbac/evalhub_events_role.yamlconfig/components/evalhub/rbac/evalhub_jobs_writer_role.yamlpolicy/rbac.rego
…rder deduplication
6c493ae
into
trustyai-explainability:dev/RHAI-277
…lHub (#842) (#843) * chore(rbac): add events:create and jobs:patch/get permissions for EvalHub ServiceAccount Add two new ClusterRoles for the EvalHub ServiceAccount: - evalhub-events: grants events create, so the EvalHub server can emit Kubernetes Events against backing Job resources on evaluation lifecycle transitions (EvaluationStarted, EvaluationCompleted, EvaluationFailed, EvaluationThresholdViolated) - evalhub-jobs-writer extended with get and patch verbs, so the server can read-modify-patch the trustyai.opendatahub.io/evaluation-phase label on running Jobs Both new ClusterRoleBindings are added to the OPA policy allowlist. No functional code changes; this is the RBAC prerequisite for RHAI-277. * fix(rbac): add patch verb to evalhub-events ClusterRole for EventRecorder deduplication
…n failures (RHAI-278) (#846) * chore(rbac): add events:create and jobs:patch/get permissions for EvalHub (#842) * chore(rbac): add events:create and jobs:patch/get permissions for EvalHub ServiceAccount Add two new ClusterRoles for the EvalHub ServiceAccount: - evalhub-events: grants events create, so the EvalHub server can emit Kubernetes Events against backing Job resources on evaluation lifecycle transitions (EvaluationStarted, EvaluationCompleted, EvaluationFailed, EvaluationThresholdViolated) - evalhub-jobs-writer extended with get and patch verbs, so the server can read-modify-patch the trustyai.opendatahub.io/evaluation-phase label on running Jobs Both new ClusterRoleBindings are added to the OPA policy allowlist. No functional code changes; this is the RBAC prerequisite for RHAI-277. * fix(rbac): add patch verb to evalhub-events ClusterRole for EventRecorder deduplication * feat(evalhub): emit events for job infrastructure failures (#844) * feat(evalhub): emit events for job infrastructure failures Patch evaluation-phase=Failed label and evaluation-status annotation on Job resources. Include deduplication logic to skip event emission if the EvalHub server has already marked the failure. * fix(evalhub): emit EvaluationFailed event only after successful EvalHub POST * test(evalhub): add lifecycle unit tests for failure reconciler event emission (RHOAIENG-80114) (#845) * feat(evalhub): emit events for job infrastructure failures Patch evaluation-phase=Failed label and evaluation-status annotation on Job resources. Include deduplication logic to skip event emission if the EvalHub server has already marked the failure. * fix(evalhub): emit EvaluationFailed event only after successful EvalHub POST * test(evalhub): add lifecycle unit tests for failure reconciler event emission (RHOAIENG-80114) Cover the full Reconcile() path for EvalHubEvaluationJobFailureReconciler and EvalHubEvaluationFailedKueueWorkloadsReconciler using fake.Client and record.FakeRecorder. Tests use a local httptest.Server to absorb EvalHub POST calls and an interceptor to capture the evaluation-phase=Failed label patch before job deletion. Cases added: - dedup: server already set evaluation-phase=Failed → no operator event - OOM-killed adapter container → EvaluationFailed event + label patch + job cleanup - ErrImagePull on init container → EvaluationFailed event + label patch + job cleanup - Kueue QuotaReserved=Inadmissible → EvaluationFailed event + job label + workload annotation * fix(evalhub): prevent duplicate POST and Event on Kueue workload retry Annotate the Workload before emitting the Event and patching Job labels so that workloadFailedEventAlreadyReported acts as the primary dedup gate on retry. Previously, if both patchJobFailureLabels and annotateWorkloadReported failed after a successful POST, the next reconcile would re-POST to EvalHub and re-emit the Event. * fix(evalhub): retry Job failure labels after partial Workload commit When the Workload annotation succeeds but patchJobFailureLabels fails, the Job permanently lacks the evaluation-phase and evaluation-status metadata. Requeue on patchJobFailureLabels failure and add a retry branch that patches only the Job labels when the Workload annotation is already present. * fix(evalhub): clean up Job when server already handled failure The serverAlreadyHandledFailure early-return path skipped Job deletion, leaving failed Jobs in the namespace indefinitely. Add the same deleteEvalHubFailureSyncedJob call that the failureAlreadyReported path already performs. * docs(evalhub): clarify evalhub-events role binding target The role is currently bound to the operator SA but is intended for the EvalHub server SA once RHAI-277 lands. Update the comment to reflect the current state and planned rebinding. * fix(evalhub): prevent duplicate POST and Event on Job failure retry When promotePatch failed, the retry path re-executed the POST to EvalHub and re-emitted the Kubernetes Event because failurePendingReport only skipped the pending-annotation write. Move the POST inside the !failurePendingReport block so retries skip it, and move Eventf after promotePatch so it only fires once the promote succeeds.
Add two new ClusterRoles for the EvalHub ServiceAccount:
Both new ClusterRoleBindings are added to the OPA policy allowlist. No functional code changes; this is the RBAC prerequisite for RHAI-277.
Summary by CodeRabbit