Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions actions/k8s/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ func (c *ActionsClient) setupInformer(ctx context.Context) error {
obj = tombstone.Obj
}
taskAction, ok := obj.(*executorv1.TaskAction)
if !ok {
if !ok || c.shouldSkipTaskAction(taskAction) {
return
}
c.dispatchEvent(taskAction, watch.Deleted)
Expand Down Expand Up @@ -509,7 +509,10 @@ func buildActionUpdate(ctx context.Context, taskAction *executorv1.TaskAction, e
}

phase := GetPhaseFromConditions(taskAction)
if eventType == watch.Deleted {
if eventType == watch.Deleted && !isTerminalPhase(phase) {
// Only force ABORTED if the action wasn't already in a terminal phase.
// Otherwise a missed-delete tombstone or post-terminal CR cleanup would
// overwrite a recorded Succeeded/Failed status with Aborted.
phase = common.ActionPhase_ACTION_PHASE_ABORTED
}

Expand Down
59 changes: 59 additions & 0 deletions actions/k8s/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,3 +741,62 @@ func TestBuildActionUpdate_NilErrorStateWhenAbsent(t *testing.T) {

assert.Nil(t, upd.ErrorState)
}

// A delete event (real or DeletedFinalStateUnknown tombstone delivered after
// informer resync) on a TaskAction that already reached a terminal phase must
// NOT overwrite the recorded phase with ABORTED.
func TestBuildActionUpdate_DeleteAfterTerminalPreservesPhase(t *testing.T) {
cases := []struct {
name string
condition string
want common.ActionPhase
}{
{"succeeded", string(executorv1.ConditionTypeSucceeded), common.ActionPhase_ACTION_PHASE_SUCCEEDED},
{"failed", string(executorv1.ConditionTypeFailed), common.ActionPhase_ACTION_PHASE_FAILED},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ta := &executorv1.TaskAction{
Spec: executorv1.TaskActionSpec{
Project: "p", Domain: "d", RunName: "r", ActionName: "a",
},
Status: executorv1.TaskActionStatus{
Conditions: []metav1.Condition{
{Type: tc.condition, Status: metav1.ConditionTrue},
},
},
}

upd := buildActionUpdate(context.Background(), ta, watch.Deleted)

require.NotNil(t, upd)
assert.Equal(t, tc.want, upd.Phase, "delete event must not overwrite a terminal phase with ABORTED")
assert.True(t, upd.IsDeleted)
})
}
}

// A delete event on a TaskAction that is still running (no terminal condition)
// should still mark the action as ABORTED.
func TestBuildActionUpdate_DeleteOnNonTerminalForcesAborted(t *testing.T) {
ta := &executorv1.TaskAction{
Spec: executorv1.TaskActionSpec{
Project: "p", Domain: "d", RunName: "r", ActionName: "a",
},
Status: executorv1.TaskActionStatus{
Conditions: []metav1.Condition{
{
Type: string(executorv1.ConditionTypeProgressing),
Status: metav1.ConditionTrue,
Reason: string(executorv1.ConditionReasonExecuting),
},
},
},
}

upd := buildActionUpdate(context.Background(), ta, watch.Deleted)

require.NotNil(t, upd)
assert.Equal(t, common.ActionPhase_ACTION_PHASE_ABORTED, upd.Phase)
assert.True(t, upd.IsDeleted)
}
Loading