Merge lp:~allenap/maas/parallel-tests-reliability into lp:~maas-committers/maas/trunk

[Gavin Panella]

Make bin/test.parallel reliable.

I have run bin/test.parallel 10 times with only one failure. This turned out to be a broken test; see the fix in https://code.launchpad.net/~allenap/maas/finding-something-that-should-not-be-there--bug-1659244/+merge/315552.

I haven't swapped bin/test.parallel into the `make test` slot because it does not yet produce XUnit/JUnit output which Jenkins consumes. I also would like to get feedback from other team members to see if it is working reliably for them in different environments to my own.

[Blake Rouse (blake-rouse)]

Looks good. Just a couple of questions.

review: Needs Information

[Gavin Panella (allenap)]

Thanks. I fixed a couple of problems you encountered when running bin/test.parallel.

I'm not sure how to fix the "No address found for host localhost" problem you hit (http://paste.ubuntu.com/23863538/). I've seen this happen too.

I would like to get test.js working again, locally, but there's no obvious reason why it breaks.

[Blake Rouse (blake-rouse)]

Well test.js works locally for me. It also must work on the lander as its a requirement for branches to land. As long as this branch doesn't remove that requirement I am good with landing it, once that test failure I am getting is fixed.

review: Approve

[Gavin Panella (allenap)]

Re-enabled test.js, and test failure fixed. Thanks!

=== modified file 'Makefile'
--- Makefile	2017-01-20 11:30:38 +0000
+++ Makefile	2017-01-25 16:51:52 +0000
@@ -90,10 +90,6 @@
 	echo '/src/**/*.pyc' >> $@
 	echo '/etc/**/*.pyc' >> $@
 
-run/etc/ntp.conf: templates/ntp.conf
-	@mkdir -p $(@D)
-	@cp templates/ntp.conf $@
-
 configure-buildout:
 	utilities/configure-buildout
 
@@ -163,8 +159,7 @@
 	@touch --no-create $@
 
 bin/test.rack: \
-  bin/buildout buildout.cfg versions.cfg setup.py bin/maas-rack \
-  run/etc/ntp.conf
+  bin/buildout buildout.cfg versions.cfg setup.py bin/maas-rack
 	$(buildout) install rack-test
 	@touch --no-create $@
 
@@ -397,7 +392,7 @@
 	find . -type d -name '__pycache__' -print0 | xargs -r0 $(RM) -r
 	find . -type f -name '*~' -print0 | xargs -r0 $(RM)
 	find . -type f -name dropin.cache -print0 | xargs -r0 $(RM)
