Skip to content

Conversation

@mmarchini
Copy link
Contributor

@mmarchinimmarchini commented Jun 27, 2020

Add an Action that will find every PR with the request-ci label and
will start a Jenkins CI for each of these Pull Requests. The scheduler
event is used to circumvent GitHub Actions limitations on Pull Requests
from forks (where secrets are not accessible and the GITHUB_TOKEN is
read-only).

If the Action fails to start a CI, it will add a request-ci-failed
label and will leave a comment with the error message from NCU.

Requirements to land

  • Create a Jenkins token for @nodejs-github-bot
  • Add JENKINS_USER and JENKINS_TOKEN secrets to this repository (or to the entire org, if we want this to work on forks such as quic and node-auto-test as well)
  • Land ncu-ci: command to start CI for PRs node-core-utils#445 and wait for the next NCU release
  • Test more extensively on node-auto-test before landing
  • Create request-ci and request-ci-failed labels in this repository
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@mmarchinimmarchini added the blocked PRs that are blocked by other issues or PRs. label Jun 27, 2020
@nodejs-github-botnodejs-github-bot added meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Jun 27, 2020
Comment on lines +55 to +62
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should provide a way to set ncu configs via environment variables, so we don't have to worry about writing to disk here.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're worried about the number of concurrent Jenkins jobs running, we could reduce this to 5 and increase the scheduler to 15-30 minutes.

@mscdex
Copy link
Contributor

If we add something like this, shouldn't we just get rid of the "CERTIFY_SAFE" checkbox in CI then, since there is no way to make such a confirmation explicit when applying the new label to a PR?

@mmarchini
Copy link
ContributorAuthor

That's a good question. We could consider that adding the label means the collaborator is certifying that this PR is safe (same could be said about starting a CI though)

@mmarchini
Copy link
ContributorAuthor

We could also have two labels, but that seems unnecessary

@Trott
Copy link
Member

@nodejs/build

@mmarchini
Copy link
ContributorAuthor

mmarchini commented Jun 28, 2020

Not sure who I should ping to get consensus on adding the secrets to the repo (or if I need to do it) and to create the labels. @nodejs/tsc maybe?

Also don't know who has access to the github-bot account to create a Jenkins token for it.

Copy link
Contributor

@cclausscclauss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM Impressive work!

@mmarchinimmarchiniforce-pushed the auto-start-ci-via-label branch from f1c9444 to a1121faCompareJuly 30, 2020 05:41
@mmarchinimmarchini self-assigned this Jul 30, 2020
@mmarchinimmarchini removed the blocked PRs that are blocked by other issues or PRs. label Jul 30, 2020
@mmarchini
Copy link
ContributorAuthor

Ok, figured out the github-bot bits, will create the tokens on the repository before landing it.

Removed the blocked label, but it would be good to have some more reviews (especially on the general approach), I intend to land on friday or saturday to give folks a bit more time to review. I also ask to let me land it since there are other tasks that need to be accomplished while landing (create labels, add secrets to the repo)

(I'd like to land this before the commit queue since both use a similar approach, but this action won't make changes to our main branch)

Copy link
Member

@phillipjphillipj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work 👍

@mmarchini
Copy link
ContributorAuthor

mmarchini commented Jul 31, 2020

@nodejs/build @nodejs/tsc any other reviews/comments/suggestions/objections before I land this tomorrow? (I thought about pinging collaborators but that might be too much :) )

Add an Action that will find every PR with the `request-ci` label and will start a Jenkins CI for each of these Pull Requests. The scheduler event is used to circumvent GitHub Actions limitations on Pull Requests from forks (where secrets are not accessible and the GITHUB_TOKEN is read-only). If the Action fails to start a CI, it will add a `request-ci-failed` label and will leave a comment with the error message from NCU. Fixes: nodejs/github-bot#234
@mmarchinimmarchiniforce-pushed the auto-start-ci-via-label branch from 9a4ae9c to 84de5d9CompareJuly 31, 2020 22:24
@mmarchini
Copy link
ContributorAuthor

Labels created:
image

mmarchini added a commit that referenced this pull request Jul 31, 2020
Add an Action that will find every PR with the `request-ci` label and will start a Jenkins CI for each of these Pull Requests. The scheduler event is used to circumvent GitHub Actions limitations on Pull Requests from forks (where secrets are not accessible and the GITHUB_TOKEN is read-only). If the Action fails to start a CI, it will add a `request-ci-failed` label and will leave a comment with the error message from NCU. Fixes: nodejs/github-bot#234 PR-URL: #34089 Reviewed-By: Christian Clauss <[email protected]>
@mmarchini
Copy link
ContributorAuthor

Landed in 6cab3b0

codebytere pushed a commit that referenced this pull request Aug 5, 2020
Add an Action that will find every PR with the `request-ci` label and will start a Jenkins CI for each of these Pull Requests. The scheduler event is used to circumvent GitHub Actions limitations on Pull Requests from forks (where secrets are not accessible and the GITHUB_TOKEN is read-only). If the Action fails to start a CI, it will add a `request-ci-failed` label and will leave a comment with the error message from NCU. Fixes: nodejs/github-bot#234 PR-URL: #34089 Reviewed-By: Christian Clauss <[email protected]>
@codebyterecodebytere mentioned this pull request Aug 10, 2020
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Add an Action that will find every PR with the `request-ci` label and will start a Jenkins CI for each of these Pull Requests. The scheduler event is used to circumvent GitHub Actions limitations on Pull Requests from forks (where secrets are not accessible and the GITHUB_TOKEN is read-only). If the Action fails to start a CI, it will add a `request-ci-failed` label and will leave a comment with the error message from NCU. Fixes: nodejs/github-bot#234 PR-URL: #34089 Reviewed-By: Christian Clauss <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Add an Action that will find every PR with the `request-ci` label and will start a Jenkins CI for each of these Pull Requests. The scheduler event is used to circumvent GitHub Actions limitations on Pull Requests from forks (where secrets are not accessible and the GITHUB_TOKEN is read-only). If the Action fails to start a CI, it will add a `request-ci-failed` label and will leave a comment with the error message from NCU. Fixes: nodejs/github-bot#234 PR-URL: #34089 Reviewed-By: Christian Clauss <[email protected]>
@codebyterecodebytere mentioned this pull request Sep 28, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

metaIssues and PRs related to the general management of the project.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@mmarchini@mscdex@Trott@phillipj@cclauss@himself65@nodejs-github-bot