Refactor DirectorySandbox to take the Mesos sandbox path
Rather than taking "/path/to/mesos_task_dir/sandbox" as a constructor
argument, the sandbox objects are now passed just "/path/to/mesos_task_dir"
as this simplifies a few operations that need the folder created by
Mesos.
The "sandbox" subfolder is still created with the same name, but this is
now only handled internally. There is no user-visible behaviour change.
diff --git a/src/main/python/apache/aurora/executor/common/sandbox.py b/src/main/python/apache/aurora/executor/common/sandbox.py
index fe1562e..64b7d00 100644
--- a/src/main/python/apache/aurora/executor/common/sandbox.py
+++ b/src/main/python/apache/aurora/executor/common/sandbox.py
@@ -71,34 +71,35 @@
class DefaultSandboxProvider(SandboxProvider):
- SANDBOX_NAME = 'sandbox'
MESOS_DIRECTORY_ENV_VARIABLE = 'MESOS_DIRECTORY'
def from_assigned_task(self, assigned_task, **kwargs):
- sandbox_root = os.path.join(os.environ[self.MESOS_DIRECTORY_ENV_VARIABLE], self.SANDBOX_NAME)
+ mesos_dir = os.environ[self.MESOS_DIRECTORY_ENV_VARIABLE]
container = assigned_task.task.container
if container.docker:
- return DockerDirectorySandbox(sandbox_root, **kwargs)
+ return DockerDirectorySandbox(mesos_dir, **kwargs)
elif container.mesos and container.mesos.image:
return FileSystemImageSandbox(
- sandbox_root,
+ mesos_dir,
user=self._get_sandbox_user(assigned_task),
**kwargs)
else:
- return DirectorySandbox(sandbox_root, user=self._get_sandbox_user(assigned_task), **kwargs)
+ return DirectorySandbox(mesos_dir, user=self._get_sandbox_user(assigned_task), **kwargs)
class DirectorySandbox(SandboxInterface):
""" Basic sandbox implementation using a directory on the filesystem """
- def __init__(self, root, user=getpass.getuser(), **kwargs):
- self._root = root
+ SANDBOX_NAME = 'sandbox'
+
+ def __init__(self, mesos_dir, user=getpass.getuser(), **kwargs):
+ self._mesos_dir = mesos_dir
self._user = user
@property
def root(self):
- return self._root
+ return os.path.join(self._mesos_dir, self.SANDBOX_NAME)
@property
def container_root(self):
@@ -154,26 +155,24 @@
class DockerDirectorySandbox(DirectorySandbox):
""" A sandbox implementation that configures the sandbox correctly for docker containers. """
- def __init__(self, root, **kwargs):
- self._mesos_host_sandbox = os.environ[DefaultSandboxProvider.MESOS_DIRECTORY_ENV_VARIABLE]
-
+ def __init__(self, mesos_dir, **kwargs):
# remove the user value from kwargs if it was set.
kwargs.pop('user', None)
- super(DockerDirectorySandbox, self).__init__(root, user=None, **kwargs)
+ super(DockerDirectorySandbox, self).__init__(mesos_dir, user=None, **kwargs)
def _create_symlinks(self):
# This sets up the container to have a similar directory structure to the host.
- # It takes self._mesos_host_sandbox (e.g. "[exec-root]/runs/RUN1/") and:
+ # It takes self._mesos_dir (e.g. "[exec-root]/runs/RUN1/") and:
# - Sets mesos_host_sandbox_root = "[exec-root]/runs/" (one level up from mesos_host_sandbox)
# - Creates mesos_host_sandbox_root (recursively)
- # - Symlinks _mesos_host_sandbox -> $MESOS_SANDBOX (typically /mnt/mesos/sandbox)
+ # - Symlinks self._mesos_dir -> $MESOS_SANDBOX (typically /mnt/mesos/sandbox)
# $MESOS_SANDBOX is provided in the environment by the Mesos containerizer.
- mesos_host_sandbox_root = os.path.dirname(self._mesos_host_sandbox)
+ mesos_host_sandbox_root = os.path.dirname(self._mesos_dir)
try:
safe_mkdir(mesos_host_sandbox_root)
- os.symlink(os.environ['MESOS_SANDBOX'], self._mesos_host_sandbox)
+ os.symlink(os.environ['MESOS_SANDBOX'], self._mesos_dir)
except (IOError, OSError) as e:
raise self.CreationError('Failed to create the sandbox root: %s' % e)
@@ -195,10 +194,8 @@
# already exists.
_USER_OR_GROUP_NAME_EXISTS = 9
- def __init__(self, root, **kwargs):
- self._task_fs_root = os.path.join(
- os.environ[DefaultSandboxProvider.MESOS_DIRECTORY_ENV_VARIABLE],
- TASK_FILESYSTEM_MOUNT_POINT)
+ def __init__(self, mesos_dir, **kwargs):
+ self._task_fs_root = os.path.join(mesos_dir, TASK_FILESYSTEM_MOUNT_POINT)
self._no_create_user = kwargs.pop('no_create_user', False)
self._mounted_volume_paths = kwargs.pop('mounted_volume_paths', None)
@@ -208,7 +205,7 @@
raise self.Error(
'Failed to initialize FileSystemImageSandbox: no value specified for sandbox_mount_point')
- super(FileSystemImageSandbox, self).__init__(root, **kwargs)
+ super(FileSystemImageSandbox, self).__init__(mesos_dir, **kwargs)
def _verify_group_match_in_taskfs(self, group_id, group_name):
try:
diff --git a/src/test/python/apache/aurora/executor/common/test_sandbox.py b/src/test/python/apache/aurora/executor/common/test_sandbox.py
index 0b4958a..7309b67 100644
--- a/src/test/python/apache/aurora/executor/common/test_sandbox.py
+++ b/src/test/python/apache/aurora/executor/common/test_sandbox.py
@@ -31,6 +31,14 @@
from gen.apache.aurora.api.ttypes import AssignedTask, Container, DockerContainer, TaskConfig
+def test_sandbox_root_path():
+ with temporary_dir() as d:
+ mesos_dir = os.path.join(d, 'task1')
+ ds = DirectorySandbox(mesos_dir)
+
+ assert os.path.join(mesos_dir, DirectorySandbox.SANDBOX_NAME) == ds.root
+
+
def test_directory_sandbox():
with temporary_dir() as d:
ds1 = DirectorySandbox(os.path.join(d, 'task1'))
@@ -55,16 +63,16 @@
getpwnam.return_value.pw_gid = 123
getpwnam.return_value.pw_uid = 456
- with temporary_dir() as d:
- real_path = os.path.join(d, 'sandbox')
- ds = DirectorySandbox(real_path, 'cletus')
+ with temporary_dir() as mesos_dir:
+ ds = DirectorySandbox(mesos_dir, 'cletus')
ds.create()
- assert os.path.exists(real_path)
+ assert os.path.exists(ds.root)
getpwnam.assert_called_with('cletus')
getgrgid.assert_called_with(123)
- chown.assert_called_with(real_path, 456, 123)
- chmod.assert_called_with(real_path, 0700)
+
+ chown.assert_called_with(ds.root, 456, 123)
+ chmod.assert_called_with(ds.root, 0700)
@mock.patch('grp.getgrgid')
@@ -73,10 +81,9 @@
@mock.patch('os.chmod')
def test_create_no_user(*args):
with temporary_dir() as d:
- real_path = os.path.join(d, 'sandbox')
- ds = DirectorySandbox(real_path)
+ ds = DirectorySandbox(d)
ds.create()
- assert os.path.exists(real_path)
+ assert os.path.exists(ds.root)
for mocked in args:
mocked.assert_not_called()
@@ -95,8 +102,7 @@
getpwnam.side_effect = KeyError('johndoe')
with temporary_dir() as d:
- real_path = os.path.join(d, 'sandbox')
- ds = DirectorySandbox(real_path, 'cletus')
+ ds = DirectorySandbox(d, 'cletus')
with pytest.raises(DirectorySandbox.CreationError):
ds.create()
@@ -108,8 +114,7 @@
chown.side_effect = IOError('Disk is borked')
with temporary_dir() as d:
- real_path = os.path.join(d, 'sandbox')
- ds = DirectorySandbox(real_path)
+ ds = DirectorySandbox(d)
with pytest.raises(DirectorySandbox.CreationError):
ds.create()
@@ -123,16 +128,14 @@
'MESOS_SANDBOX': 'some-sandbox'
}):
with temporary_dir() as d:
- real_path = os.path.join(d, 'sandbox')
- ds = DockerDirectorySandbox(real_path)
+ ds = DockerDirectorySandbox(d)
with pytest.raises(DirectorySandbox.CreationError):
ds.create()
def test_destroy_ioerror():
with temporary_dir() as d:
- real_path = os.path.join(d, 'sandbox')
- ds = DirectorySandbox(real_path)
+ ds = DirectorySandbox(d)
ds.create()
with mock.patch('shutil.rmtree') as shutil_rmtree:
@@ -148,10 +151,7 @@
@mock.patch.dict(os.environ, {'MESOS_DIRECTORY': MOCK_MESOS_DIRECTORY})
def test_verify_group_match(mock_check_output):
with temporary_dir() as d:
- sandbox = FileSystemImageSandbox(
- os.path.join(d, 'sandbox'),
- user='someuser',
- sandbox_mount_point='/some/path')
+ sandbox = FileSystemImageSandbox(d, user='someuser', sandbox_mount_point='/some/path')
mock_check_output.return_value = 'test-group:x:2:'
@@ -184,9 +184,7 @@
os.makedirs(os.path.join(task_fs_path, 'etc'))
with mock.patch.dict(os.environ, {'MESOS_DIRECTORY': d}):
- sandbox = FileSystemImageSandbox(
- os.path.join(d, 'sandbox'),
- sandbox_mount_point='/some/sandbox/path')
+ sandbox = FileSystemImageSandbox(d, sandbox_mount_point='/some/sandbox/path')
sandbox._copy_files()
@@ -332,7 +330,7 @@
sandbox_directory = os.path.join(MOCK_MESOS_DIRECTORY, 'sandbox')
sandbox = FileSystemImageSandbox(
- sandbox_directory,
+ MOCK_MESOS_DIRECTORY,
user='someuser',
no_create_user=True,
mounted_volume_paths=['/some/container/path', '/some/other/container/path'],
@@ -375,10 +373,10 @@
@mock.patch.dict(os.environ, {'MESOS_DIRECTORY': MOCK_MESOS_DIRECTORY})
def test_filesystem_sandbox_no_volumes(mock_safe_mkdir, mock_check_call, mock_isfile):
sandbox_mount_point = '/some/mount/point'
- sandbox_directory = os.path.join(MOCK_MESOS_DIRECTORY, 'sandbox'),
+ sandbox_directory = os.path.join(MOCK_MESOS_DIRECTORY, 'sandbox')
sandbox = FileSystemImageSandbox(
- sandbox_directory,
+ MOCK_MESOS_DIRECTORY,
user='someuser',
no_create_user=True,
mounted_volume_paths=None,
@@ -418,7 +416,7 @@
sandbox_directory = os.path.join(MOCK_MESOS_DIRECTORY, 'sandbox')
sandbox = FileSystemImageSandbox(
- sandbox_directory,
+ MOCK_MESOS_DIRECTORY,
user='someuser',
no_create_user=True,
mounted_volume_paths=['/some/path/to/file', '/some/path/to/directory'],
