.github: Set commit status to error when workflow are cancelled

[pchaigno]

GitHub jobs are usually set to status 'error' when cancelled. We should do the same for ci-xxx jobs when they are cancelled. Having the state appear as an error clarifies that the author, janitor, and reviewers should take notice of that workflow's result.

[nbusseneau]

it can also happen for other automatic reasons (not yet determined)

Did you see this happen? The only case I've seen workflows get cancelled is when they hit the timeout, which is expected.

I personally still think both user cancellation and timeout cancellation resulting in pending is better than marking the job as failed, because when a job is cancelled it should simply be retriggered. Keeping it marked as pending makes it clearer IMO.

In any case, if we do want to move forward with marking jobs as failure, then I would recommend we instead just refactor both in the existing failure state by replacing:

- name: Set commit status to failure
  if: ${{ failure() }}

with

- name: Set commit status to failure
  if: ${{ !success() }}

(or failure() || cancelled())

[pchaigno]

Did you see this happen? The only case I've seen workflows get cancelled is when they hit the timeout, which is expected.

I've seen this is https://github.com/cilium/cilium/actions/runs/836840304, and it is indeed because of the 30 minutes timeout. I was not aware that was a potential reason for cancellation.

I personally still think both user cancellation and timeout cancellation resulting in pending is better than marking the job as failed, because when a job is cancelled it should simply be retriggered. Keeping it marked as pending makes it clearer IMO.

Ideally, we'd have a cancelled status, but we don't :'-(. Marking timeouts as pending is very weird to me: it's a failure case and whoever is responsible for the PR should check it to see what happened. They should not just retry blindly. It's also a behavior that differs from what Jenkins does (and maybe from what GitHub itself does in pull_request events). In particular, for non-required GitHub checks, if I'm a janitor and I see them as pending, I'm likely to think the PR author just didn't trigger them because it was unnecessary.

We can raise this on Monday to get the opinion of more folks.

In any case, if we do want to move forward with marking jobs as failure, then I would recommend we instead just refactor both in the existing failure state by replacing:

👍 I'll make the change.