Page MenuHomePhabricator

Set up Jenkins for chromium-render repository
Closed, ResolvedPublic3 Estimated Story Points

Description

When someone +2's a change, nothing happens. Let Jenkins take care of submitting the patch.

Repository: https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/services/chromium-render

We should run any tests that we run locally, at minimum npm test

Acceptance Criteria (?)

  • Submit a test that breaks npm test and show that Jenkins rejects it. Abandon the patch.

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Jdlrobson updated the task description. (Show Details)
Jdlrobson set the point value for this task to 3.
phuedx subscribed.

So what remains to be done here? html2pdf test is broken?

greg subscribed.

Adding our kanban, just so it's not lost. :)

Change 394058 abandoned by Phuedx:
Add npm job for the Chromium render service

Reason:
Superseded by I0565bdf2.

https://gerrit.wikimedia.org/r/394058

So what remains to be done here? html2pdf test is broken?

Per the latest build failure, yes.

Change 407719 had a related patch set uploaded (by Niedzielski; owner: Sniedzielski):
[integration/config@master] WIP: Fix: run NPM containers as "node" user

https://gerrit.wikimedia.org/r/407719

Change 407719 abandoned by Niedzielski:
WIP: Fix: run NPM containers as "node" user

Reason:
We don't seem to actually use docker-node :[

https://gerrit.wikimedia.org/r/407719

html2pdf test is broken

I'm afraid I've mucked this up. I'm not new to experimenting with JJB but I don't have much Docker experience. I think the only things I changed were to try running the container as (what turned out to be) a nonexistent user, node, and to run an interactive session with -it bash in which I performed read only commands like cd and ls. I've since wiped the workspace, reset the Jenkins config as usual with jenkins_jobs, and tried it on different Jenkins nodes but the build now succeeds every time so I must conclude that I've altered the container state some how. How can I "reset" the container back to its initial state? Obviously, fixing the problem is good but I want to do so in a well-defined and permanent way.

@Niedzielski: If you're at a loss, then the RelEng folk are super-responsive and helpful in #wikimedia-releng on Freenode.

@phuedx, fortunately, it seems to be failing as expected now.

  1. CI tests against master are passing because they're downloading Chromium through NPM.
  2. CI tests against this patch which assumes Chromium exists fail.

For #2, I believe the tests are failing because of either a version mismatch or a missing dependency. My reasoning is that:

  1. On my machine (Ubuntu v17.10), using Chromium v64.0.3282.119 via apt install works.
  2. The container has v63.0.3239.84 (although v64.0.3282.119-1 is available).
  3. Downloading the NPM version (v65.0.3312.0) directly into the container src/ mount (and unzipping and referencing the local executable-- e.g., see patchset) works. Docker command used (after temporarily disabling the --rm and clear JJB commands and a fresh build):
docker run --env <(/usr/bin/env) \
  --volume /srv/jenkins-workspace/workspace/chromium-render-npm-browser-node-6-docker/log:/log \
  --volume /srv/jenkins-workspace/workspace/chromium-render-npm-browser-node-6-docker/cache:/cache \
  --volume /srv/jenkins-workspace/workspace/chromium-render-npm-browser-node-6-docker/src:/src \
  -u nobody -ti --entrypoint=/bin/bash docker-registry.wikimedia.org/releng/npm-browser-test:0.1.0

My recommendation is either:

  1. Update the Docker NPM image (npm-browser-test) to use the latest version of Chromium from apt. I believe this requires releng privileges and impacts other jobs.
  2. Accept that downloading Chromium is wasteful but working. sshing into integration-slave-docker-1005, wget https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/526987/chrome-linux.zip consistently completed in less than a second. In actual CI builds, it's still less than 10 seconds. There are only 33 patchsets so far and I don't think we'll be making so many more. There are better things we can work on.
  3. Override PUPPETEER_DOWNLOAD_HOST instead of PUPPETEER_SKIP_CHROMIUM_DOWNLOAD and point to a WMF maintained repo of Chromium builds. This ticket will need discussion and possibly re-estimation if we go this route.

