policy: no use of PR code in live jobs

[refack]

Please don't use code from branches or forks in live jobs.

IMO also the use in testing jobs is questionable, since it's hard to audit.

[mhdawson]

Do you have some specific examples (I'm ok if they identify me as the culprit :))? I have a few guesses as I think I've done what you are mentioned in a few cases. In the case of the compiler selection script I wanted to:

• be able to test before the change landed
• let it run a short time to build confidence before affecting the release jobs (as the same script is used for both test/release). In some cases this was also testing changes to test machines that I wanted to be comfortable with before rolling those changes out to the release machine.

[maclover7]

I think this is related to my personal branch to move all node-test-commit subjobs to one bash script, and I think that testing this with live jobs is justified, to make sure there are no regressions. Also, since it's tracked using Git, any "bad' commands that may be run are stored in version control.

[joaocgreis]

Over long periods of time, this would be a good policy. Anything that's stable should be using the main repo.

+1 to the points raised by @mhdawson, plus:

• what about when things are broken, need to be fixed as fast as possible and no one is available to review? I'd hate to be in a situation where I couldn't fix issues because of this policy.

[mmarchini]

I think having a clear policy documented somewhere regarding changing job settings is a good idea. IMO we should avoid using untested and unreviewed scripts/config on our main jobs unless something broke and the only way to fix it is by editing job configs.

[joaocgreis]

I think of the jobs in Jenkins as organized in 4 categories. I'll list them here, along with my opinion. If others agree with me this might serve as a base for this policy.

1. node-test-pull-request and all of its dependencies: I'd recommend to be extremely careful with anything here, make sure no changes come as a surprise to other WG members.

   • For new features, make sure there's consensus in the WG by opening an issue.
   • For urgent fixes, we should log them somewhere (a PR sounds good when it's something PR-able).

   In any case, always run a test job for each node major version (When adding new targets, can we ensure they work on all release lines #1182) and monitor the changes. If any job started by node collaborators is broken, restart the failed sub-job then check the PR number and mention it there (for example inspector: break in eval script node#14581 (comment)).

2. test jobs for other projects (libuv, node-gyp, chakracore, ...): Generally, all the same as above. This is a different category because each of these affect much less people, and also because in some cases it's easy and reasonable to get in touch with everyone that uses them. Hence, some exceptions can be made, for example, I'd like to be able to change the chakracore jobs without having to go through a review cycle.

3. maintenance jobs (git-clean-*, ...): We should keep these agile, they are only used by us. For now, I'd prefer if we can just change them as needed (we have the job history if we need to go back). These are all great candidates for pipelines, when that happens I'd say we can open PRs and point the jobs to the PR branches and then to the main repo when they land.

4. personal test jobs: (please prefix your username instead of suffixing it, to keep the jobs organized) There should be no policy over these, as long as they don't make jobs of the categories above fail. This should be the recommended way of testing changes for 1 and 2 (3 can be change and test, since they're only started by us).

We require WG members to show some familiarity with Jenkins/Ansible/... before joining. This should translate into some trust afterwards. I fear that demanding a full review cycle for every small change will make collaborating too much of a burden. I believe it is reasonable in some cases, and too much in others, as listed above.

[richardlau]

I just noticed this while investigating smartos failures in nodereport-continuous-integration but it looks like node-test-commit-smartos is using:

curl https://raw.githubusercontent.com/maclover7/build/8fbce71/jenkins/scripts/node-test-commit.sh | bash -ex -s

e.g. https://ci.nodejs.org/job/node-test-commit-smartos/nodes=smartos16-64/19818/consoleFull

02:43:22 [smartos16-64] $ /bin/sh -xe /var/tmp/jenkins5679813579958051228.sh
02:43:22 + bash -ex -s
02:43:22 + curl https://raw.githubusercontent.com/maclover7/build/8fbce71/jenkins/scripts/node-test-commit.sh

There is currently no equivalent node-test-commit.sh in jenkins/scripts.