Repository navigation
Conversation
camielvs
left a comment
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
4 findings, inline below: two Medium on how broad the rejection scope is, two Low on the metric and the fast-fail message.
Cross-repo note, not for this PR: the API puts orchestration_error_message into system_error_exception_full, and tangle-ui shows that field as a "System error" banner (SystemErrorSection.tsx) and a "System Error Logs" viewer (TaskOverview/logs.tsx). So a rejected task shows FAILED, but its error still reads as a system error. Missing-outputs failures already look like this. The fix belongs in tangle-ui.
camielvs
left a comment
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
Second pass, on 27b1146. The four comments from the first review are addressed: the kind is in the scope, there's a spec digest, the fast-fail message names the task, and the metric has an attempted attribute.
13 findings inline:
- Medium: The scope is still wider than some policies (what the digest covers, spot VMs, wrapping launchers). The one-hour cache treats state-dependent denials as permanent. Platform-wide denials no longer reach system-error alerting. Pod-level denials under a Job still hang. The state and stream endpoints still return 409 for rejected tasks.
- Low: transaction ordering vs. the cache, a duplicate traceback for invalid retries, pickling, the denied spec not being logged, ValidatingAdmissionPolicy, and the test helper's shared session.
The "System Error Logs" labelling from the first review is unchanged. The new test at tests/test_execution_nodes_api_service.py:626 asserts system_error_exception_full == message, so that test will need updating when tangle-ui stops reading that field.
…ERROR - When an admission webhook denies the request to create a Job or Pod, the launcher raises `LaunchRejectedError` with the webhook's message. A denial is a 400 or 403 whose Status message contains `admission webhook "…" denied the request`. The submitted workload is logged at WARNING with its command lines left out. - The orchestrator marks the execution FAILED, puts the message in `orchestration_error_message`, and skips its downstream executions. It records no system error, so a denial isn't counted in `execution.system_errors` or sent to Bugsnag. It is counted in a new `execution.launch_rejections` counter, by reason. - `get_container_execution_log` returns the error of a FAILED execution that has no container execution, as it already did for SYSTEM_ERROR, so the UI shows the denial. - RBAC 403s, unreachable webhooks (500) and every other create failure stay SYSTEM_ERROR. A denial is something the user can fix, such as a cluster policy that refuses the workload. As SYSTEM_ERRORs, a fan-out of 80 denied tasks produced 80 system errors and set off system-error alerts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9e93587 to
6f66778
Compare
What
When an admission webhook denies a launch, the execution now ends FAILED with the webhook's message, not SYSTEM_ERROR. It isn't counted in
execution.system_errorsor sent to Bugsnag. Its downstream executions are SKIPPED, as for any other failure.create_namespaced_job/create_namespaced_podfails withadmission webhook "…" denied the request…… is forbidden: User … cannot create …)failed calling webhook(webhook unreachable)launchers/interfaces.py: newLaunchRejectedError(LauncherError), withmessage(shown to the user) andreason(a short code for metrics).launchers/kubernetes_launchers.py: on a denial, the Job and Pod launchers raiseLaunchRejectedError(reason="admission_denied")with the webhook's message. They log the submitted workload at WARNING, with command lines left out, because the user's message doesn't include it.orchestrator_sql.py: aLaunchRejectedErrormakes the execution FAILED, with the message inorchestration_error_message. It logs a WARNING and adds to a new counter,execution.launch_rejections, with the attributeexecution.launch_rejection.reason.api_server_sql.py:get_container_execution_logreturns the error for a FAILED execution with no container execution, as it already did for SYSTEM_ERROR. Without this, the UI shows nothing for a denied task.Why
A cluster policy started denying the launcher's Jobs. A pipeline that fanned out to 80 tasks produced 80 SYSTEM_ERRORs, which set off system-error alerts. Nothing in the system was broken, and the webhook's message says what the user needs to change.
Dev Notes
Statuswhose message readsadmission webhook "<name>" denied the request. The status code is the webhook's choice: 400 or 403 in practice. The classification needs both the code and that text, so an RBAC 403 or an unreachable webhook (500) stays a system error.execution.launch_rejections.Tests
tests/test_kubernetes_launcher_error_classification.py:LaunchRejectedErrorwith the webhook's message.LauncherErrors.tests/test_orchestrator_sql.py:LauncherErroris still SYSTEM_ERROR.tests/test_execution_nodes_api_service.py:ContainerExecutionNotReadyError.black --check .passes with 26.5.1 (the version inuv.lock) and 26.10.1.ApiExceptions with Status bodies shaped like the API server's.🤖 Generated with Claude Code