lighthouse-ci icon indicating copy to clipboard operation
lighthouse-ci copied to clipboard

fix(docker): include globally installed puppeteer

Open Eliasvdb opened this issue 3 years ago • 11 comments

I noticed puppeteer was not included in the lhci-client docker image. I altered the Dockerfile to include a global npm install of puppeteer. I set the ENV var PUPPETEER_SKIP_DOWNLOAD=true, because otherwise the puppeteer install will try to download chromium (but it will not have permission). As far as I tested this chrmium step is not needed as the docker image itself contains google-chrome-stable.

Eliasvdb avatar Jan 06 '22 08:01 Eliasvdb

cc @patrickhulce

Eliasvdb avatar Jan 06 '22 08:01 Eliasvdb

I just signed my CLA, but maybe it needs some time to process. That check should be green soon I guess.

Eliasvdb avatar Jan 06 '22 08:01 Eliasvdb

@patrickhulce I added the changes you suggested. I am unable to test it locally with your command though. When I build the docker image and run docker run -v "$(pwd)/lhci-data:/home/lhci/reports/.lighthouseci" -v "$(pwd)/scripts:/home/lhci/reports/scripts" lhci-client lhci collect --puppeteerScript=scripts/puppeteer-test.js --url=https://example.com from project root, I get the following error:

Failed to move to new namespace: PID namespaces supported, Network namespace supported, but failed: errno = Operation not permitted


TROUBLESHOOTING: https://github.com/puppeteer/puppeteer/blob/main/docs/troubleshooting.md

    at onClose (/usr/local/lib/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserRunner.js:229:20)
    at ChildProcess.<anonymous> (/usr/local/lib/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserRunner.js:220:79)
    at ChildProcess.emit (events.js:326:22)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:276:12)

I tried setting --puppeteerLaunchOptions, like so: docker run -v "$(pwd)/lhci-data:/home/lhci/reports/.lighthouseci" -v "$(pwd)/scripts:/home/lhci/reports/scripts" lhci-client lhci collect --puppeteerScript=scripts/puppeteer-test.js --puppeteerLaunchOptions="{'args':['--no-sandbox', '--disable-setuid-sandbox']}" --url=https://example.com, but that isn't working either (same error), am I setting them correctly?

Eliasvdb avatar Feb 10 '22 09:02 Eliasvdb

but that isn't working either (same error), am I setting them correctly?

No puppeteerLaunchOptions isn't parsed as a string. I would expect --puppeteerLaunchOptions.args=--no-sandbox --puppeteerLaunchOptions.args=--disable-setuid-sandbox to work though.

patrickhulce avatar Feb 10 '22 14:02 patrickhulce

I would expect --puppeteerLaunchOptions.args=--no-sandbox --puppeteerLaunchOptions.args=--disable-setuid-sandbox to work though

Ah yes, that's it! Seems to work correctly then. Whenever a puppeteerscript is provided it seems puppeteer will need to be launched with both those options though. Should those be configured somewhere then instead of leaving it up to the user to specify them? What do you think?

Let me know if you would like to see anything changed, I'll make sure to fix it sooner this time 😃

Eliasvdb avatar Feb 10 '22 15:02 Eliasvdb

Should those be configured somewhere then instead of leaving it up to the user to specify them? What do you think?

Nah, let's just add a doc example of how to properly invoke it 👍

CLA issues + an example in the docs and I think we're good to go 🎉

patrickhulce avatar Feb 10 '22 15:02 patrickhulce

Nah, let's just add a doc example of how to properly invoke it 👍

I'll make sure to add it!

CLA issues + an example in the docs and I think we're good to go 🎉

I'm not sure what the problem is, I have signed the CLA with my GitHub username 🤔

Eliasvdb avatar Feb 10 '22 15:02 Eliasvdb

I added the info to the documentation.

Now I only need to figure out why my CLA is seen as missing...

Eliasvdb avatar Feb 10 '22 15:02 Eliasvdb

@patrickhulce Found and solved the CLA issue! If there's anything else, let me know!

Eliasvdb avatar Feb 11 '22 09:02 Eliasvdb

Those failing tests seem unrelated to my change, does it happen more often? Seems timeout related?

Eliasvdb avatar Feb 11 '22 15:02 Eliasvdb

They probably are, there are quite a few flakes.

patrickhulce avatar Feb 12 '22 20:02 patrickhulce

Hey team, are we still planning on merging this ? Thanks 🙏

Djiit avatar Nov 10 '22 09:11 Djiit