Fix job status aggregation logic

[nienjiuntai]

Fix #34821 by checking if all the jobs are done before set status to fail

[wxiaoguang]

You can add (or fix) a test case here: run_job_status_test.go

[TheFox0x7]

specifically modify one here:

gitea/models/actions/run_job_status_test.go

Lines 67 to 69 in 182cdd3

	{[]Status{StatusFailure, StatusWaiting}, StatusFailure},
	{[]Status{StatusFailure, StatusRunning}, StatusFailure},
	{[]Status{StatusFailure, StatusBlocked}, StatusFailure},

I'm not sure that this is a right fix, more that github does it weirdly in this case. Unless entire workflow can recover from the failure (which I don't know if it's possible) I think we do this correctly now.
But I guess it kind of depends what signal you're more interested in - is it the pass/fail or are there any jobs still running/pending.

I don't think the fix is complete though as there's still the case of the waiting and blocked. Logically looking if we consider running as not a failure, then waiting and blocked shouldn't be as well as they are before the running state.
It wouldn't make much sense to have overall status be failure when we have failure+blocked but running when the block converts to failure.

[wxiaoguang]

Hmm, I also have the question: should we report the failure as soon as possible?

IMO the old behavior (reporting failure as soon as possible) is better to end users.

If a job fails, then the final status must be also "failed". Reporting failure to users early can tell them "there is something" in first time, no need to wait all jobs finishes.

[wxiaoguang]

You see, the comment is still there ...

[lunny]

Or maybe we can introduce a new status like running with part failure.

[TheFox0x7]

I think that would be the best solution. That way UI can indicate that run has already failed BUT there's something still pending.

[nienjiuntai]

You see, the comment is still there ...

Yes, I'm aware this comment, but it doesn't match the behavior tested in Github Action

Fail fast property will cause a weired behavior that user cannot cancel a workflow or get current running workflows correctly by filter after a job fail.

[wxiaoguang]

Fail fast property will cause a weired behavior that user cannot cancel a workflow or get current running workflows correctly by filter after a job fail.

It sounds a strong reason.

So I think we can write the details into comment, and use this solution, and backport. Leave more improvements to the future.

[lunny]

#34823 (comment)

This could be another PR

[wxiaoguang]

I don't think the fix is complete though as there's still the case of the waiting and blocked. Logically looking if we consider running as not a failure, then waiting and blocked shouldn't be as well as they are before the running state. It wouldn't make much sense to have overall status be failure when we have failure+blocked but running when the block converts to failure.

Merged now, let's see whether we should continue changing the priority, then never make the "failure" win other status.

Maybe something related:

1. Should we "fix the bug" that a "failure" task isn't able to be cancelled or rerun? Then still make the the status "fail fast" (win other status)
2. Should we really introduce a bunch of new statuses like failed-running, or failed-waiting, etc. Or introduce a combined status mechanism like (failure, running)?
   • Either solution seems too complex than it should be

Open for discussion.

[lunny]

We just need only to introduce running-failure, no other status is necessary.
Looks like two new statuses need to be introduced if that's the right direction.

[wxiaoguang]

We just need only to introduce running-failure, no other status is necessary.

Why? "waiting" is similar to "running" in this case and the "waiting" will be come "running" sooner or later.

[wxiaoguang]

more that github does it weirdly in this case.

Hmm, it's also strange to me.

According to my experience, GitHub still makes failure have higher priority. It is what I have seen for the latest 1.24 branch:

1. One job fails, others are running
2. The commit status shows a red cross (failure)
3. I can click into the commit and see its jobs, others are still running, I can cancel the running ones.

I suspect there are some problems in this PR or the whole design (ps: I don't really use Actions so I can't comment more or spend more time on it)

[lunny]

more that github does it weirdly in this case.

Hmm, it's also strange to me.

According to my experience, GitHub still makes failure have higher priority. It is what I have seen for the latest 1.24 branch:

1. One job fails, others are running
2. The commit status shows a red cross (failure)
3. I can click into the commit and see its jobs, others are still running, I can cancel the running ones.

I suspect there are some problems in this PR or the whole design (ps: I don't really use Actions so I can't comment more or spend more time on it)

I think other CI/CDs should have the similar problems?

[wxiaoguang]

My brief understanding of the problem is (just my guess, some of my previous comments might be not right):

GitHub has 2 kinds of "aggregation logic"

• The commit status for commit list: failure has higher priority
• The action status for action details page: running has higher priority

If I understand correctly, Gitea also has 2 kinds of "aggregation logic", the same to GitHub.

• The commit status for commit list: (need to check code's logic to confirm)
• The action status for action details page: this PR, matches GitHub now

So, overall this change should be fine.