Bug 1391476 - Require that all cache paths be declared as volumes; r?dustin draft
authorGregory Szorc <gps@mozilla.com>
Wed, 23 Aug 2017 08:57:59 -0700
changeset 651408 4d9331997a1550d71a19cb7dab3e24c2a125b1bc
parent 651407 a53676dc0c4f26e7e2cdd36ad4d7039cf7a98923
child 651409 1ee5da62f1f3cf399d9d14b1f9f9c449a7c45b0d
push id75721
push usergszorc@mozilla.com
push dateWed, 23 Aug 2017 17:48:59 +0000
reviewersdustin
bugs1391476
milestone57.0a1
Bug 1391476 - Require that all cache paths be declared as volumes; r?dustin See the inline comment for the rationale here. This check may not catch all volumes and caches. But after subsequent commits refactor how permissions for caches and volumes are handled, this edge case will likely result in permissions errors in the task, so it isn't worth worrying about. Several Dockerfile have been updated to add missing VOLUME so the check passes. In the case of desktop1604-test, we stopped removing /home/worker/.cache because you can't remove a mount point, which is what volumes are inside Docker containers. MozReview-Commit-ID: GEyNkkX00kN
taskcluster/docker/android-gradle-build/Dockerfile
taskcluster/docker/desktop-build/Dockerfile
taskcluster/docker/desktop1604-test/Dockerfile
taskcluster/docker/image_builder/Dockerfile
taskcluster/docker/lint/Dockerfile
taskcluster/docker/valgrind-build/Dockerfile
taskcluster/taskgraph/transforms/docker_image.py
taskcluster/taskgraph/transforms/task.py
--- a/taskcluster/docker/android-gradle-build/Dockerfile
+++ b/taskcluster/docker/android-gradle-build/Dockerfile
@@ -1,15 +1,15 @@
 # TODO remove VOLUME below when the base image is updated next.
 FROM          taskcluster/centos6-build-upd:0.1.6.20160329195300
 MAINTAINER    Nick Alexander <nalexander@mozilla.com>
 
 # BEGIN ../desktop-build/Dockerfile
 
-# TODO remove when base image is updated
+VOLUME /home/worker/checkouts
 VOLUME /home/worker/workspace
 VOLUME /home/worker/tooltool-cache
 
 # %include python/mozbuild/mozbuild/action/tooltool.py
 COPY topsrcdir/python/mozbuild/mozbuild/action/tooltool.py /build/tooltool.py
 COPY topsrcdir/python/mozbuild/mozbuild/action/tooltool.py /builds/tooltool.py
 COPY topsrcdir/python/mozbuild/mozbuild/action/tooltool.py /setup/tooltool.py
 COPY topsrcdir/python/mozbuild/mozbuild/action/tooltool.py /tmp/tooltool.py
--- a/taskcluster/docker/desktop-build/Dockerfile
+++ b/taskcluster/docker/desktop-build/Dockerfile
@@ -1,15 +1,16 @@
 # TODO remove VOLUME below when the base image is updated next.
 FROM          taskcluster/centos6-build-upd:0.1.7.20170801103900
 MAINTAINER    Dustin J. Mitchell <dustin@mozilla.com>
 
-# TODO remove when base image is updated
 VOLUME /home/worker/workspace
 VOLUME /home/worker/tooltool-cache
