From fceb71128eecf543dfa779d923c4e15d9a28e366 Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Wed, 21 Jun 2017 11:33:42 +0100 Subject: [PATCH] Return code is not very useful to assume a task failed So I thought I fixed it before, but there's still one location where the `rc` value is influential to decide whether a task failed or not. We already established in #24867 that it is up to the module to decide what the return code actually means, not the task executor. We modified the existing modules to move that logic into the module (eg. for command, shell, etc.) This relates to the integration tests of win_robocopy, where different return codes have different meanings: - 0 -- No files copied. - 1 -- Files copied successfully! (changed) - 2 -- Some Extra files or directories were detected. No files were copied. (warning) - 3 -- (2+1) Some files were copied. Additional files were present. (changed) - 4 -- Some mismatched files or directories were detected. Housekeeping might be required! (changed + warning) - 5 -- (4+1) Some files were copied. Some files were mismatched. (changed + warning) - 6 -- (4+2) Additional files and mismatched files exist. No files were copied. (warning) - 7 -- (4+1+2) Files were copied, a file mismatch was present, and additional files were present. (changed + warning) - 8 -- Some files or directories could not be copied! (changed + failed) - 9 - 15 -- Fatal error. Check log message! (failed) - 16 -- Serious Error! No files were copied! Do you have permissions to access $src and $dest? (failed) This also fixes #24652 --- lib/ansible/executor/task_result.py | 2 +- test/units/executor/test_task_result.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ansible/executor/task_result.py b/lib/ansible/executor/task_result.py index 8d0b3ffa34..f62d430bf1 100644 --- a/lib/ansible/executor/task_result.py +++ b/lib/ansible/executor/task_result.py @@ -67,7 +67,7 @@ class TaskResult: 'results' in self._result and True in [True for x in self._result['results'] if 'failed_when_result' in x]: return self._check_key('failed_when_result') else: - return self._check_key('failed') or self._result.get('rc', 0) != 0 + return self._check_key('failed') def is_unreachable(self): return self._check_key('unreachable') diff --git a/test/units/executor/test_task_result.py b/test/units/executor/test_task_result.py index cd31f3794e..74717b73d5 100644 --- a/test/units/executor/test_task_result.py +++ b/test/units/executor/test_task_result.py @@ -125,11 +125,11 @@ class TestTaskResult(unittest.TestCase): tr = TaskResult(mock_host, mock_task, dict()) self.assertFalse(tr.is_failed()) - # test failed result with rc values + # test failed result with rc values (should not matter) tr = TaskResult(mock_host, mock_task, dict(rc=0)) self.assertFalse(tr.is_failed()) tr = TaskResult(mock_host, mock_task, dict(rc=1)) - self.assertTrue(tr.is_failed()) + self.assertFalse(tr.is_failed()) # test with failed in result tr = TaskResult(mock_host, mock_task, dict(failed=True))