From c7041ad87261ae9c41f02f38ba0dbe1bab197f15 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                          |  8 ++++++--
 salt/modules/transactional_update.py            |  4 ++--
 tests/unit/modules/test_chroot.py               | 12 +++++++++---
 tests/unit/modules/test_transactional_update.py | 12 ++++++++++--
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/salt/modules/chroot.py b/salt/modules/chroot.py
index 1e2948607e..fda5b0c63c 100644
--- a/salt/modules/chroot.py
+++ b/salt/modules/chroot.py
@@ -188,8 +188,12 @@ 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):
-            return {"result": False, "comment": "Can't parse container command output"}
+        except ValueError:
+            return {
+                "result": False,
+                "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 a0f3f8e6af..7d3724e0c4 100644
--- a/tests/unit/modules/test_chroot.py
+++ b/tests/unit/modules/test_chroot.py
@@ -133,19 +133,25 @@ 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, "stderr": "Error"}),
+            "cmd.run_chroot": MagicMock(
+                return_value={"retcode": 1, "stdout": "", "stderr": "Error",}
+            ),
         }
         with patch.dict(chroot.__utils__, utils_mock), patch.dict(
             chroot.__salt__, salt_mock
         ):
             self.assertEqual(
                 chroot.call("/chroot", "test.ping"),
-                {"result": False, "comment": "Can't parse container command output"},
+                {
+                    "result": False,
+                    "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 08a704c212..19e477d02f 100644
--- a/tests/unit/modules/test_transactional_update.py
+++ b/tests/unit/modules/test_transactional_update.py
@@ -370,7 +370,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()
@@ -422,7 +426,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.2