kubeflow / trainer / 36777559515

72%

Build Type

push

github

Committed by web-flow

Commit Message

fix(controller): stop TrainJob reconcile panicking on an unsupported runtimeRef (#4088)

* fix(controller): stop TrainJob reconcile panicking on an unsupported runtimeRef

Reconcile treats an unresolvable spec.runtimeRef as a reportable state and sets
a Failed condition with TrainJobRuntimeNotSupportedReason, then passes the
runtime it did not find to setTrainJobStatus, which calls TrainJobStatus on a
nil interface.

r.runtimes is a map of interface values, so a lookup miss leaves the runtime
nil. controller-runtime recovers the panic and requeues, so the manager stays
up, but the unwind happens before the status patch: the condition never reaches
the API server and every retry panics at the same point, leaving the TrainJob
with nothing that explains why it never starts.

Deriving the status only when a runtime was found leaves the rest of the path
untouched, so the condition now survives to the patch.

Admission normally rejects an unsupported runtimeRef, which is why this has
gone unnoticed, but webhook.failurePolicy is a documented Helm value and a
TrainJob can be admitted under Ignore while the webhook is unavailable.

Signed-off-by: Abhishek <abhikokadwar2@gmail.com>

* fix(controller): derive TrainJob status inside the supported-runtime branch

Address review feedback: setTrainJobStatus now runs in the branch where the
runtime was found, straight after reconcileObjects, instead of behind a
separate check on ok. The unsupported runtime path can no longer reach it, so
the guard and its comment go away.

It stays outside the IsTrainJobFinished check, so finished TrainJobs still have
their status refreshed as before. Running it before setSuspendedCondition does
not change the result: the JobSet status plugin starts from a copy of the
existing status and never touches the Suspended condition, and
setSuspendedCondition only reads spec.suspend and that condition.

Signed-off-by: Abhishek <abhikokadwar2@gmail.com>

---------

Signed-off-by: Abhishek <ab... (continued)

Coverage Stats

1 of 15 new or added lines in 1 file covered. (6.67%)

4 existing lines in 2 files now uncovered.

2812 of 3986 relevant lines covered (70.55%)

0.82 hits per line