-	$(RM) -r media/demo/* media/development
+	$(RM) -r media/demo/* media/development media/development.*
 	$(RM) $(js_enums) $(js_enums).tmp
 	$(RM) src/maasserver/data/templates.py
 	$(RM) *.log

=== modified file 'media/README'
--- media/README	2016-12-07 12:46:14 +0000
+++ media/README	2017-01-25 16:51:52 +0000
@@ -1,5 +1,5 @@
 This folder contains somewhat ephemeral things: subfolders serve as
-MEDIA_ROOT for maasserver.djangosettings.demo and .development
-environments. The media/demo directory should always exist and not be
-deleted, though its contents can be. The media/development directory
-should be created and destroyed by tests, as needed.
+MEDIA_ROOT for `demo` and `development` environments. The media/demo
+directory should always exist and not be deleted, though its contents
+can be. The media/development.$$ (where $$ is the PID of the running
+process) directories are created and destroyed by tests, as needed.

=== modified file 'required-packages/forbidden'
--- required-packages/forbidden	2014-09-24 13:05:22 +0000
+++ required-packages/forbidden	2017-01-25 16:51:52 +0000
@@ -0,0 +1,3 @@
+python3-testresources
+python3-testscenarios
+python3-testtools

=== renamed file 'templates/ntp.conf' => 'run/etc/ntp.conf'
=== modified file 'src/maasserver/api/tests/test_discoveries.py'
--- src/maasserver/api/tests/test_discoveries.py	2016-12-12 14:17:23 +0000
+++ src/maasserver/api/tests/test_discoveries.py	2017-01-25 16:51:52 +0000
@@ -9,6 +9,7 @@
 import http.client
 import json
 import random
+from unittest.mock import ANY
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
@@ -519,8 +520,11 @@
         scan_all_rack_networks(cidrs=cidrs)
         self.assertThat(
             self.call_racks_sync_mock, MockCalledOnceWith(
-                cluster.ScanNetworks, controllers=self.started,
+                cluster.ScanNetworks, controllers=ANY,
                 kwargs={'cidrs': cidrs}))
+        # Check `controllers` separately because its order may vary.
+        controllers = self.call_racks_sync_mock.call_args[1]["controllers"]
+        self.assertItemsEqual(self.started, controllers)
 
     def test__calls_racks_synchronously_with_force_ping(self):
         scan_all_rack_networks(ping=True)

=== modified file 'src/maasserver/djangosettings/development.py'
--- src/maasserver/djangosettings/development.py	2017-01-20 08:22:34 +0000
+++ src/maasserver/djangosettings/development.py	2017-01-25 16:51:52 +0000
@@ -63,7 +63,9 @@
 
 # Absolute filesystem path to the directory that will hold user-uploaded files.
 # Example: "/home/media/media.lawrence.com/media/"
-MEDIA_ROOT = abspath("media/development")
+# To allow tests to run in parallel the current process's PID is part of this
+# path. See MediaRootFixture for details.
+MEDIA_ROOT = abspath("media/development.%d" % os.getpid())
 
 INTERNAL_IPS = ('127.0.0.1', '::1')
 

=== modified file 'src/maasserver/regiondservices/tests/test_ntp.py'
--- src/maasserver/regiondservices/tests/test_ntp.py	2016-12-12 14:17:23 +0000
+++ src/maasserver/regiondservices/tests/test_ntp.py	2017-01-25 16:51:52 +0000
@@ -5,8 +5,6 @@
 
 __all__ = []
 
-from os.path import join
-
 from crochet import wait_for
 from maasserver.models.config import Config
 from maasserver.regiondservices import ntp
@@ -123,10 +121,6 @@
         # Ensure that we never actually execute against systemd.
         self.patch_autospec(service_monitor, "restartService")
 
-        maasroot = self.useFixture(MAASRootFixture()).path
-        with open(join(maasroot, "etc", "ntp.conf"), "w") as ntp_conf:
-            ntp_conf.write("# Placeholder NTP configuration file.\n")
-
         with TwistedLoggerFixture() as logger:
             yield service._tryUpdate()
 

=== modified file 'src/maasserver/testing/resources.py'
--- src/maasserver/testing/resources.py	2017-01-20 08:21:22 +0000
+++ src/maasserver/testing/resources.py	2017-01-25 16:51:52 +0000
@@ -217,6 +217,7 @@
 
     def make(self, dependencies):
         databases = dependencies["templates"]
+        clusterlock = databases.cluster.lock
         with databases.cluster.connect() as conn:
             with conn.cursor() as cursor:
                 for database in databases:
@@ -226,7 +227,11 @@
                     stmt = (
                         "CREATE DATABASE %s WITH TEMPLATE %s"
                         % (dbname, template))
-                    cursor.execute(stmt)
+                    # Create the database with a shared lock to the cluster to
+                    # avoid racing a DjangoPristineDatabaseManager.make in a
+                    # concurrently running test process.
+                    with clusterlock.shared:
+                        cursor.execute(stmt)
                     debug(
                         "Created {dbname}; statement: {stmt}",
                         dbname=dbname, stmt=stmt)

=== modified file 'src/maasserver/utils/tests/test_dblocks.py'
--- src/maasserver/utils/tests/test_dblocks.py	2015-12-01 18:12:59 +0000
+++ src/maasserver/utils/tests/test_dblocks.py	2017-01-25 16:51:52 +0000
@@ -9,7 +9,7 @@
     closing,
     contextmanager,
 )
-from random import randint
+from random import randrange
 import sys
 
 from django.db import (
@@ -25,18 +25,24 @@
 from maasserver.utils import dblocks
 from testtools.matchers import Equals
 
+# Use "high" objid numbers to avoid conflicts with predeclared locks.
+objid_min = 2 << 10
+objid_max = 2 << 16
+
 
 def get_locks():
-    """Return the set of locks held."""
-    stmt = "SELECT objid FROM pg_locks WHERE classid = %s"
+    """Return the set of locks held between `objid_min` and `objid_max`."""
     with closing(connection.cursor()) as cursor:
-        cursor.execute(stmt, [dblocks.classid])
+        cursor.execute(
+            "SELECT objid FROM pg_locks "
+            "WHERE classid = %s AND objid >= %s AND objid < %s",
+            [dblocks.classid, objid_min, objid_max])
         return {result[0] for result in cursor.fetchall()}
 
 
 def random_objid():
     """Return a 'high' objid that's won't coincide with predeclared locks."""
-    return randint(2 << 10, 2 << 16)
+    return randrange(objid_min, objid_max)
 
 
 @transaction.atomic
@@ -99,10 +105,15 @@
         objid = random_objid()
         lock = self.make_lock(objid)
 
-        locks_held_before = get_locks()
-        with lock:
-            locks_held = get_locks()
-        locks_held_after = get_locks()
+        # Take an exclusive lock on the database cluster to prevent this
+        # section from running concurrently with any other thusly delimited
+        # section. Note that `self.databases` is the test resource for the
+        # database(s), which then has a reference to the cluster resource.
+        with self.databases.cluster.lock.exclusive:
+            locks_held_before = get_locks()
+            with lock:
+                locks_held = get_locks()
+            locks_held_after = get_locks()
 
         locks_obtained = locks_held - locks_held_before
         self.assertEqual({objid}, locks_obtained)
@@ -266,12 +277,16 @@
         objid = random_objid()
         lock = self.make_lock(objid)
 
-        with transaction.atomic():
-            locks_held_before = get_locks()
-            with lock:
-                locks_held = get_locks()
-            locks_held_after = get_locks()
-        locks_held_after_txn = get_locks()
+        # Take an exclusive lock on the database cluster to prevent this
+        # section from running concurrently with any other thusly delimited
+        # section.
+        with self.databases.cluster.lock.exclusive:
+            with transaction.atomic():
+                locks_held_before = get_locks()
+                with lock:
+                    locks_held = get_locks()
+                locks_held_after = get_locks()
+            locks_held_after_txn = get_locks()
 
         locks_obtained = locks_held - locks_held_before
         self.assertEqual({objid}, locks_obtained)

=== modified file 'src/maasserver/utils/tests/test_dbtasks.py'
--- src/maasserver/utils/tests/test_dbtasks.py	2016-06-22 17:03:02 +0000
+++ src/maasserver/utils/tests/test_dbtasks.py	2017-01-25 16:51:52 +0000
@@ -291,7 +291,7 @@
 
         self.assertDocTestMatches(
             """\
-            Unhandled failure in database task.
+            ...Unhandled failure in database task.
             Traceback (most recent call last):
             ...
             builtins.ZeroDivisionError: ...

=== modified file 'src/maasserver/websockets/handlers/tests/test_subnet.py'
--- src/maasserver/websockets/handlers/tests/test_subnet.py	2016-12-20 06:52:13 +0000
+++ src/maasserver/websockets/handlers/tests/test_subnet.py	2017-01-25 16:51:52 +0000
@@ -5,6 +5,7 @@
 
 __all__ = []
 
+import re
 from unittest.mock import sentinel
 
 from fixtures import FakeLogger
@@ -15,14 +16,14 @@
 from maasserver.utils.orm import reload_object
 from maasserver.websockets.base import dehydrate_datetime
 from maasserver.websockets.handlers.subnet import SubnetHandler
-from maastesting.matchers import (
-    DocTestMatches,
-    MockCalledOnceWith,
-)
+from maastesting.matchers import MockCalledOnceWith
 from netaddr import IPNetwork
 from provisioningserver.utils.network import IPRangeStatistics
 from testtools import ExpectedException
-from testtools.matchers import Equals
+from testtools.matchers import (
+    Equals,
+    MatchesRegex,
+)
 
 
 class TestSubnetHandler(MAASServerTestCase):
@@ -160,8 +161,11 @@
         handler.scan({
             "id": subnet.id,
         })
-        self.assertThat(logger.output, DocTestMatches(
-            "User...%s...scan...%s" % (user.username, cidr)))
+        # Use MatchesRegex here rather than DocTestMatches because usernames
+        # can contain characters that confuse DocTestMatches (e.g. periods).
+        self.assertThat(logger.output, MatchesRegex(
+            "User '%s' initiated a neighbour discovery scan against subnet: %s"
+            % (re.escape(user.username), re.escape(str(cidr)))))
 
     def test__scan_ipv6_fails(self):
         user = factory.make_admin()

=== modified file 'src/maastesting/fixtures.py'
--- src/maastesting/fixtures.py	2016-08-09 00:14:06 +0000
+++ src/maastesting/fixtures.py	2017-01-25 16:51:52 +0000
@@ -22,10 +22,9 @@
 )
 import logging
 import os
+from pathlib import Path
 from subprocess import (
     CalledProcessError,
-    check_output,
-    DEVNULL,
     PIPE,
     Popen,
 )
@@ -404,7 +403,7 @@
 
 
 class MAASRootFixture(fixtures.Fixture):
-    """Copy an existing `MAAS_ROOT` to a new temporary location.
+    """Create a new, pristine, `MAAS_ROOT` in a temporary location.
 
     Also updates `MAAS_ROOT` in the environment to point to this new location.
     """
@@ -416,10 +415,25 @@
             raise NotADirectoryError("MAAS_ROOT is not defined.")
         elif os.path.isdir(maasroot):
             self.path = self.useFixture(TempDirectory()).join("run")
-            # Neither shutil.copytree nor /bin/cp deal with relative symlinks,
-            # but /bin/cp is much faster at copying dereferenced files.
-            command = "/bin/cp", "-r", "--dereference", maasroot, self.path
-            check_output(command, stdin=DEVNULL, stderr=PIPE)
+            # Work only in $MAAS_ROOT/run; reference the old $MAAS_ROOT.
+            etc = Path(self.path).joinpath("etc")
+            src = Path(maasroot)
+            # Create and populate $MAAS_ROOT/run/etc/{ntp,ntp.conf}. The
+            # `.keep` file is not strictly necessary, but it's created for
+            # consistency with the source tree's `run` directory.
+            ntp = etc.joinpath("ntp")
+            ntp.mkdir(parents=True)
+            ntp.joinpath(".keep").touch()
+            ntp_conf = etc.joinpath("ntp.conf")
+            ntp_conf.write_bytes(src.joinpath("etc", "ntp.conf").read_bytes())
+            # Create and populate $MAAS_ROOT/run/etc/maas.
+            maas = etc.joinpath("maas")
+            maas.mkdir(parents=True)
+            maas.joinpath("drivers.yaml").symlink_to(
+                src.joinpath("etc", "maas", "drivers.yaml").resolve())
+            maas.joinpath("templates").symlink_to(
+                src.joinpath("etc", "maas", "templates").resolve())
+            # Update the environment.
             self.useFixture(EnvironmentVariable("MAAS_ROOT", self.path))
         else:
             raise NotADirectoryError(

=== modified file 'src/maastesting/testcase.py'
--- src/maastesting/testcase.py	2017-01-11 09:19:28 +0000
+++ src/maastesting/testcase.py	2017-01-25 16:51:52 +0000
@@ -24,7 +24,10 @@
 import crochet
 from maastesting.crochet import EventualResultCatchingMixin
 from maastesting.factory import factory
-from maastesting.fixtures import TempDirectory
+from maastesting.fixtures import (
+    MAASRootFixture,
+    TempDirectory,
+)
 from maastesting.matchers import DocTestMatches
 from maastesting.runtest import (
     MAASCrochetRunTest,
@@ -154,6 +157,10 @@
                 return MAASRunTest
 
     def setUp(self):
+        # Every test gets a pristine MAAS_ROOT, when it is defined.
+        if "MAAS_ROOT" in os.environ:
+            self.useFixture(MAASRootFixture())
+
         # Capture Twisted logs and add them as a test detail.
         twistedLog = self.useFixture(TwistedLoggerFixture())
         self.addDetail("Twisted logs", twistedLog.getContent())

=== modified file 'src/maastesting/tests/test_conflict_markers.py'
--- src/maastesting/tests/test_conflict_markers.py	2015-12-01 18:12:59 +0000
+++ src/maastesting/tests/test_conflict_markers.py	2017-01-25 16:51:52 +0000
@@ -5,12 +5,16 @@
 
 __all__ = []
 
-from pipes import quote
+from os.path import (
+    isdir,
+    join,
+)
 from subprocess import (
     PIPE,
     Popen,
     STDOUT,
 )
+from unittest import skipIf
 
 from maastesting import root
 from maastesting.testcase import MAASTestCase
@@ -19,25 +23,24 @@
     UTF8_TEXT,
 )
 
-# Do not use '=======' as a conflict marker since it's
-# used in docstrings.
-# Express the conflict markers so that this very file won't contain
-# them.
-CONFLICT_MARKERS = "<" * 7, ">" * 7
-
 
 class TestConflictMarkers(MAASTestCase):
+    """Search for merge conflict markers if this is a branch."""
 
-    def execute(self, *command):
-        process = Popen(command, stdout=PIPE, stderr=STDOUT, cwd=root)
+    @skipIf(not isdir(join(root, ".bzr")), "Not a branch.")
+    def test_no_conflict_markers(self):
+        # Do not search for '=======' as a conflict marker since it's used in
+        # docstrings, search for angle brackets instead. Express the conflict
+        # markers as a regular expression so that this very file won't match.
+        command = (
+            "bzr ls --kind=file --recursive --versioned --null | "
+            "xargs -r0 egrep -I '[<]{7}|[>]{7}' -C 3")
+        process = Popen(
+            command, shell=True, stdout=PIPE, stderr=STDOUT, cwd=root)
         output, _ = process.communicate()
         if len(output) != 0:
-            name = "stdout/err from `%s`" % " ".join(map(quote, command))
+            name = "stdout/err from `%s`" % command
             self.addDetail(name, Content(UTF8_TEXT, lambda: [output]))
-            self.assertEqual('', output, "Conflict markers present!")
-        self.assertEqual(1, process.wait(), "(return code is not one)")
-
-    def test_no_conflict_markers(self):
-        command = ["egrep", "-rI", "--exclude=*~", "--exclude-dir=include"]
-        command.append("|".join(CONFLICT_MARKERS))
-        self.execute(*command)
+            self.fail("Conflict markers present!")
+        # Don't check the process's exit code because xargs muddles things.
+        # Checking the output should suffice.

=== modified file 'src/maastesting/tests/test_fixtures.py'
--- src/maastesting/tests/test_fixtures.py	2016-08-09 00:14:06 +0000
+++ src/maastesting/tests/test_fixtures.py	2017-01-25 16:51:52 +0000
@@ -266,6 +266,17 @@
             self.assertThat(fixture.path, Not(SamePath(self.maasroot)))
             files_expected = set(listdirs(self.maasroot))
             files_observed = set(listdirs(fixture.path))
+
+            # Some files, like regiond.conf and rackd.conf, may be created by
+            # concurrently running test suites. In particular, regiond.conf is
+            # created as a side-effect of loading Django's settings which
+            # takes place before tests are running. We ignore these files.
+            files_missing = files_expected - files_observed
+            if "etc/maas/regiond.conf" in files_missing:
+                files_expected.remove("etc/maas/regiond.conf")
+            if "etc/maas/rackd.conf" in files_missing:
+                files_expected.remove("etc/maas/rackd.conf")
+
             self.assertThat(files_observed, Equals(files_expected))
         self.assertThat(fixture.path, Not(PathExists()))
 

=== modified file 'src/metadataserver/models/scriptset.py'
--- src/metadataserver/models/scriptset.py	2017-01-21 00:49:34 +0000
+++ src/metadataserver/models/scriptset.py	2017-01-25 16:51:52 +0000
@@ -16,6 +16,7 @@
 )
 from maasserver.models.cleansave import CleanSave
 from maasserver.preseed import CURTIN_INSTALL_LOG
+from metadataserver import DefaultMeta
 from metadataserver.enum import (
     RESULT_TYPE,
     RESULT_TYPE_CHOICES,
@@ -91,6 +92,9 @@
 
 class ScriptSet(CleanSave, Model):
 
+    class Meta(DefaultMeta):
+        """Needed for South to recognize this model."""
+
     objects = ScriptSetManager()
 
     last_ping = DateTimeField(blank=True, null=True)

=== modified file 'src/provisioningserver/dhcp/__init__.py'
--- src/provisioningserver/dhcp/__init__.py	2016-04-12 22:22:23 +0000
+++ src/provisioningserver/dhcp/__init__.py	2017-01-25 16:51:52 +0000
@@ -13,7 +13,7 @@
     abstractproperty,
 )
 
-from provisioningserver.path import get_path
+from provisioningserver.path import get_tentative_path
 
 # Location of the DHCPv4 configuration file.
 DHCPv4_CONFIG_FILE = '/var/lib/maas/dhcpd.conf'
@@ -65,8 +65,8 @@
 
     descriptive_name = "DHCPv4"
     template_basename = 'dhcpd.conf.template'
-    interfaces_filename = get_path(DHCPv4_INTERFACES_FILE)
-    config_filename = get_path(DHCPv4_CONFIG_FILE)
+    interfaces_filename = get_tentative_path(DHCPv4_INTERFACES_FILE)
+    config_filename = get_tentative_path(DHCPv4_CONFIG_FILE)
     dhcp_service = "dhcpd"
     ipv6 = False
 
@@ -79,7 +79,7 @@
 
     descriptive_name = "DHCPv6"
     template_basename = 'dhcpd6.conf.template'
-    interfaces_filename = get_path(DHCPv6_INTERFACES_FILE)
-    config_filename = get_path(DHCPv6_CONFIG_FILE)
+    interfaces_filename = get_tentative_path(DHCPv6_INTERFACES_FILE)
+    config_filename = get_tentative_path(DHCPv6_CONFIG_FILE)
     dhcp_service = "dhcpd6"
     ipv6 = True

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2017-01-24 09:24:36 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2017-01-25 16:51:52 +0000
@@ -1229,6 +1229,7 @@
     #     self.assertTrue(client.isSecure())
 
     def test_authenticateRegion_accepts_matching_digests(self):
+        set_shared_secret_on_filesystem(factory.make_bytes())
         client = self.make_running_client()
 
         def calculate_digest(_, message):
@@ -1243,6 +1244,7 @@
         self.assertTrue(extract_result(d))
 
     def test_authenticateRegion_rejects_non_matching_digests(self):
+        set_shared_secret_on_filesystem(factory.make_bytes())
         client = self.make_running_client()
 
         def calculate_digest(_, message):

=== modified file 'src/provisioningserver/tests/test_config.py'
--- src/provisioningserver/tests/test_config.py	2016-08-11 01:50:21 +0000
+++ src/provisioningserver/tests/test_config.py	2017-01-25 16:51:52 +0000
@@ -19,6 +19,7 @@
 from fixtures import EnvironmentVariableFixture
 import formencode
 import formencode.validators
+import maastesting
 from maastesting.factory import factory
 from maastesting.fixtures import ImportErrorFixture
 from maastesting.matchers import (
@@ -638,8 +639,9 @@
         self.assertEqual({"tftp_port": example_port}, config.store)
 
     def test_default_tftp_root(self):
-        maas_root = os.getenv("MAAS_ROOT")
-        self.assertIsNotNone(maas_root)
+        # The default tftp_root is calculated relative to MAAS_ROOT at module
+        # import time, so we need to recreate that value.
+        maas_root = os.path.join(maastesting.root, "run")
         config = ClusterConfiguration({})
         self.assertEqual(
             os.path.join(maas_root, "var/lib/maas/boot-resources/current"),

=== modified file 'src/provisioningserver/utils/fs.py'
--- src/provisioningserver/utils/fs.py	2016-12-14 08:43:09 +0000
+++ src/provisioningserver/utils/fs.py	2017-01-25 16:51:52 +0000
@@ -45,6 +45,7 @@
 import threading
 from time import sleep
 
+from provisioningserver.path import get_path
 from provisioningserver.utils import sudo
 from provisioningserver.utils.shell import ExternalProcessError
 from provisioningserver.utils.twisted import retries
@@ -485,7 +486,7 @@
 
 
 class RunLock(SystemLock):
-    """Always create a lock file at ``/run/lock/maas@${path,modified}``.
+    """Lock file at ``${MAAS_ROOT}/run/lock/maas@${path,modified}``.
 
     This implements an advisory file lock, by proxy, on the given file-system
     path. This is especially useful if you do not have permissions to the
@@ -498,12 +499,12 @@
     def __init__(self, path):
         abspath = FilePath(path).asTextMode().path.lstrip("/")
         discriminator = abspath.replace(":", "::").replace("/", ":")
-        lockpath = "/run/lock/maas@%s" % discriminator
+        lockpath = get_path("run", "lock", "maas@%s" % discriminator)
         super(RunLock, self).__init__(lockpath)
 
 
 class NamedLock(SystemLock):
-    """Always create a lock file at ``/run/lock/maas.${name}``.
+    """Lock file at ``${MAAS_ROOT}/run/lock/maas.${name}``.
 
     This implements an advisory lock, by proxy, for an abstract name. The name
     must be a string and can contain only numerical digits, hyphens, and ASCII
@@ -524,5 +525,5 @@
                 "Lock name contains illegal characters: %s"
                 % "".join(sorted(illegal)))
         else:
-            lockpath = "/run/lock/maas:%s" % name
+            lockpath = get_path("run", "lock", "maas:%s" % name)
             super(NamedLock, self).__init__(lockpath)

=== modified file 'src/provisioningserver/utils/tests/test_fs.py'
--- src/provisioningserver/utils/tests/test_fs.py	2017-01-06 09:50:17 +0000
+++ src/provisioningserver/utils/tests/test_fs.py	2017-01-25 16:51:52 +0000
@@ -33,6 +33,7 @@
 from maastesting.testcase import MAASTestCase
 from maastesting.utils import age_file
 import provisioningserver.config
+from provisioningserver.path import get_tentative_path
 from provisioningserver.utils.fs import (
     atomic_copy,
     atomic_delete,
@@ -773,13 +774,13 @@
 
     def test__string_path(self):
         filename = '/foo/bar/123:456.txt'
-        expected = '/run/lock/maas@foo:bar:123::456.txt'
+        expected = get_tentative_path('/run/lock/maas@foo:bar:123::456.txt')
         observed = RunLock(filename).path
         self.assertEqual(expected, observed)
 
     def test__byte_path(self):
         filename = b'/foo/bar/123:456.txt'
-        expected = '/run/lock/maas@foo:bar:123::456.txt'
+        expected = get_tentative_path('/run/lock/maas@foo:bar:123::456.txt')
         observed = RunLock(filename).path
         self.assertEqual(expected, observed)
 
@@ -789,7 +790,7 @@
 
     def test__string_name(self):
         name = factory.make_name("lock")
-        expected = '/run/lock/maas:' + name
+        expected = get_tentative_path('/run/lock/maas:' + name)
         observed = NamedLock(name).path
         self.assertEqual(expected, observed)
 

=== modified file 'src/provisioningserver/utils/tests/test_utils.py'
--- src/provisioningserver/utils/tests/test_utils.py	2016-09-22 02:53:33 +0000
+++ src/provisioningserver/utils/tests/test_utils.py	2017-01-25 16:51:52 +0000
@@ -13,7 +13,6 @@
 from unittest.mock import sentinel
 
 from fixtures import EnvironmentVariableFixture
-from maastesting import root
 from maastesting.factory import factory
 from maastesting.testcase import MAASTestCase
 import provisioningserver
@@ -40,20 +39,17 @@
 )
 
 
-def get_branch_dir(*path):
-    """Locate a file or directory relative to ``MAAS_ROOT``.
-
-    This function assumes that ``MAAS_ROOT`` has been set to a ``run``
-    subdirectory of this working-tree's root.
-    """
-    return os.path.abspath(os.path.join(root, "run", *path))
+def get_run_path(*path):
+    """Locate a file or directory relative to ``MAAS_ROOT``."""
+    maas_root = os.environ["MAAS_ROOT"]
+    return os.path.abspath(os.path.join(maas_root, *path))
 
 
 class TestLocateConfig(MAASTestCase):
     """Tests for `locate_config`."""
 
     def test_returns_branch_etc_maas(self):
-        self.assertEqual(get_branch_dir('etc/maas'), locate_config())
+        self.assertEqual(get_run_path('etc/maas'), locate_config())
         self.assertThat(locate_config(), DirExists())
 
     def test_defaults_to_global_etc_maas_if_variable_is_unset(self):
@@ -71,18 +67,18 @@
     def test_locates_config_file(self):
         filename = factory.make_string()
         self.assertEqual(
-            get_branch_dir('etc/maas/', filename),
+            get_run_path('etc/maas/', filename),
             locate_config(filename))
 
     def test_locates_full_path(self):
         path = [factory.make_string() for counter in range(3)]
         self.assertEqual(
-            get_branch_dir('etc/maas/', *path),
+            get_run_path('etc/maas/', *path),
             locate_config(*path))
 
     def test_normalizes_path(self):
         self.assertEqual(
-            get_branch_dir('etc/maas/bar/szot'),
+            get_run_path('etc/maas/bar/szot'),
             locate_config('foo/.././bar///szot'))