Skip to content

Commit 17e9fcf

Browse files
authored
fix(executor): prevent invalid TaskActions from staying nonterminal (#7967)
fix(executor): retry failed validation status writes Signed-off-by: 1fanwang <1fannnw@gmail.com>
1 parent b3fae56 commit 17e9fcf

2 files changed

Lines changed: 43 additions & 2 deletions

File tree

executor/pkg/controller/taskaction_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,9 @@ func (r *TaskActionReconciler) reconcileTask(
374374
setCondition(taskAction, flyteorgv1.ConditionTypeFailed, metav1.ConditionTrue, reason, err.Error())
375375
setCondition(taskAction, flyteorgv1.ConditionTypeProgressing, metav1.ConditionFalse, reason, err.Error())
376376
start := time.Now()
377-
updErr := r.Status().Update(ctx, taskAction) // error intentionally ignored: terminal either way
377+
updErr := r.Status().Update(ctx, taskAction)
378378
r.metrics.recordK8sOp(ctx, opStatusUpdate, start, updErr)
379-
return ctrl.Result{}, nil // terminal — do not requeue
379+
return ctrl.Result{}, updErr
380380
}
381381

382382
// Ensure finalizer is present (once validation passes)

executor/pkg/controller/taskaction_validation_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,16 @@ package controller
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"strings"
78
"testing"
89

10+
"k8s.io/client-go/tools/events"
11+
"sigs.k8s.io/controller-runtime/pkg/client"
12+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
13+
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
14+
915
flyteorgv1 "github.com/flyteorg/flyte/v2/executor/api/v1"
1016
pluginsCore "github.com/flyteorg/flyte/v2/flyteplugins/go/tasks/pluginmachinery/core"
1117
)
@@ -109,3 +115,38 @@ func TestValidateTaskAction_PluginNotFound(t *testing.T) {
109115
t.Errorf("expected reason %q, got %q", flyteorgv1.ConditionReasonPluginNotFound, reason)
110116
}
111117
}
118+
119+
func TestReconcileTask_ValidationStatusUpdate(t *testing.T) {
120+
statusErr := errors.New("status update failed")
121+
for _, tc := range []struct {
122+
name string
123+
updateErr error
124+
}{
125+
{name: "success"},
126+
{name: "failure", updateErr: statusErr},
127+
} {
128+
t.Run(tc.name, func(t *testing.T) {
129+
k8sClient := fake.NewClientBuilder().
130+
WithInterceptorFuncs(interceptor.Funcs{
131+
SubResourceUpdate: func(context.Context, client.Client, string, client.Object, ...client.SubResourceUpdateOption) error {
132+
return tc.updateErr
133+
},
134+
}).
135+
Build()
136+
reconciler := &TaskActionReconciler{
137+
Client: k8sClient,
138+
Recorder: events.NewFakeRecorder(1),
139+
}
140+
taskAction := validTaskAction()
141+
taskAction.Spec.RunName = ""
142+
143+
_, err := reconciler.reconcileTask(context.Background(), taskAction, taskAction.DeepCopy())
144+
if !errors.Is(err, tc.updateErr) {
145+
t.Fatalf("expected %v, got %v", tc.updateErr, err)
146+
}
147+
if !isTerminal(taskAction) {
148+
t.Fatal("expected validation failure to remain terminal")
149+
}
150+
})
151+
}
152+
}

0 commit comments

Comments
 (0)