Dockerize WPT runs & add Jenkins k8s specs #153

jeffcarp · 2017-10-13T03:02:06Z

This PR introduces a number of big changes:

Adds a Dockerfile for generating a container capable of running any browser of WPT we currently run
- It does not bundle a version of FF, it uses --install-browser
- Currently this does not include Chrome
- The whole running setup is end-to-end tested in Travis CI (FF only for now)
Adds Kubernetes specs (in k8s/) for setting up a Jenkins instance (https://ci.wpt.fyi)

Future work after this lands (I will make bugs for these):

Get Chrome running in the container (this might involve writing --install-browser for Chrome)
Delete run/run.py once Chrome support lands
Further investigate sharding FF builds

More context around this work is described in this comment: #164 (comment)

lukebjerring · 2017-11-02T13:57:20Z

@mdittmer has ramped up on Docker recently - will get him to eyeball this.

mdittmer

Mostly requests for documentation and admissions of ignorance on my part.

mdittmer · 2017-11-02T17:05:01Z

ci_container.sh

@@ -0,0 +1,8 @@
+IMAGE_NAME=wptd-testrun-jenkins
+
+docker build -t $IMAGE_NAME .


nit: "${IMAGE_NAME}"

Done. Why is the bare $IMAGE_NAME undesirable?

If it ever contained spaces or special characters, then the meaning of the command would change.

mdittmer · 2017-11-02T17:05:20Z

ci_container.sh

+
+docker run \
+  -p 4445:4445 \
+  --entrypoint "/bin/bash" $IMAGE_NAME \


mdittmer · 2017-11-02T17:07:53Z

Dockerfile

+    libdbus-glib-1-2
+
+RUN mkdir /wptdashboard
+ADD . /wptdashboard


I think our base image should only include dependency installation and possibly some general environment setup (i.e., exclude ADD here and RUN below). Could we have a Dockerfile.jenkins or something for this?

What would the base image be used for other than test runs?

To test our code, to run a dev server for compiling protos, and/or to deploy modules to other environments.

Splitting into multiple Dockerfiles sounds reasonable, but for the sake of simplicity let's start with this one and as the scope is expanded with each PR we can split them up as necessary.

mdittmer · 2017-11-02T17:08:45Z

ci_container.sh

@@ -0,0 +1,8 @@
+IMAGE_NAME=wptd-testrun-jenkins
+
+docker build -t $IMAGE_NAME .


May need more flags if we want to build from Dockerfile.jenkins or similar.

mdittmer · 2017-11-02T17:09:47Z

ci_container.sh

@@ -0,0 +1,8 @@
+IMAGE_NAME=wptd-testrun-jenkins
+
+docker build -t $IMAGE_NAME .


Can we incorporate a version number here?

The image we push for the Jenkins bots to pull needs to have a constant name (otherwise updating the image will require changing the version in Jenkins).

I think the standard practice for this is to version images, and then use a :latest version/tag for use cases such as that. We could also use a version name like :jenkins if we want.

mdittmer · 2017-11-02T17:37:26Z

run/jenkins.py

+    p.communicate(input=patch)
+
+
+def get_and_validate_platform(wptd_path):


Documentation plz 😸

mdittmer · 2017-11-02T17:37:41Z

run/jenkins.py

+
+
+def report_to_summary(wpt_report):
+    test_files = {}


Ditto: Show me the docs.

mdittmer · 2017-11-02T17:38:04Z

run/jenkins.py

+
+
+def report_to_summary(wpt_report):
+    test_files = {}


Should there be code sharing between this file and run.py?

(see above - this file is meant to replace run.py)

mdittmer · 2017-11-02T17:38:52Z

run/jenkins.py

+    for result in wpt_report['results']:
+        test_file = result['test']
+        assert test_file not in test_files, (
+            'Assumption that each test_file only shows up once broken!')


More context? Which key showed up twice?

Added a comment.

My apologies; my comment wasn't clear. My intention was that the assertion error message should contain more context (such as the value of test_file).

mdittmer · 2017-11-02T17:39:28Z

run/jenkins.py

+        assert test_file not in test_files, (
+            'Assumption that each test_file only shows up once broken!')
+
+        if result['status'] in ('OK', 'PASS'):


Is there a schema somewhere we could lift legitimate values from? (I'm assuming not, but on the off chance there is...)

A schema for the possible test results? I'm not sure. This line in particular should probably have a discussion bug filed for it.

Yes, that is what I was asking: does WPT define the data format for its results (including this particular bit of the results).

jeffcarp · 2017-11-02T20:40:41Z

Addressed your feedback, thanks for taking a look @mdittmer

mdittmer · 2017-11-05T17:55:19Z

util/ci_container_inner.sh

@@ -6,6 +6,8 @@ export WPT_PATH=/web-platform-tests

 git clone --depth 1 https://github.com/w3c/web-platform-tests $WPT_PATH

+sudo chown -R $(whoami):$(whoami) $HOME


Prefer $(id -u $USER):$(id -g $USER) in case group not named after user.

Done, thanks

jeffcarp · 2017-11-07T00:24:19Z

Ok - I addressed the latest round of feedback. I'm going to go ahead and land this so we have something common to build off of. This is a massive change to our running infra and I think landing this now and following up with incremental improvement PRs is the way to go. 💯

This reverts commit 17eb327. Conflicts in run/jenkins.py resolved by deleting it (some commits touching jenkins.py after the reverted commit not reverted.)

jeffcarp added the test-running label Oct 13, 2017

jeffcarp self-assigned this Oct 13, 2017

jeffcarp force-pushed the docker branch 5 times, most recently from 42eb5c3 to e39e1f9 Compare October 17, 2017 02:49

jeffcarp mentioned this pull request Oct 18, 2017

Test every commit of web-platform-tests within 1 hour #164

Closed

jeffcarp changed the title ~~WIP idea: containerize builds and run in short-lived VMs~~ WIP: Dockerize builds Oct 30, 2017

jeffcarp changed the title ~~WIP: Dockerize builds~~ WIP: Dockerize WPT runs Oct 30, 2017

jeffcarp force-pushed the docker branch from a0b11a8 to b25c3ca Compare October 31, 2017 06:42

jeffcarp changed the title ~~WIP: Dockerize WPT runs~~ Dockerize WPT runs & add Jenkins k8s specs Nov 1, 2017

jeffcarp requested a review from lukebjerring November 1, 2017 22:29

jeffcarp force-pushed the docker branch 2 times, most recently from 889b7b2 to ef904ad Compare November 1, 2017 22:42

mdittmer reviewed Nov 2, 2017

View reviewed changes

jeffcarp force-pushed the docker branch from ef904ad to 0d2064e Compare November 2, 2017 18:10

Dockerize WPT runs and add Jenkins k8s specs

95fd845

jeffcarp force-pushed the docker branch from 0d2064e to 95fd845 Compare November 2, 2017 20:40

command line flags and chmod (#195)

c3ce844

mdittmer reviewed Nov 5, 2017

View reviewed changes

This was referenced Nov 7, 2017

Add sauce_name field in browsers.json #201

Open

Add a short doc describing the current state of running infra to doc/ #202

Closed

Address feedback

e8a88c6

jeffcarp merged commit 17eb327 into master Nov 7, 2017

jeffcarp mentioned this pull request Nov 7, 2017

Investigate using a Docker container to simplify test running #72

Closed

foolip added a commit that referenced this pull request Nov 28, 2017

Revert "Dockerize WPT runs & add Jenkins k8s specs (#153)"

1c41f1b

This reverts commit 17eb327. Conflicts in run/jenkins.py resolved by deleting it (some commits touching jenkins.py after the reverted commit not reverted.)

foolip deleted the docker branch December 1, 2017 22:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dockerize WPT runs & add Jenkins k8s specs #153

Dockerize WPT runs & add Jenkins k8s specs #153

jeffcarp commented Oct 13, 2017 •

edited

Loading

lukebjerring commented Nov 2, 2017

mdittmer left a comment

mdittmer Nov 2, 2017

jeffcarp Nov 2, 2017

mdittmer Nov 3, 2017

mdittmer Nov 2, 2017

mdittmer Nov 2, 2017

jeffcarp Nov 2, 2017

mdittmer Nov 3, 2017

jeffcarp Nov 7, 2017

mdittmer Nov 2, 2017

mdittmer Nov 2, 2017

jeffcarp Nov 2, 2017

mdittmer Nov 3, 2017

mdittmer Nov 2, 2017

jeffcarp Nov 2, 2017

mdittmer Nov 2, 2017

jeffcarp Nov 2, 2017

mdittmer Nov 2, 2017

jeffcarp Nov 2, 2017

mdittmer Nov 2, 2017

jeffcarp Nov 2, 2017

mdittmer Nov 3, 2017

mdittmer Nov 2, 2017

jeffcarp Nov 2, 2017

mdittmer Nov 3, 2017

jeffcarp commented Nov 2, 2017

mdittmer Nov 5, 2017

jeffcarp Nov 7, 2017

jeffcarp commented Nov 7, 2017

		@@ -0,0 +1,8 @@
		IMAGE_NAME=wptd-testrun-jenkins

		docker build -t $IMAGE_NAME .

		p.communicate(input=patch)


		def get_and_validate_platform(wptd_path):

		@@ -6,6 +6,8 @@ export WPT_PATH=/web-platform-tests

		git clone --depth 1 https://github.com/w3c/web-platform-tests $WPT_PATH

		sudo chown -R $(whoami):$(whoami) $HOME

Dockerize WPT runs & add Jenkins k8s specs #153

Dockerize WPT runs & add Jenkins k8s specs #153

Conversation

jeffcarp commented Oct 13, 2017 • edited Loading

lukebjerring commented Nov 2, 2017

mdittmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffcarp commented Nov 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffcarp commented Nov 7, 2017

jeffcarp commented Oct 13, 2017 •

edited

Loading