Add a resume-ci label to issues?

[benjamingr]

Hey, not sure if this is the right place to ask (is it?)

There is a request-ci label which saves me the hassle of going to jenkins, entering all the details and starting a build for pull requests.

It would be useful to me if there was a similar label for resume-ci (when most builds pass but one is flakey or failed due to an unrelated infra issue).

Bonus points if it queues the "resume" even if the build didn't finish yet - but either way would save me time.

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[benjamingr]

This would still be useful

[richardlau]

I'm going to move this over to the core repo as CI triggering via labels is now done via Actions (https://github.com/nodejs/node/blob/master/.github/workflows/auto-start-ci.yml).

I'm not sure if it's possible to resume a build in Jenkins before it has completed (i.e. any queuing would have to be self-managed).

[Trott]

I can see how this would seem to be useful on the surface, and maybe it really would be, but I wonder if it would encourage resuming CI runs without looking at what actually failed. I could be wrong, but I would be cautious about the way the more we make it so that we don't check actual CI runs and just keep re-running them until they pass, the less useful we make CI.

[Trott]

I can see how this would seem to be useful on the surface, and maybe it really would be, but I wonder if it would encourage resuming CI runs without looking at what actually failed. I could be wrong, but I would be cautious about the way the more we make it so that we don't check actual CI runs and just keep re-running them until they pass, the less useful we make CI.

Instead of effort put into making such a label work, I would prefer to see a large effort put into making CI more reliable so that resuming runs is the exception rather than the rule.

[RaisinTen]

I wonder if it would encourage resuming CI runs without looking at what actually failed

The functionality already exists in Jenkins, so if someone wants to resume CI without looking at the failures, they would do it regardless of the label. I don't see how just adding the label for convenience would encourage more of that.

Instead of effort put into making such a label work, I would prefer to see a large effort put into making CI more reliable so that resuming runs is the exception rather than the rule.

I think people are already putting in the effort of making CI more reliable - there are currently 42 open issues and 427 resolved ones that have the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label but flaky tests seem to pop up every now and then. Writing more tests increases the chances of introducing flaky ones.

[richardlau]

If anyone wants to attempt this:

• https://github.com/nodejs/node/blob/HEAD/.github/workflows/auto-start-ci.yml is the workflow that looks for the request-ci label
• That workflow passes a list of PR numbers that have that label to https://github.com/nodejs/node/blob/HEAD/tools/actions/start-ci.sh
• start-ci.sh calls node-core-utils's ncu-ci run to start the CI

  node/tools/actions/start-ci.sh

  Line 13 in 2d6aea7

  ncu-ci run "$pr" >output 2>&1 || ci_started=no

For resuming you will need to add functionality to node-core-utils. You would need to investigate if there is a Jenkins REST API end point for resuming a build.

[Trott]

The functionality already exists in Jenkins, so if someone wants to resume CI without looking at the failures, they would do it regardless of the label. I don't see how just adding the label for convenience would encourage more of that.

IIUC, the label means that Triagers can do it too, so it expands the pool of people that have access to the resume feature. This is good, but also has the potential for the unintended effect I noted.

I think people are already putting in the effort of making CI more reliable

Some people are, yes. But there is not a large coordinated effort like there was when the Testing WG existed, for example. And I think we need something like that. (You'll notice I'm not volunteering to do it, so I'm part of the problem here, for sure.)

[RaisinTen]

IIUC, the label means that Triagers can do it too, so it expands the pool of people that have access to the resume feature. This is good, but also has the potential for the unintended effect I noted.

That shouldn't be necessary though. We can make use of the GH APIs to check if the person who added the label is a collaborator and then run the actions.

Some people are, yes. But there is not a large coordinated effort like there was when the Testing WG existed, for example. And I think we need something like that. (You'll notice I'm not volunteering to do it, so I'm part of the problem here, for sure.)

So we don't have a clear path to make CI more resilient? Just wanted to make sure if you were -1 about a PR landing that adds this feature at least for collaborators only.

[Trott]

Just wanted to make sure if you were -1 about a PR landing that adds this feature at least for collaborators only.

I'm not -1 on that or extending it to triagers.

[benjamingr]

Hey, I'd like the TSC to discuss two things here (personal opinions below):

• Should triagers be able to land things and run CI from the GitHub labels?
• Should Node.js add a resume-ci label?

Regarding CI and landing things: Personally I think running CI posts some but not significant risks - and it means the TSC needs to decide how strict it wants to be about onboarding triagers . Landing things with the label is fine given NCU enforces reviews, green CI and a correct commit format.

Regarding resume-ci: Given the CI is flaky and has been for 5+ years at different points I do see a lot of value in such a label or streamlining how CI runs with flaky tests better (that is: not just mark them flaky but logic like: "If this test fails everywhere and in only one architecture and is not in a file changed in this PR - automatically don't fail the CI on it")

[Ayase-252]

I think the TSC had decided that triagers are eligible to use commit queue to land PR.

Refs: nodejs/TSC#1039 (comment)

[benjamingr]

Great I didn't know that! Then just ci and the resume-ci label

[mhdawson]

is not in a file changed in this PR

@Trott I'm wondering how we would do that in general. Mapping tests to which files they might be affected by is not obvious to me.

The resume ci label seems useful to me as well, I share the concern over making sure we look at failures before resuming, but that might be handled by documenting or surfacing existing documentation that sets the expectations. I'd personally be fine with an expectation that you look at failures make sure there are open issues for them before resuming.

[mhdawson]

Discussed in previous TSC meeting, see https://github.com/nodejs/TSC/blob/main/meetings/2022-03-10.md#nodejsnode and in #42125

Removing from TSC agenda as we think questions have been answered.