Repository navigation
Add a resume-ci label to issues? #40817
Description
Activity
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.
This would still be useful
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).
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.
Reacted by Tobias NießenI 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.
Reacted by Richard Lau, Michaël Zasso, Mesteery and Tobias NießenI 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 involving tests that fail intermittently in CI. label but flaky tests seem to pop up every now and then. Writing more tests increases the chances of introducing flaky ones.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-cilabel - 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.shcallsnode-core-utils'sncu-ci runto start the CInode/tools/actions/start-ci.sh
Line 13 in 2d6aea7
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.- https://github.com/nodejs/node/blob/HEAD/.github/workflows/auto-start-ci.yml is the workflow that looks for the
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.)
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.
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.
Reacted by Darshan SenHey, 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-cilabel?
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")Reacted by Moshe AtlowI think the TSC had decided that triagers are eligible to use commit queue to land PR.
Reacted by Benjamin Gruenbaum and Moshe AtlowGreat I didn't know that! Then just ci and the resume-ci label
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.
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.
This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.
Hey, not sure if this is the right place to ask (is it?)
There is a
request-cilabel 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.