For #2, I believe the tests are failing because of either a version mismatch or a missing dependency. My reasoning is that:

  1. On my machine (Ubuntu v17.10), using Chromium v64.0.3282.119 via apt install works.
  2. The container has v63.0.3239.84 (although v64.0.3282.119-1 is available).

As you mention it seems that a newer version of chromium is available on stretch:

(/^ヮ^)/*:・゚✧ docker run --rm -it --user root --entrypoint /bin/bash docker-registry.wikimedia.org/releng/npm-browser-test:0.1.0
root@b873b9a69595:/# apt-get update
Get:1 http://apt.wikimedia.org/wikimedia stretch-wikimedia InRelease [21.5 kB]
Ign:2 http://mirrors.wikimedia.org/debian stretch InRelease
Get:3 http://mirrors.wikimedia.org/debian stretch-updates InRelease [91.0 kB]
Get:4 http://security.debian.org stretch/updates InRelease [63.0 kB]
Get:5 http://mirrors.wikimedia.org/debian stretch-backports InRelease [91.8 kB]
Get:6 http://apt.wikimedia.org/wikimedia stretch-wikimedia/main amd64 Packages [41.4 kB]
Get:7 http://mirrors.wikimedia.org/debian stretch Release [118 kB]
Get:8 http://mirrors.wikimedia.org/debian stretch Release.gpg [2,434 B]
Get:9 http://security.debian.org stretch/updates/main amd64 Packages [341 kB]
Get:10 http://mirrors.wikimedia.org/debian stretch-updates/main amd64 Packages [8,384 B]
Get:11 http://mirrors.wikimedia.org/debian stretch-backports/contrib amd64 Packages [6,140 B]
Get:12 http://mirrors.wikimedia.org/debian stretch-backports/main amd64 Packages [336 kB]
Get:13 http://mirrors.wikimedia.org/debian stretch/main amd64 Packages [9,531 kB]
Fetched 10.7 MB in 3s (2,830 kB/s)
Reading package lists... Done
root@b873b9a69595:/# apt-cache policy chromium
chromium:
  Installed: 63.0.3239.84-1~deb9u1
  Candidate: 64.0.3282.119-1~deb9u1
  Version table:
     64.0.3282.119-1~deb9u1 500
        500 http://security.debian.org stretch/updates/main amd64 Packages
 *** 63.0.3239.84-1~deb9u1 100
        100 /var/lib/dpkg/status
     62.0.3202.89-1~deb9u1 500
        500 http://mirrors.wikimedia.org/debian stretch/main amd64 Packages

Which is what we would get if we rebuilt the npm-browser-test container again today, seemingly: https://github.com/wikimedia/integration-config/blob/master/dockerfiles/npm-browser-test/Dockerfile.template#L4

If that version behaves as expected (am I reading that correctly?), then this may be solvable via a simple changelog bump https://github.com/wikimedia/integration-config/blob/master/dockerfiles/npm-browser-test/changelog#L1 to trigger a rebuild of the package.

@hashar since you built the original image, does that seem ok to you? Seems like we'll need to test the updated image with the 12 existing tests that utilize this image to ensure there aren't regressions which is time-consuming, but that's the only problem I can foresee...

It should be fine to just upgrade it - Chromium 64 is already out in the wild, so if tests are broken, it presumably means the real thing is broken too.

(Also in the past Chromium upgrades just happened automatically IIRC, there wasn't manual verification)

@thcipriani, @Legoktm, @hashar, the latest version of Chromium appears to test correctly.

Thanks @Niedzielski for the deep analyzes on T179552#3947612

My recommendation is either:

  1. Update the Docker NPM image (npm-browser-test) to use the latest version of Chromium from apt. I believe this requires releng privileges and impacts other jobs.

That is what we should do. With ideally one day having the CI container rebuild automatically whenever a new Chromium is made available in Debian Stretch. It is correct that only releng/roots can get the containers deployed on the CI infrastructure.

  1. Accept that downloading Chromium is wasteful but working. sshing into integration-slave-docker-1005, wget https://storage.googleapis.com/chromium-browser-snapshots/Linux_x64/526987/chrome-linux.zip consistently completed in less than a second. In actual CI builds, it's still less than 10 seconds. There are only 33 patchsets so far and I don't think we'll be making so many more. There are better things we can work on.

The chromium-render in production would run with the version from Debian Stretch. So supposedly it is nicer to test with that version instead of a randomly downloaded one. At some point the service will be run in a container which would have Chromium installed from the Debian package as well. So that is why we have instructed puppeter to skip downloading chromium entirely. From a past comment:

I don't mind downloading Chrome over and over. But we should aim at having the capacity to use a locally available Chromium.

Ugh. Per https://github.com/GoogleChrome/puppeteer/blob/master/docs/api.md#environment-variables, we need to set the PUPPETEER_SKIP_CHROMIUM_DOWNLOAD environment variable to stop this from happening.

  1. Override PUPPETEER_DOWNLOAD_HOST instead of PUPPETEER_SKIP_CHROMIUM_DOWNLOAD and point to a WMF maintained repo of Chromium builds. This ticket will need discussion and possibly re-estimation if we go this route.

Building our own Chromium is challenging. It is not that easy to build and maintain, it is way better to share the burden / delegate that task to Debian.org.

Which is what we would get if we rebuilt the `npm-browser-test` container again today, seemingly: https://github.com/wikimedia/integration-config/blob/master/dockerfiles/npm-browser-test/Dockerfile.template#L4

If that version (64) behaves as expected (am I reading that correctly?), then this may be solvable via a simple changelog bump https://github.com/wikimedia/integration-config/blob/master/dockerfiles/npm-browser-test/changelog#L1 to trigger a rebuild of the package.

@hashar since you built the original image, does that seem ok to you? Seems like we'll need to test the updated image with the 12 existing tests that utilize this image to ensure there aren't regressions which is time-consuming, but that's the only problem I can foresee...

Yes that is exactly it. Touching the changelog file would cause docker-pkg on contint1001 to rebuild the container from scratch and thus get whatever latest versions are available in the apt repository.

I love the idea of splitting the container refresh and job update in two different changes. That makes reverting the Jenkins jobs configuration easy in case something goes wrong :]

Change 408876 had a related patch set uploaded (by Hashar; owner: Legoktm):
[integration/config@master] Rebuild npm-browser-test image

https://gerrit.wikimedia.org/r/408876

Change 408876 merged by jenkins-bot:
[integration/config@master] Rebuild npm-browser-test image

https://gerrit.wikimedia.org/r/408876

Mentioned in SAL (#wikimedia-releng) [2018-02-08T10:02:58Z] <hashar> Rebuilding docker-pkg images on contint1001. Would get chromium 64 into npm-browser-test | T179552

Retried on https://gerrit.wikimedia.org/r/#/c/398527/ by using check experimental and the PDF rendering seems to work now:

00:01:01.053   html2pdf
00:01:02.881     ✓ should return a letter-sized PDF (1828ms)

It bails out at Renderer though: https://integration.wikimedia.org/ci/job/chromium-render-npm-browser-node-6-docker/28/console

I rebased https://gerrit.wikimedia.org/r/#/c/398527/ Prevent puppeteer from downloading Chromium , removed a Depends-On on abandoned patch and ... SUCCESS.

Console output is https://integration.wikimedia.org/ci/job/chromium-render-npm-browser-node-6-docker/29/console

Seems like we can move the Jenkins job to test pipeline so it get runs on every new patchset?

Seems like we can move the Jenkins job to test pipeline so it get runs on every new patchset?

That'd be great. Thanks, y'all!

Change 409115 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[integration/config@master] Update chromium-render to use Debian Chromium

https://gerrit.wikimedia.org/r/409115

Seems like we can move the Jenkins job to test pipeline so it get runs on every new patchset?

I've made the promotion here. For production and development reasons described in the patchset, I've moved @hashar's Puppeteer download disablement to the Jenkins Job Builder config. Thanks for all the help!

Change 398527 merged by Pmiazga:
[mediawiki/services/chromium-render@master] Prevent puppeteer from downloading Chromium

https://gerrit.wikimedia.org/r/398527

chromium-render-deploy

I'm not sure what's wanted for this part of the task. The repo barely exists and chromium-render will be a subfolder of it, if I understand correctly. I think we can punt on this until it's needed.

chromium-render

This is ready for review: https://gerrit.wikimedia.org/r/#/c/409115/ .

phuedx renamed this task from Set up Jenkins for chromium-render and chromium-render-deploy repositories to Set up Jenkins for chromium-render repository.Feb 14 2018, 7:24 AM
phuedx updated the task description. (Show Details)

I'm shuffling this over to blocked on our board since it needs releng review. Please check the patch out when you get a chance!

Change 410621 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/services/chromium-render@master] Update: add CHROME_BIN Puppeteer seam for CI tests

https://gerrit.wikimedia.org/r/410621

Change 409115 merged by jenkins-bot:
[integration/config@master] chromium-render must not download Chromium

https://gerrit.wikimedia.org/r/409115

Change 411085 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[integration/config@master] Update: promote chromium-render to test and gate-and-submit

https://gerrit.wikimedia.org/r/411085

Change 411085 merged by jenkins-bot:
[integration/config@master] Update: promote chromium-render to test and gate-and-submit

https://gerrit.wikimedia.org/r/411085

Thanks @hashar for getting those last couple patches across the finish line! @pmiazga, the final patch is here https://gerrit.wikimedia.org/r/#/c/410621/ and has been verified to run the tests successfully without downloading Chromium.

Change 410621 merged by Pmiazga:
[mediawiki/services/chromium-render@master] Update: add CHROME_BIN Puppeteer seam for CI tests

https://gerrit.wikimedia.org/r/410621

Niedzielski added a subscriber: Jdrewniak.

@Jdrewniak, would you mind QAing this guy and signing it off? Basically, you submit a patch to the chromium-render repo and see that it passes or fails as expected. Additionally, you should see in the Jenkins logs explicit mention that Chromium download has been skipped! If you have _any_ trouble, please ping me! Thank you!

Change 411297 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/services/chromium-render@master] [DNM] Failing a test to see if Jenkins -1's the patch

https://gerrit.wikimedia.org/r/411297

Change 411297 abandoned by Jdrewniak:
[DNM] Failing a test to see if Jenkins -1's the patch

Reason:
Jenkins failed the test successfully

https://gerrit.wikimedia.org/r/411297

Change 398529 merged by Niedzielski:
[mediawiki/services/chromium-render@master] Remove dependencies, use chromium instead

https://gerrit.wikimedia.org/r/398529

Ok, Jenkin's is successfully -1 and +2'ing the patches. This patch was +2'd https://gerrit.wikimedia.org/r/#/c/411292/

In all instances the cached version of Chromium is being used.
**INFO** Skipping Chromium download. "PUPPETEER_SKIP_CHROMIUM_DOWNLOAD" environment variable was found. Appears in the logs.

Also looks like Jenkins in merging the patches successfully too :)

That was epic! Thank you to everyone involved!

antoine-approve

Aside: That the above is a Phab keyword/meme, is as awesome as the hard work done for this task.

That is my token of appreciation whenever a task get fixed via team work :-} I hope the deployment part will be easier!