Bug 1391476 - Require that all cache paths be declared as volumes; r?dustin draft
authorGregory Szorc <gps@mozilla.com>
Tue, 22 Aug 2017 17:40:20 -0700
changeset 650872 a3010c14c6d45c6bd3765fd574c3900593f160b1
parent 650860 6a1c5d468b86b13d1f286d3263c10849f3da9cd2
child 650873 c7df9aec9234933051d83b94840c8ba259e72fb7
push id75523
push usergszorc@mozilla.com
push dateWed, 23 Aug 2017 00:42:07 +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. Depending on the order transforms run in, it is possible to evade this check. But I think it is good enough. Plus subsequent commits will essentially make tasks without properly declared caches and volumes impossible to run without permissions failures. 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,16 @@
 # 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,17 @@
 # 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,19 @@ 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.
+# Keep this list in sync with taskgraph.transforms.docker_image.fill_template()
+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,14 @@
 # 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
@@ -735,16 +735,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']:
@@ -1144,16 +1146,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.