+VOLUME /home/worker/checkouts
+VOLUME /home/worker/.tc-vcs
 
 # Add build scripts; these are the entry points from the taskcluster worker, and
 # operate on environment variables
 ADD             bin /home/worker/bin
 RUN             chmod +x /home/worker/bin/*
 
 # %include python/mozbuild/mozbuild/action/tooltool.py
 ADD topsrcdir/python/mozbuild/mozbuild/action/tooltool.py /builds/tooltool.py
--- a/taskcluster/docker/desktop1604-test/Dockerfile
+++ b/taskcluster/docker/desktop1604-test/Dockerfile
@@ -1,14 +1,22 @@
 FROM          ubuntu:16.04
 MAINTAINER    Joel Maher <joel.maher@gmail.com>
 
 RUN useradd -d /home/worker -s /bin/bash -m worker
 WORKDIR /home/worker
 
+# We need to declare all potentially cache volumes as caches. Also,
+# making high I/O paths volumes increase I/O throughput because of
+# AUFS slowness.
+VOLUME /home/worker/.cache
+VOLUME /home/worker/checkouts
+VOLUME /home/worker/tooltool-cache
+VOLUME /home/worker/workspace
+
 # %include python/mozbuild/mozbuild/action/tooltool.py
 ADD topsrcdir/python/mozbuild/mozbuild/action/tooltool.py /setup/tooltool.py
 
 # %include testing/mozharness/external_tools/robustcheckout.py
 ADD topsrcdir/testing/mozharness/external_tools/robustcheckout.py /usr/local/mercurial/robustcheckout.py
 
 # %include taskcluster/docker/recipes/common.sh
 ADD topsrcdir/taskcluster/docker/recipes/common.sh /setup/common.sh
@@ -32,22 +40,16 @@ RUN           bash /setup/system-setup.s
 ADD topsrcdir/taskcluster/docker/recipes/xvfb.sh /home/worker/scripts/xvfb.sh
 
 # %include taskcluster/docker/recipes/run-task
 ADD topsrcdir/taskcluster/docker/recipes/run-task /home/worker/bin/run-task
 
 # %include taskcluster/scripts/tester/test-linux.sh
 ADD topsrcdir/taskcluster/scripts/tester/test-linux.sh /home/worker/bin/test-linux.sh
 
-# This will create a host mounted filesystem when the cache is stripped
-# on Try. This cancels out some of the performance losses of aufs. See
-# bug 1291940.
-VOLUME /home/worker/checkouts
-VOLUME /home/worker/workspace
-
 # Set variable normally configured at login, by the shells parent process, these
 # are taken from GNU su manual
 ENV           HOME          /home/worker
 ENV           SHELL         /bin/bash
 ENV           USER          worker
 ENV           LOGNAME       worker
 ENV           HOSTNAME      taskcluster-worker
 ENV           LANG          en_US.UTF-8
@@ -74,19 +76,16 @@ RUN mkdir Documents; mkdir Pictures; mkd
 RUN npm install -g taskcluster-vcs@2.3.12 \
  && npm install -g taskcluster-npm-cache@1.1.14 \
  && rm -rf ~/.npm
 ENV PATH $PATH:/home/worker/bin
 
 # TODO Re-enable worker when bug 1093833 lands
 #USER          worker
 
-# clean up
-RUN rm -Rf .cache && mkdir -p .cache
-
 # Disable Ubuntu update prompt
 # http://askubuntu.com/questions/515161/ubuntu-12-04-disable-release-notification-of-14-04-in-update-manager
 ADD release-upgrades /etc/update-manager/release-upgrades
 
 # Disable tools with on-login popups that interfere with tests; see bug 1240084 and bug 984944.
 ADD autostart/jockey-gtk.desktop autostart/deja-dup-monitor.desktop /etc/xdg/autostart/
 
 # Bug 1345105 - Do not run periodical update checks and downloads
--- a/taskcluster/docker/image_builder/Dockerfile
+++ b/taskcluster/docker/image_builder/Dockerfile
@@ -16,17 +16,18 @@ ADD topsrcdir/testing/mozharness/externa
 ADD topsrcdir/taskcluster/docker/recipes/run-task /usr/local/bin/run-task
 
 # Add and run setup script
 ADD build-image.sh      /usr/local/bin/build-image.sh
 ADD download-and-compress /usr/local/bin/download-and-compress
 ADD setup.sh            /setup/setup.sh
 RUN bash /setup/setup.sh
 
-# Setup a workspace that won't use AUFS
+# Setup a workspace that won't use AUFS.
+VOLUME /home/worker/checkouts
 VOLUME /home/worker/workspace
 
 # Set variable normally configured at login, by the shells parent process, these
 # are taken from GNU su manual
 ENV           HOME          /home/worker
 ENV           SHELL         /bin/bash
 ENV           USER          worker
 ENV           LOGNAME       worker
--- a/taskcluster/docker/lint/Dockerfile
+++ b/taskcluster/docker/lint/Dockerfile
@@ -1,14 +1,17 @@
 FROM          ubuntu:16.04
 MAINTAINER    Andrew Halberstadt <ahalberstadt@mozilla.com>
 
 RUN useradd -d /home/worker -s /bin/bash -m worker
 WORKDIR /home/worker
 
+VOLUME /home/worker/.cache
+VOLUME /home/worker/checkouts
+
 RUN mkdir /build
 # %include python/mozbuild/mozbuild/action/tooltool.py
 ADD topsrcdir/python/mozbuild/mozbuild/action/tooltool.py /build/tooltool.py
 
 # %include testing/mozharness/external_tools/robustcheckout.py
 ADD topsrcdir/testing/mozharness/external_tools/robustcheckout.py /usr/local/mercurial/robustcheckout.py
 
 # %include taskcluster/docker/recipes/install-node.sh
--- a/taskcluster/docker/valgrind-build/Dockerfile
+++ b/taskcluster/docker/valgrind-build/Dockerfile
@@ -1,13 +1,13 @@
 # TODO remove VOLUME below when the base image is updated next.
 FROM          taskcluster/centos6-build-upd:0.1.7.20170801103900
 MAINTAINER    Dustin J. Mitchell <dustin@mozilla.com>
 
-# TODO remove when base image is updated
+VOLUME /home/worker/checkouts
 VOLUME /home/worker/workspace
 VOLUME /home/worker/tooltool-cache
 
 # Add build scripts; these are the entry points from the taskcluster worker, and
 # operate on environment variables
 
 # %include taskcluster/docker/desktop-build/bin
 ADD topsrcdir/taskcluster/docker/desktop-build/bin /home/worker/bin
--- a/taskcluster/taskgraph/transforms/docker_image.py
+++ b/taskcluster/taskgraph/transforms/docker_image.py
@@ -97,16 +97,21 @@ def fill_template(config, tasks):
                 'implementation': 'docker-worker',
                 'os': 'linux',
                 'docker-image': docker_image('image_builder'),
                 'caches': [{
                     'type': 'persistent',
                     'name': 'level-{}-imagebuilder-v1'.format(config.params['level']),
                     'mount-point': '/home/worker/checkouts',
                 }],
+                'volumes': [
+                    # Keep in sync with Dockerfile.
+                    '/home/worker/checkouts',
+                    '/home/worker/workspace',
+                ],
                 'artifacts': [{
                     'type': 'file',
                     'path': '/home/worker/workspace/artifacts/image.tar.zst',
                     'name': 'public/image.tar.zst',
                 }],
                 'env': {
                     'HG_STORE_PATH': '/home/worker/checkouts/hg-store',
                     'HASH': context_hash,
--- a/taskcluster/taskgraph/transforms/task.py
+++ b/taskcluster/taskgraph/transforms/task.py
@@ -741,16 +741,18 @@ def build_docker_worker_payload(config, 
 
     # coalesce / superseding
     if 'coalesce-name' in task and level > 1:
         key = COALESCE_KEY.format(
             project=config.params['project'],
             name=task['coalesce-name'])
         payload['supersederUrl'] = "https://coalesce.mozilla-releng.net/v1/list/" + key
 
+    check_caches_are_volumes(task)
+
 
 @payload_builder('generic-worker')
 def build_generic_worker_payload(config, task, task_def):
     worker = task['worker']
 
     artifacts = []
 
     for artifact in worker['artifacts']:
@@ -1150,16 +1152,40 @@ def build_task(config, tasks):
             'label': task['label'],
             'task': task_def,
             'dependencies': task.get('dependencies', {}),
             'attributes': attributes,
             'optimizations': task.get('optimizations', []),
         }
 
 
+def check_caches_are_volumes(task):
+    """Ensures that all cache paths are defined as volumes.
+
+    Caches and volumes are the only filesystem locations whose content
+    isn't defined by the Docker image itself. Some caches are optional
+    depending on the job environment. We want paths that are potentially
+    caches to have as similar behavior regardless of whether a cache is
+    used. To help enforce this, we require that all paths used as caches
+    to be declared as Docker volumes. This check won't catch all offenders.
+    But it is better than nothing.
+    """
+    volumes = set(task['worker']['volumes'])
+    paths = set(c['mount-point'] for c in task['worker'].get('caches', []))
+    missing = paths - volumes
+
+    if not missing:
+        return
+
+    raise Exception('task %s (image %s) has caches that are not declared as '
+                    'Docker volumes: %s' % (task['label'],
+                                            task['worker']['docker-image'],
+                                            ', '.join(sorted(missing))))
+
+
 @transforms.add
 def check_run_task_caches(config, tasks):
     """Audit for caches requiring run-task.
 
     run-task manages caches in certain ways. If a cache managed by run-task
     is used by a non run-task task, it could cause problems. So we audit for
     that and make sure certain cache names are exclusive to run-task.