From 9db7b6fa8e384164bc99b774ba76ad463ba8d5db Mon Sep 17 00:00:00 2001
From: Alberto Planas <aplanas@suse.com>
Date: Wed, 4 Nov 2020 16:34:47 +0100
Subject: [PATCH] transactional_update: unify with chroot.call

Return for both .call() "retcode" when fail
---
 salt/modules/chroot.py                          |  5 +++--
 salt/modules/transactional_update.py            |  4 ++--
 tests/unit/modules/test_chroot.py               |  6 ++++--
 tests/unit/modules/test_transactional_update.py | 12 ++++++++++--
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/salt/modules/chroot.py b/salt/modules/chroot.py
index 5e890b5c35..fbec3ea788 100644
--- a/salt/modules/chroot.py
+++ b/salt/modules/chroot.py
@@ -192,10 +192,11 @@ def call(root, function, *args, **kwargs):
             if isinstance(local, dict) and 'retcode' in local:
                 __context__['retcode'] = local['retcode']
             return local.get('return', data)
-        except (KeyError, ValueError):
+        except ValueError:
             return {
                 'result': False,
-                'comment': "Can't parse container command output"
+                'retcode': ret['retcode'],
+                'comment': {'stdout': ret['stdout'], 'stderr': ret['stderr']},
             }
     finally:
         __utils__['files.rm_rf'](thin_dest_path)
diff --git a/salt/modules/transactional_update.py b/salt/modules/transactional_update.py
index 9b14557e07..7bbdb697b8 100644
--- a/salt/modules/transactional_update.py
+++ b/salt/modules/transactional_update.py
@@ -988,8 +988,8 @@ def call(function, *args, **kwargs):
             if isinstance(local, dict) and "retcode" in local:
                 __context__["retcode"] = local["retcode"]
             return local.get("return", data)
-        except (KeyError, ValueError):
-            return {"result": False, "comment": ret_stdout}
+        except ValueError:
+            return {"result": False, "retcode": 1, "comment": ret_stdout}
     finally:
         __utils__["files.rm_rf"](thin_dest_path)
 
diff --git a/tests/unit/modules/test_chroot.py b/tests/unit/modules/test_chroot.py
index 045d56c5b0..eb7b8cb4aa 100644
--- a/tests/unit/modules/test_chroot.py
+++ b/tests/unit/modules/test_chroot.py
@@ -145,13 +145,14 @@ class ChrootTestCase(TestCase, LoaderModuleMockMixin):
         utils_mock = {
             'thin.gen_thin': MagicMock(return_value='/salt-thin.tgz'),
             'files.rm_rf': MagicMock(),
-            'json.find_json': MagicMock(return_value={'return': {}})
+            'json.find_json': MagicMock(side_effect=ValueError())
         }
         salt_mock = {
             'cmd.run': MagicMock(return_value=''),
             'config.option': MagicMock(),
             'cmd.run_chroot': MagicMock(return_value={
                 'retcode': 1,
+                'stdout': '',
                 'stderr': 'Error',
             }),
         }
@@ -159,7 +160,8 @@ class ChrootTestCase(TestCase, LoaderModuleMockMixin):
                 patch.dict(chroot.__salt__, salt_mock):
             self.assertEqual(chroot.call('/chroot', 'test.ping'), {
                 'result': False,
-                'comment': "Can't parse container command output"
+                'retcode': 1,
+                'comment': {'stdout': '', 'stderr': 'Error'},
             })
             utils_mock['thin.gen_thin'].assert_called_once()
             salt_mock['config.option'].assert_called()
diff --git a/tests/unit/modules/test_transactional_update.py b/tests/unit/modules/test_transactional_update.py
index b42734a53d..4616e0968f 100644
--- a/tests/unit/modules/test_transactional_update.py
+++ b/tests/unit/modules/test_transactional_update.py
@@ -372,7 +372,11 @@ class TransactionalUpdateTestCase(TestCase, LoaderModuleMockMixin):
         with patch.dict(tu.__utils__, utils_mock), patch.dict(
             tu.__opts__, opts_mock
         ), patch.dict(tu.__salt__, salt_mock):
-            assert tu.call("test.ping") == {"result": False, "comment": "Error"}
+            assert tu.call("test.ping") == {
+                "result": False,
+                "retcode": 1,
+                "comment": "Error",
+            }
 
             utils_mock["thin.gen_thin"].assert_called_once()
             salt_mock["config.option"].assert_called()
@@ -424,7 +428,11 @@ class TransactionalUpdateTestCase(TestCase, LoaderModuleMockMixin):
         with patch.dict(tu.__utils__, utils_mock), patch.dict(
             tu.__opts__, opts_mock
         ), patch.dict(tu.__salt__, salt_mock):
-            assert tu.call("test.ping") == {"result": False, "comment": "Not found"}
+            assert tu.call("test.ping") == {
+                "result": False,
+                "retcode": 1,
+                "comment": "Not found",
+            }
 
             utils_mock["thin.gen_thin"].assert_called_once()
             salt_mock["config.option"].assert_called()
-- 
2.29.1


