Giuseppe Lavagetto has submitted this change and it was merged.
Change subject: Have confctl exit with status code 1 if one action fails.
..
Have confctl exit with status code 1 if one action fails.
Change-Id: Ib389e31beff8ac758848c24a54014c9c5718ced3
---
M conftool/cli/tool.py
M conftool/tests/integration/test_tool.py
2 files changed, 25 insertions(+), 10 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, approved
Volans: Looks good to me, but someone else must approve
BBlack: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/conftool/cli/tool.py b/conftool/cli/tool.py
index 7d850aa..63cad8f 100644
--- a/conftool/cli/tool.py
+++ b/conftool/cli/tool.py
@@ -113,6 +113,9 @@
print(msg)
if not fail:
self.announce()
+return True
+else:
+return False
@staticmethod
def raise_warning():
@@ -282,9 +285,12 @@
_log.critical("Invalid configuration: %s", e)
sys.exit(1)
+exit_status = 0
for unit in args.action:
# TODO: fix base class
-cli.run_action(unit)
+if not cli.run_action(unit):
+exit_status = 1
+sys.exit(exit_status)
if __name__ == '__main__':
diff --git a/conftool/tests/integration/test_tool.py
b/conftool/tests/integration/test_tool.py
index 8557115..90a027a 100644
--- a/conftool/tests/integration/test_tool.py
+++ b/conftool/tests/integration/test_tool.py
@@ -31,7 +31,9 @@
def output_for(self, args):
with captured_output() as (out, err):
-tool.main(cmdline=args)
+with self.assertRaises(SystemExit) as cm:
+tool.main(cmdline=args)
+self.assertEquals(cm.exception_code, 0)
res = out.getvalue().strip()
output = []
if not res:
@@ -53,10 +55,7 @@
def test_get_node(self):
args = self.generate_args('get cp1008')
-with captured_output() as (out, err):
-tool.main(cmdline=args)
-res = out.getvalue().strip()
-output = json.loads(res)
+output = self.output_for(args)[0]
k = output.keys()
k.sort()
self.assertEquals(k, ['cp1008', 'tags'])
@@ -82,7 +81,9 @@
Changing values according to a regexp
"""
args = self.generate_args('set/pooled=yes re:cp105.')
-tool.main(cmdline=args)
+with self.assertRaises(SystemExit) as cm:
+tool.main(cmdline=args)
+self.assertEquals(cm.exception_code, 0)
self.assertEquals(self.irc.emit.called, True)
for hostname in ['cp1052', 'cp1053', 'cp1054', 'cp1055']:
n = node.Node('eqiad', 'cache_text', 'https', hostname)
@@ -94,7 +95,10 @@
Test creation is not possible from confctl
"""
args = self.generate_args('set/pooled=yes cp1059')
-tool.main(cmdline=args)
+with self.assertRaises(SystemExit) as cm:
+tool.main(cmdline=args)
+# In error, we exit with status 1
+self.assertEquals(cm.exception_code, 1)
# This doesn't get announced to irc.
self.assertEquals(self.irc.emit.called, False)
n = node.Node('eqiad', 'cache_text', 'https', 'cp1059')
@@ -114,7 +118,9 @@
# one node, or not if we don't
args = ['select', 'cluster=appservers', 'set/pooled=yes']
tool.ToolCliByLabel.raise_warning = mock.MagicMock()
-tool.main(cmdline=args)
+with self.assertRaises(SystemExit) as cm:
+tool.main(cmdline=args)
+self.assertEquals(cm.exception_code, 0)
self.assertEquals(tool.ToolCliByLabel.raise_warning.call_count,1)
# now let's loop through the responses from conftool get
args = ['select', 'cluster=appservers', 'get']
@@ -125,7 +131,10 @@
self.assertEquals(res[k]['pooled'], 'yes')
tool.ToolCliByLabel.raise_warning.reset_mock()
args = ['select', 'name=mw1018', 'set/pooled=inactive']
-tool.main(cmdline=args)
+with self.assertRaises(SystemExit) as cm:
+tool.main(cmdline=args)
+self.assertEquals(cm.exception_code, 0)
+
tool.ToolCliByLabel.raise_warning.assert_not_called()
out = self.output_for(['select', 'name=mw1018', 'get'])
self.assertEquals(out[0]['mw1018']['pooled'], 'inactive')
--
To view, visit https://gerrit.wikimedia.org/r/311678
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib389e31beff8ac758848c24a54014c9c5718ced3
Gerrit-PatchSet: 3
Gerrit-Project: operations/software/conftool
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Lavagetto
Gerrit-Reviewer: BBlack
Gerrit-Reviewer: Ema
Gerrit-Reviewer: Giuseppe Lavagetto
