Repository navigation
New Linter strategy #2001
Description
The linter jobs have been stuck on FreeBSD 10 since we first introduced them. We need to get rid of FreeBSD 10, it's way out of support and we don't have anyone who is interested in maintaining them anyway. There was an aborted attempt to move them to Ubuntu machines but there were specifics about the various make lint commands that caused problems with that (we got ourselves into a BSD corner). It seems that we're past that and the linter(s) are all nicely BSD+GNU. So it's time to move them or have a new strategy.
There are two possible paths I want to put forward:
- Run the lint jobs on Ubuntu machines (the jenkins-workspace machines might be a good choice). This would mean (1) making sure there is a
nodeon those machines, and (2) reconfiguring the job to use the new machines. - Run the liters in Docker using some of the infra that comes in Proposal for major changes to (and expansion of) containerised build system #1982 - namely the ability to execute a particular container that gets pulled from Docker Hub. I've set up a linter Dockerfile here: https://github.com/rvagg/node-ci-containers/tree/master/node-linter (in Docker Hub as
rvagg/node-ci-containers:node-linter, again, that 'rvagg' will change). You just run it with your directory mounted:docker run -v $(pwd):/home/iojs/workspace rvagg/node-ci-containers:node-linter. This gets our linting logic out of Jenkins and into a form that's accessible and usable by collaborators and gives us an easier upgrade path in the future.
When this is done, I want to purge FreeBSD 10 from our infra. There's some urgency to this too btw since we need to move some Joyent resources around in the next few weeks and there's a FreeBSD 10 / linter machine involved in that.
Activity
jbergstroem commentedon Oct 25, 2019
FWIW: there's nothing about the linters that is tied to FreeBSD; it was just a personal preference when I originally set it up.
I like this approach. It makes it simpler to drop in any shared infra from other CI's or github actions.
sam-github commentedon Oct 25, 2019
caveat: I may be missing something about the history and requirements, for example, I'm honestly not sure if
git node landrequires a green Travis run. I don't think I've ever seen Travis red and Jenkins green, so haven't run into that :-).Why are we setting this up in jenkins? Can linting be left to be done only on Travis?
Travis only is simpler (delete the jobs, Travis does
make lint-py doc-only lintalready), and for the lint jobs in particular, they only need one platform and Travis already gives immediate feedback on a PR without collaborator intervention.richardlau commentedon Oct 25, 2019
node-core-utilscurrently doesn't check Travis results. We don't require Travis to pass to land a PR but there is a proposal (nodejs/node#29770) to allow just Travis for doc-only changes.richardlau commentedon Oct 25, 2019
Would https://github.com/rvagg/node-ci-containers/tree/master/node-linter be simpler if it used a Node.js docker container?
sam-github commentedon Oct 25, 2019
Wouldn't node-core-utils be simpler if it only tested (E"DIT: "checked") GH build results? I'm sure there is a reason its reaching out to Jenkins, I'm just not sure what it is.
richardlau commentedon Oct 25, 2019
Yes, but it currently does not even look at GH checks. nodejs/node-core-utils#369 would have to land first.
rvagg commentedon Oct 26, 2019
@richardlau
Yes it would, I think, I don't know why I didn't think of that.
@sam-github wrt Travis, some history: Travis support came much later than Jenkins because we had a policy of no-app-access for the org and that excluded Travis. That policy changed when GitHub gave more fine-grained control over access for orgs (or something like that), so we inherited the .travis.yml from ayo.js at some point between late 2017 and late 2018. I never followed that and it was pretty much entirely out of the perview of Build and has mostly remained as such. That fact alone has driven the disconnect and lands us in this issue! I hadn't even thought of Travis as an option here.
Travis is nice for the fact that we don't have to worry at all about the security concerns. I really don't like having the two overlapping services side-by-side and not integrated at all (a beefed-up GitHub Actions with full support for custom agents on arbitrary servers we manage so we can retire Jenkins is the dream).
SO, with all that, I guess that option (3) is to remove the linter job from Jenkins entirely and rely on Travis. That would certainly aid in the reduction of the complexity of Build's responsibility. It even makes sense that a quick lint job lives in Travis and gives immediate feedback to pull requests than having to go through our CI. A variation of this option is to have it in Travis and put it in .ci.yml (#1982) for the sake of completeness in Jenkins and not have a dedicated job for linting. Quick running jobs in .ci.yml are an ideal use-case and I'm assuming collaborators will want to add more things in there like this.
rvagg commentedon Oct 26, 2019
OK, I remember now that our hassle was in the Jenkins setup being BSD-specific. We tried to introduce Ubuntu 16.04 machines to the mix to migrate but discovered how hard it was to have both BSD and GNU tooling in the same scripting. I'm pretty sure Refael ended up sorting all of that out, the scripting in there now looks for every tool and adapts as needed. We just never committed to the migration.
Also folks remember that there are different linting tasks for different release lines, so it's not quite as simple as what's in Travis.
lint-py-buildis only in newer release lines for instance. I think this release-line specificity argues for Travis and/or .ci.yml so those particulars can be baked in to the release line and not in Jenkins or some script we put in this repo.rvagg commentedon Oct 26, 2019
OK, so it turns out that from 8.x onward we have a solid enough Makefile and
lint-cithat we don't need much of the additional fluff currently in the lint script. I've boiled it down to just this which is really justmake lint-ciand some output to stdout if it fails.Here's it failing on 8.x with a JS lint error: https://ci.nodejs.org/job/rv-test-commit-linux-docker/78/label=docker-host-x64,linux_x64_container_suite=lint/console
And passing: https://ci.nodejs.org/job/rv-test-commit-linux-docker/79/label=docker-host-x64,linux_x64_container_suite=lint/console
The script simplification makes me a bit more comfortable having an interim measure of moving the linting onto the jenkins-workspace machines, they'll need
nodeinstalled, however. Then we can get rid of FreeBSD 10. We can then decide whether we want to migrate lint to .ci.yml or just Travis.ansible: prepare jenkins-workspace machines for linting duty
rvagg commentedon Oct 27, 2019
#2008
prepared the 3 jenkins-workspace machines, have switched label in the Jenkins job, simplified the script a bit, and they've taken over linting now.
In the process I also realised my previous comment about simplification isn't quite right,
lint-py-buildneeds to be run separately and both it andlint-pyneed to be run withpython3on Node >= 12 according to the script in the Jenkins job. So I'm putting that into my .ci.yml proposal too.ansible: prepare jenkins-workspace machines for linting duty
ansible: prepare jenkins-workspace machines for linting duty
github-actions commentedon Aug 23, 2020
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.
mmarchini commentedon Aug 23, 2020
node-core-utils will check Actions after the next release, so in theory we can remove linter jobs from Jenkins as those are already covered by Actions
github-actions commentedon Jun 20, 2021
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.