From a4a53f4b458cc8152690c6ed71a2081d056cf678 Mon Sep 17 00:00:00 2001 From: Julien Fortin Date: Sun, 10 Apr 2016 18:55:56 +0200 Subject: [PATCH] fixing: ifupdown2 (subprocesses) lives on after control-c + Parsing cmd string with shlex.split instead of string.split Ticket: CM-9905 Reviewed By: CCR-4363 Testing Done: ^C ifupdown2 while ifreload-ing interfaces test files (~500ifaces) + smoke tests --- addons/address.py | 2 +- addons/usercmds.py | 6 ++++++ ifupdown/ifupdownbase.py | 14 +++++++++++--- ifupdown/utils.py | 17 +++++++++++++++++ ifupdownaddons/iproute2.py | 10 +++++++++- ifupdownaddons/modulebase.py | 14 ++++++++++++-- ifupdownaddons/utilsbase.py | 17 +++++++++++++++-- 7 files changed, 71 insertions(+), 9 deletions(-) diff --git a/addons/address.py b/addons/address.py index 293d13d..791f1ec 100644 --- a/addons/address.py +++ b/addons/address.py @@ -317,7 +317,7 @@ class address(moduleBase): alias = ifaceobj.get_attr_value_first('alias') if alias: filename = '/sys/class/net/%s/ifalias' %ifaceobj.name - self.logger.info('Executing echo "" > %s' %filename) + self.logger.info('executing echo "" > %s' %filename) os.system('echo "" > %s' %filename) # XXX hwaddress reset cannot happen because we dont know last # address. diff --git a/addons/usercmds.py b/addons/usercmds.py index 72915ea..c0120ba 100644 --- a/addons/usercmds.py +++ b/addons/usercmds.py @@ -6,6 +6,9 @@ import subprocess import ifupdownaddons +import signal + +from ifupdown.utils import utils class usercmds(ifupdownaddons.modulebase.moduleBase): """ ifupdown2 addon module to configure user specified commands """ @@ -42,11 +45,14 @@ class usercmds(ifupdownaddons.modulebase.moduleBase): shell=True, stderr=subprocess.STDOUT, close_fds=True) + utils.enable_subprocess_signal_forwarding(ch, signal.SIGINT) cmd_returncode = ch.wait() cmdout = ch.communicate()[0] except Exception, e: raise Exception('failed to execute cmd \'%s\' (%s)' %(cmd, str(e))) + finally: + utils.disable_subprocess_signal_forwarding(signal.SIGINT) if cmd_returncode != 0: raise Exception(cmdout) return cmdout diff --git a/ifupdown/ifupdownbase.py b/ifupdown/ifupdownbase.py index 2a6b480..72c44a9 100644 --- a/ifupdown/ifupdownbase.py +++ b/ifupdown/ifupdownbase.py @@ -11,8 +11,13 @@ import logging import subprocess import re import os -from iface import * import rtnetlink_api as rtnetlink_api +import signal +import shlex + +from iface import * +from ifupdown.utils import utils + class ifupdownBase(object): @@ -24,19 +29,22 @@ class ifupdownBase(object): cmd_returncode = 0 cmdout = '' try: - self.logger.info('Executing ' + cmd) + self.logger.info('executing ' + cmd) if self.DRYRUN: return cmdout - ch = subprocess.Popen(cmd.split(), + ch = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, shell=False, env=cmdenv, stderr=subprocess.STDOUT, close_fds=True) + utils.enable_subprocess_signal_forwarding(ch, signal.SIGINT) cmdout = ch.communicate()[0] cmd_returncode = ch.wait() except OSError, e: raise Exception('could not execute ' + cmd + '(' + str(e) + ')') + finally: + utils.disable_subprocess_signal_forwarding(signal.SIGINT) if cmd_returncode != 0: raise Exception('error executing cmd \'%s\'' %cmd + '\n(' + cmdout.strip('\n ') + ')') diff --git a/ifupdown/utils.py b/ifupdown/utils.py index 970ee03..458999e 100644 --- a/ifupdown/utils.py +++ b/ifupdown/utils.py @@ -9,6 +9,15 @@ import os import fcntl import re +import signal + +from functools import partial + +def signal_handler_f(ps, sig, frame): + if ps: + ps.send_signal(sig) + if sig == signal.SIGINT: + raise KeyboardInterrupt class utils(): @@ -58,3 +67,11 @@ class utils(): else: return False + @classmethod + def enable_subprocess_signal_forwarding(cls, ps, sig): + signal.signal(sig, partial(signal_handler_f, ps)) + + @classmethod + def disable_subprocess_signal_forwarding(cls, sig): + signal.signal(sig, signal.SIG_DFL) + diff --git a/ifupdownaddons/iproute2.py b/ifupdownaddons/iproute2.py index 39a685a..f814490 100644 --- a/ifupdownaddons/iproute2.py +++ b/ifupdownaddons/iproute2.py @@ -6,6 +6,10 @@ import os import glob +import shlex +import signal + +from ifupdown.utils import utils from collections import OrderedDict from utilsbase import * from systemutils import * @@ -495,9 +499,11 @@ class iproute2(utilsBase): cmd = 'bridge fdb show brport %s' % dev cur_peers = [] try: - ps = subprocess.Popen((cmd).split(), stdout=subprocess.PIPE, close_fds=True) + ps = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, close_fds=True) + utils.enable_subprocess_signal_forwarding(ps, signal.SIGINT) output = subprocess.check_output(('grep', '00:00:00:00:00:00'), stdin=ps.stdout) ps.wait() + utils.disable_subprocess_signal_forwarding(signal.SIGINT) try: ppat = re.compile('\s+dst\s+(\d+.\d+.\d+.\d+)\s+') for l in output.split('\n'): @@ -510,6 +516,8 @@ class iproute2(utilsBase): except subprocess.CalledProcessError as e: if e.returncode != 1: self.logger.error(str(e)) + finally: + utils.disable_subprocess_signal_forwarding(signal.SIGINT) return cur_peers diff --git a/ifupdownaddons/modulebase.py b/ifupdownaddons/modulebase.py index 9b6bd7f..522b4db 100644 --- a/ifupdownaddons/modulebase.py +++ b/ifupdownaddons/modulebase.py @@ -10,6 +10,10 @@ import io import logging import subprocess import traceback +import signal +import shlex + +from ifupdown.utils import utils from ifupdown.iface import * #from ifupdownaddons.iproute2 import * #from ifupdownaddons.dhclient import * @@ -79,16 +83,19 @@ class moduleBase(object): self.logger.info('Executing ' + cmd) if self.DRYRUN: return cmdout - ch = subprocess.Popen(cmd.split(), + ch = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, shell=False, env=cmdenv, stderr=subprocess.STDOUT, close_fds=True) + utils.enable_subprocess_signal_forwarding(ch, signal.SIGINT) cmdout = ch.communicate()[0] cmd_returncode = ch.wait() except OSError, e: raise Exception('could not execute ' + cmd + '(' + str(e) + ')') + finally: + utils.disable_subprocess_signal_forwarding(signal.SIGINT) if cmd_returncode != 0: raise Exception('error executing cmd \'%s\'' %cmd + '(' + cmdout.strip('\n ') + ')') @@ -109,17 +116,20 @@ class moduleBase(object): self.logger.info('Executing %s (stdin=%s)' %(cmd, stdinbuf)) if self.DRYRUN: return cmdout - ch = subprocess.Popen(cmd.split(), + ch = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, stdin=subprocess.PIPE, shell=False, env=cmdenv, stderr=subprocess.STDOUT, close_fds=True) + utils.enable_subprocess_signal_forwarding(ch, signal.SIGINT) cmdout = ch.communicate(input=stdinbuf)[0] cmd_returncode = ch.wait() except OSError, e: raise Exception('could not execute ' + cmd + '(' + str(e) + ')') + finally: + utils.disable_subprocess_signal_forwarding(signal.SIGINT) if cmd_returncode != 0: raise Exception('error executing cmd \'%s (%s)\'' %(cmd, stdinbuf) + '(' + cmdout.strip('\n ') + ')') diff --git a/ifupdownaddons/utilsbase.py b/ifupdownaddons/utilsbase.py index 73fdfd5..666ffc1 100644 --- a/ifupdownaddons/utilsbase.py +++ b/ifupdownaddons/utilsbase.py @@ -8,6 +8,10 @@ import logging import subprocess import re import io +import signal +import shlex + +from ifupdown.utils import utils from ifupdown.iface import * from cache import * @@ -50,11 +54,14 @@ class utilsBase(object): shell=False, env=cmdenv, stderr=subprocess.STDOUT, close_fds=True) + utils.enable_subprocess_signal_forwarding(ch, signal.SIGINT) cmdout = ch.communicate()[0] cmd_returncode = ch.wait() except OSError, e: raise Exception('failed to execute cmd \'%s\' (%s)' %(' '.join(cmdl), str(e))) + finally: + utils.disable_subprocess_signal_forwarding(signal.SIGINT) if cmd_returncode != 0: raise Exception('failed to execute cmd \'%s\'' %' '.join(cmdl) + '(' + cmdout.strip('\n ') + ')') @@ -63,7 +70,7 @@ class utilsBase(object): def exec_command(self, cmd, cmdenv=None): """ Executes command given as string in the argument cmd """ - return self.exec_commandl(cmd.split(), cmdenv) + return self.exec_commandl(shlex.split(cmd), cmdenv) def exec_command_talk_stdin(self, cmd, stdinbuf): """ Executes command and writes to stdin of the process """ @@ -73,17 +80,20 @@ class utilsBase(object): self.logger.info('executing %s [%s]' %(cmd, stdinbuf)) if self.DRYRUN: return cmdout - ch = subprocess.Popen(cmd.split(), + ch = subprocess.Popen(shlex.split(cmd), stdout=subprocess.PIPE, stdin=subprocess.PIPE, shell=False, stderr=subprocess.STDOUT, close_fds=True) + utils.enable_subprocess_signal_forwarding(ch, signal.SIGINT) cmdout = ch.communicate(input=stdinbuf)[0] cmd_returncode = ch.wait() except OSError, e: raise Exception('failed to execute cmd \'%s\' (%s)' %(cmd, str(e))) + finally: + utils.disable_subprocess_signal_forwarding(signal.SIGINT) if cmd_returncode != 0: raise Exception('failed to execute cmd \'%s [%s]\'' %(cmd, stdinbuf) + '(' + cmdout.strip('\n ') + ')') @@ -115,10 +125,13 @@ class utilsBase(object): shell=False, stderr=None, close_fds=True) + utils.enable_subprocess_signal_forwarding(ch, signal.SIGINT) cmd_returncode = ch.wait() except Exception, e: raise Exception('failed to execute cmd \'%s\' (%s)' %(' '.join(cmdl), str(e))) + finally: + utils.disable_subprocess_signal_forwarding(signal.SIGINT) if cmd_returncode != 0: raise Exception('failed to execute cmd \'%s\'' %' '.join(cmdl))