From e5459ce1666df53abc90de1e6942bb5276e65df7 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Feb 14 2018 08:53:09 +0000 Subject: Set timeout for pungi run, by default to 1 hour. --- diff --git a/server/odcs/server/config.py b/server/odcs/server/config.py index 82b06aa..e39b4de 100644 --- a/server/odcs/server/config.py +++ b/server/odcs/server/config.py @@ -133,6 +133,11 @@ class Config(object): 'type': str, 'default': "pungi-koji", 'desc': 'Name or full-path to pungi-koji binary.'}, + 'pungi_timeout': { + 'type': int, + 'default': 3600, + 'desc': 'Time in seconds after which the local pungi-koji is ' + 'killed and compose is marked as failed'}, 'pungi_runroot_enabled': { 'type': bool, 'default': False, diff --git a/server/odcs/server/pdc.py b/server/odcs/server/pdc.py index b90cadc..f27cd75 100644 --- a/server/odcs/server/pdc.py +++ b/server/odcs/server/pdc.py @@ -222,7 +222,7 @@ class PDC(object): old_module = module_map[key] if (module['variant_release'] != old_module['variant_release']): raise ModuleLookupError("%s conflicts with %s" % (module['variant_uid'], - old_module['variant_uid'])) + old_module['variant_uid'])) if expand: added_module_list = new_modules diff --git a/server/odcs/server/pungi.py b/server/odcs/server/pungi.py index 7800196..b408147 100644 --- a/server/odcs/server/pungi.py +++ b/server/odcs/server/pungi.py @@ -190,7 +190,7 @@ class Pungi(object): if conf.raw_config_wrapper_conf_path: output_path = os.path.join(topdir, "raw_config.conf") shutil.copy2(conf.raw_config_wrapper_conf_path, - os.path.join(topdir, "pungi.conf")) + os.path.join(topdir, "pungi.conf")) else: output_path = os.path.join(topdir, "pungi.conf") @@ -269,7 +269,8 @@ class Pungi(object): td = tempfile.mkdtemp() self._write_cfgs(td) pungi_cmd = self.get_pungi_cmd(td, conf.target_dir) - odcs.server.utils.execute_cmd(pungi_cmd, cwd=td) + odcs.server.utils.execute_cmd(pungi_cmd, cwd=td, + timeout=conf.pungi_timeout) finally: try: if td is not None: diff --git a/server/odcs/server/utils.py b/server/odcs/server/utils.py index 63c3574..7113447 100644 --- a/server/odcs/server/utils.py +++ b/server/odcs/server/utils.py @@ -23,10 +23,12 @@ import errno import functools import os +import signal import time import subprocess import requests from distutils.spawn import find_executable +from threading import Timer from odcs.server import conf, log @@ -70,7 +72,13 @@ def makedirs(path, mode=0o775): raise -def execute_cmd(args, stdout=None, stderr=None, cwd=None): +def _kill_process_group(proc, args): + log.error("Timeout occured while running: %s", args) + pgrp = os.getpgid(proc.pid) + os.killpg(pgrp, signal.SIGINT) + + +def execute_cmd(args, stdout=None, stderr=None, cwd=None, timeout=None): """ Executes command defined by `args`. If `stdout` or `stderr` is set to Python file object, the stderr/stdout output is redirecter to that file. @@ -81,6 +89,8 @@ def execute_cmd(args, stdout=None, stderr=None, cwd=None): :param stdout: Python file object to redirect the stdout to. :param stderr: Python file object to redirect the stderr to. :param cwd: string defining the current working directory for command. + :param timeout: Timeout in seconds after which the process and all its + children are killed. :raises RuntimeError: Raised when command exits with non-zero exit code. """ out_log_msg = "" @@ -89,9 +99,24 @@ def execute_cmd(args, stdout=None, stderr=None, cwd=None): if stderr: out_log_msg += ", stderr log: %s" % stderr.name + # Execute command and use `os.setsid` in preexec_fn to create new process + # group so we can kill the main process and also children processes in + # case of timeout. log.info("Executing command: %s%s" % (args, out_log_msg)) - proc = subprocess.Popen(args, stdout=stdout, stderr=stderr, cwd=cwd) - proc.communicate() + proc = subprocess.Popen(args, stdout=stdout, stderr=stderr, cwd=cwd, + preexec_fn=os.setsid) + + # Setup timer to kill whole process group if needed. + if timeout: + timeout_timer = Timer(timeout, _kill_process_group, [proc, args]) + + try: + if timeout: + timeout_timer.start() + proc.communicate() + finally: + if timeout: + timeout_timer.cancel() if proc.returncode != 0: err_msg = "Command '%s' returned non-zero value %d%s" % (args, proc.returncode, out_log_msg) diff --git a/server/tests/test_composerthread.py b/server/tests/test_composerthread.py index 257379c..2635dd7 100644 --- a/server/tests/test_composerthread.py +++ b/server/tests/test_composerthread.py @@ -239,7 +239,7 @@ class TestComposerThread(ModelsBaseTest): create_koji_session.return_value = koji_session koji_session.getLastEvent.return_value = {"id": 123} - def mocked_execute_cmd(args, stdout=None, stderr=None, cwd=None): + def mocked_execute_cmd(args, stdout=None, stderr=None, cwd=None, **kwargs): pungi_cfg = open(os.path.join(cwd, "pungi.conf"), "r").read() self.assertTrue(pungi_cfg.find("gather_method = 'nodeps'") != -1) diff --git a/server/tests/test_pungi.py b/server/tests/test_pungi.py index d0ea249..25789f2 100644 --- a/server/tests/test_pungi.py +++ b/server/tests/test_pungi.py @@ -173,7 +173,10 @@ class TestPungi(unittest.TestCase): pungi = Pungi(pungi_cfg) pungi.run(self.compose) - execute_cmd.assert_called_once() + execute_cmd.assert_called_once_with( + ['pungi-koji', AnyStringWith('pungi.conf'), + AnyStringWith('--target-dir'), '--nightly'], + cwd=AnyStringWith('/tmp/'), timeout=3600) @patch("odcs.server.utils.execute_cmd") def test_pungi_run_raw_config(self, execute_cmd): diff --git a/server/tests/test_utils.py b/server/tests/test_utils.py new file mode 100644 index 0000000..9a0d682 --- /dev/null +++ b/server/tests/test_utils.py @@ -0,0 +1,51 @@ +# -*- coding: utf-8 -*- +# +# Copyright (c) 2017 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +import unittest + +from mock import patch +import time + +from odcs.server.utils import execute_cmd + + +class TestUtilsExecuteCmd(unittest.TestCase): + + def setUp(self): + super(TestUtilsExecuteCmd, self).setUp() + + def tearDown(self): + super(TestUtilsExecuteCmd, self).tearDown() + + def test_execute_cmd_timeout_called(self): + start_time = time.time() + with self.assertRaises(RuntimeError): + execute_cmd(["/usr/bin/sleep", "5"], timeout=1) + stop_time = time.time() + + self.assertTrue(stop_time - start_time < 2) + + @patch("odcs.server.utils._kill_process_group") + def test_execute_cmd_timeout_not_called(self, killpg): + execute_cmd(["/usr/bin/true"], timeout=1) + time.sleep(2) + killpg.assert_not_called()