Return-Path: X-Original-To: apmail-aurora-commits-archive@minotaur.apache.org Delivered-To: apmail-aurora-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 1BDAF1858E for ; Thu, 11 Feb 2016 19:24:20 +0000 (UTC) Received: (qmail 50921 invoked by uid 500); 11 Feb 2016 19:24:20 -0000 Delivered-To: apmail-aurora-commits-archive@aurora.apache.org Received: (qmail 50886 invoked by uid 500); 11 Feb 2016 19:24:20 -0000 Mailing-List: contact commits-help@aurora.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@aurora.apache.org Delivered-To: mailing list commits@aurora.apache.org Received: (qmail 50877 invoked by uid 99); 11 Feb 2016 19:24:20 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 11 Feb 2016 19:24:20 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id DD695E0534; Thu, 11 Feb 2016 19:24:19 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jcohen@apache.org To: commits@aurora.apache.org Message-Id: <42599b3cd49a4cda90d973e97a96e74f@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: aurora git commit: Fix the executor to fail tasks quickly in the event of unknown exceptions when creating sandboxes. Date: Thu, 11 Feb 2016 19:24:19 +0000 (UTC) Repository: aurora Updated Branches: refs/heads/master e7862e0e7 -> 0650b8d1f Fix the executor to fail tasks quickly in the event of unknown exceptions when creating sandboxes. Bugs closed: AURORA-1614 Reviewed at https://reviews.apache.org/r/43486/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/0650b8d1 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/0650b8d1 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/0650b8d1 Branch: refs/heads/master Commit: 0650b8d1ffd8da5e4881e7f39b97728f293b499d Parents: e7862e0 Author: Joshua Cohen Authored: Thu Feb 11 13:23:49 2016 -0600 Committer: Joshua Cohen Committed: Thu Feb 11 13:23:49 2016 -0600 ---------------------------------------------------------------------- .../apache/aurora/executor/aurora_executor.py | 3 + .../apache/aurora/executor/common/sandbox.py | 12 ++- .../executor/common/test_directory_sandbox.py | 93 ---------------- .../aurora/executor/common/test_sandbox.py | 108 +++++++++++++++++++ .../aurora/executor/test_thermos_executor.py | 24 ++++- 5 files changed, 142 insertions(+), 98 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/0650b8d1/src/main/python/apache/aurora/executor/aurora_executor.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/executor/aurora_executor.py b/src/main/python/apache/aurora/executor/aurora_executor.py index 96d7a33..dde19a6 100644 --- a/src/main/python/apache/aurora/executor/aurora_executor.py +++ b/src/main/python/apache/aurora/executor/aurora_executor.py @@ -129,6 +129,9 @@ class AuroraExecutor(ExecutorBase, Observable): except self._sandbox.Error as e: self._die(driver, mesos_pb2.TASK_FAILED, 'Failed to initialize sandbox: %s' % e) return + except Exception as e: + self._die(driver, mesos_pb2.TASK_FAILED, 'Unknown exception initializing sandbox: %s' % e) + return self.sandbox_created.set() return True http://git-wip-us.apache.org/repos/asf/aurora/blob/0650b8d1/src/main/python/apache/aurora/executor/common/sandbox.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/executor/common/sandbox.py b/src/main/python/apache/aurora/executor/common/sandbox.py index 4780232..36f1eab 100644 --- a/src/main/python/apache/aurora/executor/common/sandbox.py +++ b/src/main/python/apache/aurora/executor/common/sandbox.py @@ -122,8 +122,11 @@ class DirectorySandbox(SandboxInterface): class DockerDirectorySandbox(DirectorySandbox): """ A sandbox implementation that configures the sandbox correctly for docker. """ + MESOS_DIRECTORY_ENV_VARIABLE = 'MESOS_DIRECTORY' + MESOS_SANDBOX_ENV_VARIABLE = 'MESOS_SANDBOX' + def __init__(self, sandbox_name): - self._mesos_host_sandbox = os.environ['MESOS_DIRECTORY'] + self._mesos_host_sandbox = os.environ[self.MESOS_DIRECTORY_ENV_VARIABLE] self._root = os.path.join(self._mesos_host_sandbox, sandbox_name) super(DockerDirectorySandbox, self).__init__(self._root, user=None) @@ -136,8 +139,11 @@ class DockerDirectorySandbox(DirectorySandbox): # $MESOS_SANDBOX is provided in the environment by the Mesos docker containerizer. mesos_host_sandbox_root = os.path.dirname(self._mesos_host_sandbox) - os.makedirs(mesos_host_sandbox_root) - os.symlink(os.environ['MESOS_SANDBOX'], self._mesos_host_sandbox) + try: + safe_mkdir(mesos_host_sandbox_root) + os.symlink(os.environ[self.MESOS_SANDBOX_ENV_VARIABLE], self._mesos_host_sandbox) + except (IOError, OSError) as e: + raise self.CreationError('Failed to create the sandbox root: %s' % e) def create(self): self._create_symlinks() http://git-wip-us.apache.org/repos/asf/aurora/blob/0650b8d1/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py b/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py deleted file mode 100644 index a80c6ef..0000000 --- a/src/test/python/apache/aurora/executor/common/test_directory_sandbox.py +++ /dev/null @@ -1,93 +0,0 @@ -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -import os - -import mock -import pytest -from twitter.common.contextutil import temporary_dir - -from apache.aurora.executor.common.sandbox import DirectorySandbox - - -def test_directory_sandbox(): - with temporary_dir() as d: - ds1 = DirectorySandbox(os.path.join(d, 'task1')) - ds2 = DirectorySandbox(os.path.join(d, 'task2')) - ds1.create() - ds2.create() - assert os.path.exists(ds1.root) - assert os.path.exists(ds2.root) - ds1.destroy() - assert not os.path.exists(ds1.root) - assert os.path.exists(ds2.root) - ds2.destroy() - assert not os.path.exists(ds2.root) - - -@mock.patch('grp.getgrgid') -@mock.patch('pwd.getpwnam') -@mock.patch('os.chown') -@mock.patch('os.chmod') -def test_create(chmod, chown, getpwnam, getgrgid): - getgrgid.return_value.gr_name = 'foo' - 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') - ds.create() - assert os.path.exists(real_path) - - 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) - - -@mock.patch('pwd.getpwnam') -def test_user_does_not_exist(getpwnam): - getpwnam.side_effect = KeyError('johndoe') - - with temporary_dir() as d: - real_path = os.path.join(d, 'sandbox') - ds = DirectorySandbox(real_path, 'cletus') - with pytest.raises(DirectorySandbox.CreationError): - ds.create() - - getpwnam.assert_called_with('cletus') - - -@mock.patch('os.chown') -def test_create_ioerror(chown): - chown.side_effect = IOError('Disk is borked') - - with temporary_dir() as d: - real_path = os.path.join(d, 'sandbox') - ds = DirectorySandbox(real_path) - 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.create() - - with mock.patch('shutil.rmtree') as shutil_rmtree: - shutil_rmtree.side_effect = IOError('What even are you doing?') - with pytest.raises(DirectorySandbox.DeletionError): - ds.destroy() http://git-wip-us.apache.org/repos/asf/aurora/blob/0650b8d1/src/test/python/apache/aurora/executor/common/test_sandbox.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/executor/common/test_sandbox.py b/src/test/python/apache/aurora/executor/common/test_sandbox.py new file mode 100644 index 0000000..bd402fc --- /dev/null +++ b/src/test/python/apache/aurora/executor/common/test_sandbox.py @@ -0,0 +1,108 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import os + +import mock +import pytest +from twitter.common.contextutil import temporary_dir + +from apache.aurora.executor.common.sandbox import DirectorySandbox, DockerDirectorySandbox + + +def test_directory_sandbox(): + with temporary_dir() as d: + ds1 = DirectorySandbox(os.path.join(d, 'task1')) + ds2 = DirectorySandbox(os.path.join(d, 'task2')) + ds1.create() + ds2.create() + assert os.path.exists(ds1.root) + assert os.path.exists(ds2.root) + ds1.destroy() + assert not os.path.exists(ds1.root) + assert os.path.exists(ds2.root) + ds2.destroy() + assert not os.path.exists(ds2.root) + + +@mock.patch('grp.getgrgid') +@mock.patch('pwd.getpwnam') +@mock.patch('os.chown') +@mock.patch('os.chmod') +def test_create(chmod, chown, getpwnam, getgrgid): + getgrgid.return_value.gr_name = 'foo' + 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') + ds.create() + assert os.path.exists(real_path) + + 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) + + +@mock.patch('pwd.getpwnam') +def test_user_does_not_exist(getpwnam): + getpwnam.side_effect = KeyError('johndoe') + + with temporary_dir() as d: + real_path = os.path.join(d, 'sandbox') + ds = DirectorySandbox(real_path, 'cletus') + with pytest.raises(DirectorySandbox.CreationError): + ds.create() + + getpwnam.assert_called_with('cletus') + + +@mock.patch('os.chown') +def test_create_ioerror(chown): + chown.side_effect = IOError('Disk is borked') + + with temporary_dir() as d: + real_path = os.path.join(d, 'sandbox') + ds = DirectorySandbox(real_path) + with pytest.raises(DirectorySandbox.CreationError): + ds.create() + + +@mock.patch('os.makedirs') +def test_docker_sandbox_create_ioerror(makedirs): + makedirs.side_effect = IOError('Disk is borked') + + with mock.patch.dict('os.environ', { + DockerDirectorySandbox.MESOS_DIRECTORY_ENV_VARIABLE: 'some-directory', + DockerDirectorySandbox.MESOS_SANDBOX_ENV_VARIABLE: 'some-sandbox' + }): + with temporary_dir() as d: + real_path = os.path.join(d, 'sandbox') + ds = DockerDirectorySandbox(real_path) + 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.create() + + with mock.patch('shutil.rmtree') as shutil_rmtree: + shutil_rmtree.side_effect = IOError('What even are you doing?') + with pytest.raises(DirectorySandbox.DeletionError): + ds.destroy() http://git-wip-us.apache.org/repos/asf/aurora/blob/0650b8d1/src/test/python/apache/aurora/executor/test_thermos_executor.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/executor/test_thermos_executor.py b/src/test/python/apache/aurora/executor/test_thermos_executor.py index ef60ec2..06601df 100644 --- a/src/test/python/apache/aurora/executor/test_thermos_executor.py +++ b/src/test/python/apache/aurora/executor/test_thermos_executor.py @@ -84,13 +84,20 @@ class FailingStartingTaskRunner(ThermosTaskRunner): class FailingSandbox(DirectorySandbox): + def __init__(self, root, exception_type): + self._exception_type = exception_type + super(FailingSandbox, self).__init__(root) + def create(self): - raise self.CreationError('Could not create directory!') + raise self._exception_type('Could not create directory!') class FailingSandboxProvider(SandboxProvider): + def __init__(self, exception_type=DirectorySandbox.CreationError): + self._exception_type = exception_type + def from_assigned_task(self, assigned_task): - return FailingSandbox(safe_mkdtemp()) + return FailingSandbox(safe_mkdtemp(), exception_type=self._exception_type) class SlowSandbox(DirectorySandbox): @@ -478,6 +485,19 @@ class TestThermosExecutor(object): updates = proxy_driver.method_calls['sendStatusUpdate'] assert updates[-1][0][0].state == mesos_pb2.TASK_FAILED + def test_failing_runner_initialize_unknown_exception(self): + proxy_driver = ProxyDriver() + + with temporary_dir() as td: + te = FastThermosExecutor( + runner_provider=make_provider(td), + sandbox_provider=FailingSandboxProvider(exception_type=Exception)) + te.launchTask(proxy_driver, make_task(HELLO_WORLD_MTI)) + proxy_driver.wait_stopped() + + updates = proxy_driver.method_calls['sendStatusUpdate'] + assert updates[-1][0][0].state == mesos_pb2.TASK_FAILED + def test_slow_runner_initialize(self): proxy_driver = ProxyDriver()