From 14dbb3c47d698222f7616ddda4afac78e988024b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Jan 2018 13:41:05 +0100 Subject: [PATCH 01/19] library: make Cmd.create() independent of AnsibleUtil Instead, pass the provider parameter. --- library/network_connections.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index efb7b57..3a64b56 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -2187,13 +2187,12 @@ AnsibleUtil = _AnsibleUtil() class Cmd: @staticmethod - def create(): - provider = AnsibleUtil.params['provider'] + def create(provider): if provider == 'nm': return Cmd_nm() elif provider == 'initscripts': return Cmd_initscripts() - AnsibleUtil.fail_json('unsupported provider %s' % (provider)) + raise MyError('unsupported provider %s' % (provider)) def run(self): for idx, connection in enumerate(AnsibleUtil.connections): @@ -2601,8 +2600,8 @@ class Cmd_initscripts(Cmd): if __name__ == '__main__': try: - Cmd.create().run() + Cmd.create(AnsibleUtil.params['provider']).run() except Exception as e: AnsibleUtil.fail_json('fatal error: %s' % (e), - warn_traceback = True) + warn_traceback = not isinstance(e, MyError)) AnsibleUtil.exit_json() From e26449af8872765a3984fc334cb9a53c65658b3a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Jan 2018 13:45:26 +0100 Subject: [PATCH 02/19] library: introduce a RunEnvironment to make the code independent from ansible parts --- library/network_connections.py | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 3a64b56..3f6e3a3 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1960,7 +1960,10 @@ class NMUtil: ############################################################################### -class _AnsibleUtil: +class RunEnvironment: + pass + +class _AnsibleUtil(RunEnvironment): ARGS = { 'ignore_errors': { 'required': False, 'default': False, 'type': 'str' }, @@ -2186,12 +2189,15 @@ AnsibleUtil = _AnsibleUtil() class Cmd: + def __init__(self, run_env): + self._run_env = run_env + @staticmethod - def create(provider): + def create(provider, **kwargs): if provider == 'nm': - return Cmd_nm() + return Cmd_nm(**kwargs) elif provider == 'initscripts': - return Cmd_initscripts() + return Cmd_initscripts(**kwargs) raise MyError('unsupported provider %s' % (provider)) def run(self): @@ -2261,7 +2267,8 @@ class Cmd: class Cmd_nm(Cmd): - def __init__(self): + def __init__(self, **kwargs): + Cmd.__init__(self, **kwargs) self._nmutil = None self.validate_one_type = ArgValidator_ListConnections.VALIDATE_ONE_MODE_NM @@ -2453,7 +2460,8 @@ class Cmd_nm(Cmd): class Cmd_initscripts(Cmd): - def __init__(self): + def __init__(self, **kwargs): + Cmd.__init__(self, **kwargs) self.validate_one_type = ArgValidator_ListConnections.VALIDATE_ONE_MODE_INITSCRIPTS def check_name(self, idx, name = None): @@ -2599,9 +2607,12 @@ class Cmd_initscripts(Cmd): ############################################################################### if __name__ == '__main__': + ansible_util = AnsibleUtil try: - Cmd.create(AnsibleUtil.params['provider']).run() + cmd = Cmd.create(ansible_util.params['provider'], + run_env = ansible_util) + cmd.run() except Exception as e: - AnsibleUtil.fail_json('fatal error: %s' % (e), - warn_traceback = not isinstance(e, MyError)) - AnsibleUtil.exit_json() + ansible_util.fail_json('fatal error: %s' % (e), + warn_traceback = not isinstance(e, MyError)) + ansible_util.exit_json() From adff2af90bd5b4aa251a23de9b4b1ecaa2993479 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Jan 2018 13:53:04 +0100 Subject: [PATCH 03/19] library: handle check-mode in Cmd class instead of AnsibleUtil It is really related to how the Cmd class operates during run. It should not be handled by AnsibleUtil. --- library/network_connections.py | 78 +++++++++++++++++----------------- 1 file changed, 40 insertions(+), 38 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 3f6e3a3..e238496 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1979,31 +1979,8 @@ class _AnsibleUtil(RunEnvironment): self._connections = None self._run_results = None self._run_results_prepare = None - self._check_mode = CheckMode.PREPARE self._log_idx = 0 - @property - def check_mode(self): - return self._check_mode - - def check_mode_next(self): - if self._check_mode == CheckMode.PREPARE: - self._run_results_prepare = self._run_results - self._run_results = None - if self.module.check_mode: - self._check_mode = CheckMode.DRY_RUN - else: - self._check_mode = CheckMode.PRE_RUN - return self._check_mode - if self.check_mode == CheckMode.PRE_RUN: - self._run_results = None - self._check_mode = CheckMode.REAL_RUN - return CheckMode.REAL_RUN - if self._check_mode != CheckMode.DONE: - self._check_mode = CheckMode.DONE - return CheckMode.DONE - assert False - @property def module(self): module = self._module @@ -2189,8 +2166,10 @@ AnsibleUtil = _AnsibleUtil() class Cmd: - def __init__(self, run_env): + def __init__(self, run_env, is_check_mode = False): self._run_env = run_env + self._is_check_mode = is_check_mode + self._check_mode = CheckMode.PREPARE @staticmethod def create(provider, **kwargs): @@ -2200,6 +2179,28 @@ class Cmd: return Cmd_initscripts(**kwargs) raise MyError('unsupported provider %s' % (provider)) + @property + def check_mode(self): + return self._check_mode + + def check_mode_next(self): + if self._check_mode == CheckMode.PREPARE: + AnsibleUtil._run_results_prepare = AnsibleUtil._run_results + AnsibleUtil._run_results = None + if self._is_check_mode: + self._check_mode = CheckMode.DRY_RUN + else: + self._check_mode = CheckMode.PRE_RUN + return self._check_mode + if self.check_mode == CheckMode.PRE_RUN: + AnsibleUtil._run_results = None + self._check_mode = CheckMode.REAL_RUN + return CheckMode.REAL_RUN + if self._check_mode != CheckMode.DONE: + self._check_mode = CheckMode.DONE + return CheckMode.DONE + assert False + def run(self): for idx, connection in enumerate(AnsibleUtil.connections): try: @@ -2209,7 +2210,7 @@ class Cmd: except ValidationError as e: AnsibleUtil.log_fatal(idx, str(e)) self.run_prepare() - while AnsibleUtil.check_mode_next() != CheckMode.DONE: + while self.check_mode_next() != CheckMode.DONE: for idx, connection in enumerate(AnsibleUtil.connections): try: state = connection['state'] @@ -2218,7 +2219,7 @@ class Cmd: if w is None: w = 10 AnsibleUtil.log_info(idx, 'wait for %s seconds' % (w)) - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: import time time.sleep(w) elif state == 'absent': @@ -2326,7 +2327,7 @@ class Cmd_nm(Cmd): seen.add(c) AnsibleUtil.run_results_changed(idx) AnsibleUtil.log_info(idx, 'delete connection %s, %s' % (c.get_id(), c.get_uuid())) - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_delete(c) except MyError as e: @@ -2343,14 +2344,14 @@ class Cmd_nm(Cmd): AnsibleUtil.log_info(idx, 'add connection %s, %s' % (connection['name'], connection['nm.uuid'])) changed = True try: - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: con_cur = self.nmutil.connection_add(con_new) except MyError as e: AnsibleUtil.log_error(idx, 'adding connection failed: %s' % (e)) elif not self.nmutil.connection_compare(con_cur, con_new, normalize_a = True): changed = True AnsibleUtil.log_info(idx, 'update connection %s, %s' % (con_cur.get_id(), con_cur.get_uuid())) - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_update(con_cur, con_new) except MyError as e: @@ -2369,7 +2370,7 @@ class Cmd_nm(Cmd): c = connections[-1] AnsibleUtil.log_info(idx, 'delete duplicate connection %s, %s' % (c.get_id(), c.get_uuid())) changed = True - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_delete(c) except MyError as e: @@ -2383,7 +2384,7 @@ class Cmd_nm(Cmd): con = Util.first(self.nmutil.connection_list(name = connection['name'], uuid = connection['nm.uuid'])) if not con: - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: AnsibleUtil.log_error(idx, 'up connection %s, %s failed: no connection' % (connection['name'], connection['nm.uuid'])) else: AnsibleUtil.log_info(idx, 'up connection %s, %s' % (connection['name'], connection['nm.uuid'])) @@ -2403,7 +2404,7 @@ class Cmd_nm(Cmd): 'not-active' if not is_active else \ 'is-modified' if is_modified else \ 'force-state-change')) - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: try: ac = self.nmutil.connection_activate (con) except MyError as e: @@ -2434,7 +2435,7 @@ class Cmd_nm(Cmd): changed = True seen.add(ac) AnsibleUtil.log_info(idx, 'down connection %s: %s' % (connection['name'], ac.get_path())) - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.active_connection_deactivate(ac) except MyError as e: @@ -2500,7 +2501,7 @@ class Cmd_initscripts(Cmd): continue changed = True AnsibleUtil.log_info(idx, 'delete ifcfg-rh file "%s"' % (path)) - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: try: os.unlink(path) except Exception as e: @@ -2533,7 +2534,7 @@ class Cmd_initscripts(Cmd): AnsibleUtil.log_info(idx, '%s ifcfg-rh profile "%s"' % (op, name)) - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: try: IfcfgUtil.content_to_file(name, new_content) except MyError as e: @@ -2555,7 +2556,7 @@ class Cmd_initscripts(Cmd): path = IfcfgUtil.ifcfg_path(name) if not os.path.isfile(path): - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: AnsibleUtil.log_error(idx, 'ifcfg file "%s" does not exist' % (path)) else: AnsibleUtil.log_info(idx, 'ifcfg file "%s" does not exist in check mode' % (path)) @@ -2589,7 +2590,7 @@ class Cmd_initscripts(Cmd): 'force-state-change')) cmd = 'ifdown' - if AnsibleUtil.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.REAL_RUN: rc, out, err = AnsibleUtil.module.run_command([cmd, name], encoding=None) AnsibleUtil.log_info(idx, 'call `%s %s`: rc=%d, out="%s", err="%s"' % (cmd, name, rc, out, err)) if rc != 0: @@ -2610,7 +2611,8 @@ if __name__ == '__main__': ansible_util = AnsibleUtil try: cmd = Cmd.create(ansible_util.params['provider'], - run_env = ansible_util) + run_env = ansible_util, + is_check_mode = ansible_util.module.check_mode) cmd.run() except Exception as e: ansible_util.fail_json('fatal error: %s' % (e), From e053286d9c5a735d148e85b982156e0fe709aff3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Jan 2018 14:08:54 +0100 Subject: [PATCH 04/19] library: inject ArgsValidator instance instead of using instance in AnsibleUtil --- library/network_connections.py | 14 +++++++------- library/test_network_connections.py | 14 ++++++++------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index e238496..d0c3671 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1972,8 +1972,6 @@ class _AnsibleUtil(RunEnvironment): 'connections': { 'required': False, 'default': None, 'type': 'list' }, } - ARGS_CONNECTIONS = ArgValidator_ListConnections() - def __init__(self): self._module = None self._connections = None @@ -2020,7 +2018,7 @@ class _AnsibleUtil(RunEnvironment): c = self._connections if c is None: try: - c = self.ARGS_CONNECTIONS.validate(self.params['connections']) + c = ArgValidator_ListConnections().validate(self.params['connections']) except ValidationError as e: self.fail_json('configuration error: %s' % (e), warn_traceback = False) @@ -2166,8 +2164,9 @@ AnsibleUtil = _AnsibleUtil() class Cmd: - def __init__(self, run_env, is_check_mode = False): + def __init__(self, run_env, connection_validator, is_check_mode = False): self._run_env = run_env + self._connection_validator = connection_validator self._is_check_mode = is_check_mode self._check_mode = CheckMode.PREPARE @@ -2204,9 +2203,9 @@ class Cmd: def run(self): for idx, connection in enumerate(AnsibleUtil.connections): try: - AnsibleUtil.ARGS_CONNECTIONS.validate_connection_one(self.validate_one_type, - AnsibleUtil.connections, - idx) + self._connection_validator.validate_connection_one(self.validate_one_type, + AnsibleUtil.connections, + idx) except ValidationError as e: AnsibleUtil.log_fatal(idx, str(e)) self.run_prepare() @@ -2612,6 +2611,7 @@ if __name__ == '__main__': try: cmd = Cmd.create(ansible_util.params['provider'], run_env = ansible_util, + connection_validator = ArgValidator_ListConnections(), is_check_mode = ansible_util.module.check_mode) cmd.run() except Exception as e: diff --git a/library/test_network_connections.py b/library/test_network_connections.py index 0ee5376..61e71d0 100755 --- a/library/test_network_connections.py +++ b/library/test_network_connections.py @@ -40,6 +40,8 @@ def pprint(msg, obj): if nmutil is not None and isinstance(obj, NM.Connection): obj.dump() +ARGS_CONNECTIONS = n.ArgValidator_ListConnections() + class TestValidator(unittest.TestCase): def assertValidationError(self, v, value): @@ -62,12 +64,12 @@ class TestValidator(unittest.TestCase): self.assertEqual(route_list_exp, route_list_new) def do_connections_check_invalid(self, input_connections): - self.assertValidationError(n.AnsibleUtil.ARGS_CONNECTIONS, input_connections) + self.assertValidationError(ARGS_CONNECTIONS, input_connections) def do_connections_validate_nm(self, input_connections, **kwargs): if not nmutil: return - connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) + connections = ARGS_CONNECTIONS.validate(input_connections) for connection in connections: if 'type' in connection: connection['nm.exists'] = False @@ -75,7 +77,7 @@ class TestValidator(unittest.TestCase): mode = n.ArgValidator_ListConnections.VALIDATE_ONE_MODE_INITSCRIPTS for idx, connection in enumerate(connections): try: - n.AnsibleUtil.ARGS_CONNECTIONS.validate_connection_one(mode, connections, idx) + ARGS_CONNECTIONS.validate_connection_one(mode, connections, idx) except n.ValidationError as e: continue if 'type' in connection: @@ -104,10 +106,10 @@ class TestValidator(unittest.TestCase): def do_connections_validate_ifcfg(self, input_connections, **kwargs): mode = n.ArgValidator_ListConnections.VALIDATE_ONE_MODE_INITSCRIPTS - connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) + connections = ARGS_CONNECTIONS.validate(input_connections) for idx, connection in enumerate(connections): try: - n.AnsibleUtil.ARGS_CONNECTIONS.validate_connection_one(mode, connections, idx) + ARGS_CONNECTIONS.validate_connection_one(mode, connections, idx) except n.ValidationError as e: continue if 'type' in connection: @@ -122,7 +124,7 @@ class TestValidator(unittest.TestCase): def do_connections_validate(self, expected_connections, input_connections, **kwargs): - connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) + connections = ARGS_CONNECTIONS.validate(input_connections) self.assertEqual(expected_connections, connections) self.do_connections_validate_nm(input_connections, **kwargs) self.do_connections_validate_ifcfg(input_connections, **kwargs) From f58fa752af50de9d1ac5475534303fd88da403ca Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 09:34:43 +0100 Subject: [PATCH 05/19] library: move connection_modified_earlier() from AnsibleUtil to Cmd class --- library/network_connections.py | 72 +++++++++++++++++----------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index d0c3671..8ec3b20 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -2025,40 +2025,6 @@ class _AnsibleUtil(RunEnvironment): self._connections = c return c - def connection_modified_earlier(self, idx): - # for index @idx, check if any of the previous profiles [0..idx[ - # modify the connection. - - con = self.connections[idx] - assert(con['state'] in ['up', 'down']) - - # also check, if the current profile is 'up' with a 'type' (which - # possibly modifies the connection as well) - if con['state'] == 'up' \ - and 'type' in con \ - and self.run_results[idx]['changed']: - return True - - for i in reversed(range(idx)): - c = self.connections[i] - if 'name' not in c: - continue - if c['name'] != con['name']: - continue - - c_state = c['state'] - if c_state == 'up' and 'type' not in c: - pass - elif c_state == 'down': - return True - elif c_state == 'absent': - return True - elif c_state in ['present', 'up']: - if self.run_results[i]['changed']: - return True - - return False - @property def run_results(self): c = self._run_results @@ -2178,6 +2144,40 @@ class Cmd: return Cmd_initscripts(**kwargs) raise MyError('unsupported provider %s' % (provider)) + def connection_modified_earlier(self, idx): + # for index @idx, check if any of the previous profiles [0..idx[ + # modify the connection. + + con = AnsibleUtil.connections[idx] + assert(con['state'] in ['up', 'down']) + + # also check, if the current profile is 'up' with a 'type' (which + # possibly modifies the connection as well) + if con['state'] == 'up' \ + and 'type' in con \ + and AnsibleUtil.run_results[idx]['changed']: + return True + + for i in reversed(range(idx)): + c = AnsibleUtil.connections[i] + if 'name' not in c: + continue + if c['name'] != con['name']: + continue + + c_state = c['state'] + if c_state == 'up' and 'type' not in c: + pass + elif c_state == 'down': + return True + elif c_state == 'absent': + return True + elif c_state in ['present', 'up']: + if AnsibleUtil.run_results[i]['changed']: + return True + + return False + @property def check_mode(self): return self._check_mode @@ -2390,7 +2390,7 @@ class Cmd_nm(Cmd): return is_active = self.nmutil.connection_is_active(con) - is_modified = AnsibleUtil.connection_modified_earlier(idx) + is_modified = self.connection_modified_earlier(idx) force_state_change = AnsibleUtil.params_force_state_change(connection, False) if is_active and not force_state_change and not is_modified: @@ -2562,7 +2562,7 @@ class Cmd_initscripts(Cmd): return is_active = IfcfgUtil.connection_seems_active(name) - is_modified = AnsibleUtil.connection_modified_earlier(idx) + is_modified = self.connection_modified_earlier(idx) force_state_change = AnsibleUtil.params_force_state_change(connection, False) if do_up: From 97779c1b502f59e7d2f3993fe5c85fbedfcdf17d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 10:22:38 +0100 Subject: [PATCH 06/19] library: move checking force-state-change from AnsibleUtil to Cmd class --- library/network_connections.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 8ec3b20..78c6126 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -2004,15 +2004,6 @@ class _AnsibleUtil(RunEnvironment): v = default_value return v - def params_force_state_change(self, connection, default_value = None): - v = connection['force_state_change'] - if v is None: - if 'force_state_change' in self.params: - v = Util.boolean(self.params['force_state_change']) - if v is None: - v = default_value - return v - @property def connections(self): c = self._connections @@ -2130,10 +2121,16 @@ AnsibleUtil = _AnsibleUtil() class Cmd: - def __init__(self, run_env, connection_validator, is_check_mode = False): + def __init__(self, + run_env, + connection_validator, + is_check_mode = False, + force_state_change = False): self._run_env = run_env self._connection_validator = connection_validator self._is_check_mode = is_check_mode + self._force_state_change = Util.boolean(force_state_change) + self._check_mode = CheckMode.PREPARE @staticmethod @@ -2144,6 +2141,12 @@ class Cmd: return Cmd_initscripts(**kwargs) raise MyError('unsupported provider %s' % (provider)) + def connection_force_state_change(self, connection): + v = connection['force_state_change'] + if v is not None: + return v + return self._force_state_change + def connection_modified_earlier(self, idx): # for index @idx, check if any of the previous profiles [0..idx[ # modify the connection. @@ -2391,7 +2394,7 @@ class Cmd_nm(Cmd): is_active = self.nmutil.connection_is_active(con) is_modified = self.connection_modified_earlier(idx) - force_state_change = AnsibleUtil.params_force_state_change(connection, False) + force_state_change = self.connection_force_state_change(connection) if is_active and not force_state_change and not is_modified: AnsibleUtil.log_info(idx, 'up connection %s, %s skipped because already active' % @@ -2563,7 +2566,7 @@ class Cmd_initscripts(Cmd): is_active = IfcfgUtil.connection_seems_active(name) is_modified = self.connection_modified_earlier(idx) - force_state_change = AnsibleUtil.params_force_state_change(connection, False) + force_state_change = self.connection_force_state_change(connection) if do_up: if is_active is True and not force_state_change and not is_modified: @@ -2612,7 +2615,8 @@ if __name__ == '__main__': cmd = Cmd.create(ansible_util.params['provider'], run_env = ansible_util, connection_validator = ArgValidator_ListConnections(), - is_check_mode = ansible_util.module.check_mode) + is_check_mode = ansible_util.module.check_mode, + force_state_change = ansible_util.params['force_state_change']) cmd.run() except Exception as e: ansible_util.fail_json('fatal error: %s' % (e), From f3b5fed04a70fe5ae99b1409083cf1afc1e07ff7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 13:26:03 +0100 Subject: [PATCH 07/19] library: move log wrapper functions from AnsibleUtil to Cmd class --- library/network_connections.py | 157 +++++++++++++++++---------------- 1 file changed, 83 insertions(+), 74 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 78c6126..8653eb3 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1961,7 +1961,9 @@ class NMUtil: ############################################################################### class RunEnvironment: - pass + + def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): + raise NotImplementedError() class _AnsibleUtil(RunEnvironment): @@ -2041,21 +2043,6 @@ class _AnsibleUtil(RunEnvironment): self.run_results[idx]['rc'].append((rc, msg)) self.log(idx, LogLevel.INFO, 'command: %s (rc=%s)' % (msg, rc)) - def log_debug(self, idx, msg): - self.log(idx, LogLevel.DEBUG, msg) - - def log_info(self, idx, msg): - self.log(idx, LogLevel.INFO, msg) - - def log_warn(self, idx, msg): - self.log(idx, LogLevel.WARN, msg) - - def log_error(self, idx, msg, warn_traceback = False, force_fail = False): - self.log(idx, LogLevel.ERROR, msg, warn_traceback = warn_traceback, force_fail = force_fail) - - def log_fatal(self, idx, msg, warn_traceback = False): - self.log(idx, LogLevel.ERROR, msg, warn_traceback = warn_traceback, force_fail = True) - def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): self._log_idx += 1 if idx == -1: @@ -2126,13 +2113,35 @@ class Cmd: connection_validator, is_check_mode = False, force_state_change = False): - self._run_env = run_env + self.run_env = run_env self._connection_validator = connection_validator self._is_check_mode = is_check_mode self._force_state_change = Util.boolean(force_state_change) self._check_mode = CheckMode.PREPARE + def log_debug(self, idx, msg): + self.log(idx, LogLevel.DEBUG, msg) + + def log_info(self, idx, msg): + self.log(idx, LogLevel.INFO, msg) + + def log_warn(self, idx, msg): + self.log(idx, LogLevel.WARN, msg) + + def log_error(self, idx, msg, warn_traceback = False, force_fail = False): + self.log(idx, LogLevel.ERROR, msg, warn_traceback = warn_traceback, force_fail = force_fail) + + def log_fatal(self, idx, msg, warn_traceback = False): + self.log(idx, LogLevel.ERROR, msg, warn_traceback = warn_traceback, force_fail = True) + + def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): + self.run_env.log(idx, + severity, + msg, + warn_traceback = warn_traceback, + force_fail = force_fail) + @staticmethod def create(provider, **kwargs): if provider == 'nm': @@ -2210,7 +2219,7 @@ class Cmd: AnsibleUtil.connections, idx) except ValidationError as e: - AnsibleUtil.log_fatal(idx, str(e)) + self.log_fatal(idx, str(e)) self.run_prepare() while self.check_mode_next() != CheckMode.DONE: for idx, connection in enumerate(AnsibleUtil.connections): @@ -2220,7 +2229,7 @@ class Cmd: w = connection['wait'] if w is None: w = 10 - AnsibleUtil.log_info(idx, 'wait for %s seconds' % (w)) + self.log_info(idx, 'wait for %s seconds' % (w)) if self.check_mode == CheckMode.REAL_RUN: import time time.sleep(w) @@ -2237,7 +2246,7 @@ class Cmd: else: assert False except Exception as e: - AnsibleUtil.log_warn(idx, 'failure: %s [[%s]]' % (e, traceback.format_exc())) + self.log_warn(idx, 'failure: %s [[%s]]' % (e, traceback.format_exc())) raise def run_prepare(self): @@ -2254,17 +2263,17 @@ class Cmd: if connection['mac']: li_mac = SysUtil.link_info_find(mac = connection['mac']) if not li_mac: - AnsibleUtil.log_fatal(idx, 'profile specifies mac "%s" but no such interface exists' % (connection['mac'])) + self.log_fatal(idx, 'profile specifies mac "%s" but no such interface exists' % (connection['mac'])) if connection['interface_name']: li_ifname = SysUtil.link_info_find(ifname = connection['interface_name']) if not li_ifname: if connection['type'] == 'ethernet': - AnsibleUtil.log_fatal(idx, 'profile specifies interface_name "%s" but no such interface exists' % (connection['interface_name'])) + self.log_fatal(idx, 'profile specifies interface_name "%s" but no such interface exists' % (connection['interface_name'])) elif connection['type'] == 'infiniband': if connection['infiniband_p_key'] in [None, -1]: - AnsibleUtil.log_fatal(idx, 'profile specifies interface_name "%s" but no such infiniband interface exists' % (connection['interface_name'])) + self.log_fatal(idx, 'profile specifies interface_name "%s" but no such infiniband interface exists' % (connection['interface_name'])) if li_mac and li_ifname and li_mac != li_ifname: - AnsibleUtil.log_fatal(idx, 'profile specifies interface_name "%s" and mac "%s" but no such interface exists' % (connection['interface_name'], connection['mac'])) + self.log_fatal(idx, 'profile specifies interface_name "%s" and mac "%s" but no such interface exists' % (connection['interface_name'], connection['mac'])) ############################################################################### @@ -2328,14 +2337,14 @@ class Cmd_nm(Cmd): c = connections[-1] seen.add(c) AnsibleUtil.run_results_changed(idx) - AnsibleUtil.log_info(idx, 'delete connection %s, %s' % (c.get_id(), c.get_uuid())) + self.log_info(idx, 'delete connection %s, %s' % (c.get_id(), c.get_uuid())) if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_delete(c) except MyError as e: - AnsibleUtil.log_error(idx, 'delete connection failed: %s' % (e)) + self.log_error(idx, 'delete connection failed: %s' % (e)) if not seen: - AnsibleUtil.log_info(idx, 'no connection "%s"' % (name)) + self.log_info(idx, 'no connection "%s"' % (name)) def run_state_present(self, idx): connection = AnsibleUtil.connections[idx] @@ -2343,23 +2352,23 @@ class Cmd_nm(Cmd): con_new = self.nmutil.connection_create(AnsibleUtil.connections, idx, con_cur) changed = False if con_cur is None: - AnsibleUtil.log_info(idx, 'add connection %s, %s' % (connection['name'], connection['nm.uuid'])) + self.log_info(idx, 'add connection %s, %s' % (connection['name'], connection['nm.uuid'])) changed = True try: if self.check_mode == CheckMode.REAL_RUN: con_cur = self.nmutil.connection_add(con_new) except MyError as e: - AnsibleUtil.log_error(idx, 'adding connection failed: %s' % (e)) + self.log_error(idx, 'adding connection failed: %s' % (e)) elif not self.nmutil.connection_compare(con_cur, con_new, normalize_a = True): changed = True - AnsibleUtil.log_info(idx, 'update connection %s, %s' % (con_cur.get_id(), con_cur.get_uuid())) + self.log_info(idx, 'update connection %s, %s' % (con_cur.get_id(), con_cur.get_uuid())) if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_update(con_cur, con_new) except MyError as e: - AnsibleUtil.log_error(idx, 'updating connection failed: %s' % (e)) + self.log_error(idx, 'updating connection failed: %s' % (e)) else: - AnsibleUtil.log_info(idx, 'connection %s, %s already up to date' % (con_cur.get_id(), con_cur.get_uuid())) + self.log_info(idx, 'connection %s, %s already up to date' % (con_cur.get_id(), con_cur.get_uuid())) seen = set() if con_cur is not None: @@ -2370,13 +2379,13 @@ class Cmd_nm(Cmd): if not connections: break c = connections[-1] - AnsibleUtil.log_info(idx, 'delete duplicate connection %s, %s' % (c.get_id(), c.get_uuid())) + self.log_info(idx, 'delete duplicate connection %s, %s' % (c.get_id(), c.get_uuid())) changed = True if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_delete(c) except MyError as e: - AnsibleUtil.log_error(idx, 'delete duplicate connection failed: %s' % (e)) + self.log_error(idx, 'delete duplicate connection failed: %s' % (e)) seen.add(c) AnsibleUtil.run_results_changed(idx, changed) @@ -2387,9 +2396,9 @@ class Cmd_nm(Cmd): con = Util.first(self.nmutil.connection_list(name = connection['name'], uuid = connection['nm.uuid'])) if not con: if self.check_mode == CheckMode.REAL_RUN: - AnsibleUtil.log_error(idx, 'up connection %s, %s failed: no connection' % (connection['name'], connection['nm.uuid'])) + self.log_error(idx, 'up connection %s, %s failed: no connection' % (connection['name'], connection['nm.uuid'])) else: - AnsibleUtil.log_info(idx, 'up connection %s, %s' % (connection['name'], connection['nm.uuid'])) + self.log_info(idx, 'up connection %s, %s' % (connection['name'], connection['nm.uuid'])) return is_active = self.nmutil.connection_is_active(con) @@ -2397,20 +2406,20 @@ class Cmd_nm(Cmd): force_state_change = self.connection_force_state_change(connection) if is_active and not force_state_change and not is_modified: - AnsibleUtil.log_info(idx, 'up connection %s, %s skipped because already active' % - (con.get_id(), con.get_uuid())) + self.log_info(idx, 'up connection %s, %s skipped because already active' % + (con.get_id(), con.get_uuid())) return - AnsibleUtil.log_info(idx, 'up connection %s, %s (%s)' % - (con.get_id(), con.get_uuid(), - 'not-active' if not is_active else \ - 'is-modified' if is_modified else \ - 'force-state-change')) + self.log_info(idx, 'up connection %s, %s (%s)' % + (con.get_id(), con.get_uuid(), + 'not-active' if not is_active else \ + 'is-modified' if is_modified else \ + 'force-state-change')) if self.check_mode == CheckMode.REAL_RUN: try: ac = self.nmutil.connection_activate (con) except MyError as e: - AnsibleUtil.log_error(idx, 'up connection failed: %s' % (e)) + self.log_error(idx, 'up connection failed: %s' % (e)) wait_time = connection['wait'] if wait_time is None: @@ -2419,7 +2428,7 @@ class Cmd_nm(Cmd): try: self.nmutil.connection_activate_wait(ac, wait_time) except MyError as e: - AnsibleUtil.log_error(idx, 'up connection failed while waiting: %s' % (e)) + self.log_error(idx, 'up connection failed while waiting: %s' % (e)) AnsibleUtil.run_results_changed(idx) @@ -2436,12 +2445,12 @@ class Cmd_nm(Cmd): break changed = True seen.add(ac) - AnsibleUtil.log_info(idx, 'down connection %s: %s' % (connection['name'], ac.get_path())) + self.log_info(idx, 'down connection %s: %s' % (connection['name'], ac.get_path())) if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.active_connection_deactivate(ac) except MyError as e: - AnsibleUtil.log_error(idx, 'down connection failed: %s' % (e)) + self.log_error(idx, 'down connection failed: %s' % (e)) wait_time = connection['wait'] if wait_time is None: @@ -2450,12 +2459,12 @@ class Cmd_nm(Cmd): try: self.nmutil.active_connection_deactivate_wait(ac, wait_time) except MyError as e: - AnsibleUtil.log_error(idx, 'down connection failed while waiting: %s' % (e)) + self.log_error(idx, 'down connection failed while waiting: %s' % (e)) cons = self.nmutil.connection_list(name = connection['name']) if not changed: - AnsibleUtil.log_info(idx, 'down connection %s failed: no connection' % (connection['name'])) + self.log_info(idx, 'down connection %s failed: no connection' % (connection['name'])) AnsibleUtil.run_results_changed(idx, changed) @@ -2473,7 +2482,7 @@ class Cmd_initscripts(Cmd): try: f = IfcfgUtil.ifcfg_path(name) except MyError as e: - AnsibleUtil.log_error(idx, 'invalid name %s for connection' % (name)) + self.log_error(idx, 'invalid name %s for connection' % (name)) return None return f @@ -2502,15 +2511,15 @@ class Cmd_initscripts(Cmd): if not os.path.isfile(path): continue changed = True - AnsibleUtil.log_info(idx, 'delete ifcfg-rh file "%s"' % (path)) + self.log_info(idx, 'delete ifcfg-rh file "%s"' % (path)) if self.check_mode == CheckMode.REAL_RUN: try: os.unlink(path) except Exception as e: - AnsibleUtil.log_error(idx, 'delete ifcfg-rh file "%s" failed: %s' % (path, e)) + self.log_error(idx, 'delete ifcfg-rh file "%s" failed: %s' % (path, e)) if not changed: - AnsibleUtil.log_info(idx, 'delete ifcfg-rh files for %s (no files present)' % ('"'+n+'"' if n else '*')) + self.log_info(idx, 'delete ifcfg-rh files for %s (no files present)' % ('"'+n+'"' if n else '*')) AnsibleUtil.run_results_changed(idx, changed) def run_state_present(self, idx): @@ -2523,24 +2532,24 @@ class Cmd_initscripts(Cmd): old_content = IfcfgUtil.content_from_file(name) ifcfg_all = IfcfgUtil.ifcfg_create(AnsibleUtil.connections, idx, - lambda msg: AnsibleUtil.log_warn(idx, msg), + lambda msg: self.log_warn(idx, msg), old_content) new_content = IfcfgUtil.content_from_dict(ifcfg_all) if old_content == new_content: - AnsibleUtil.log_info(idx, 'ifcfg-rh profile "%s" already up to date' % (name)) + self.log_info(idx, 'ifcfg-rh profile "%s" already up to date' % (name)) return op = 'add' if (old_content['ifcfg'] is None) else 'update' - AnsibleUtil.log_info(idx, '%s ifcfg-rh profile "%s"' % (op, name)) + self.log_info(idx, '%s ifcfg-rh profile "%s"' % (op, name)) if self.check_mode == CheckMode.REAL_RUN: try: IfcfgUtil.content_to_file(name, new_content) except MyError as e: - AnsibleUtil.log_error(idx, '%s ifcfg-rh profile "%s" failed: %s' % (op, name, e)) + self.log_error(idx, '%s ifcfg-rh profile "%s" failed: %s' % (op, name, e)) AnsibleUtil.run_results_changed(idx) @@ -2559,9 +2568,9 @@ class Cmd_initscripts(Cmd): path = IfcfgUtil.ifcfg_path(name) if not os.path.isfile(path): if self.check_mode == CheckMode.REAL_RUN: - AnsibleUtil.log_error(idx, 'ifcfg file "%s" does not exist' % (path)) + self.log_error(idx, 'ifcfg file "%s" does not exist' % (path)) else: - AnsibleUtil.log_info(idx, 'ifcfg file "%s" does not exist in check mode' % (path)) + self.log_info(idx, 'ifcfg file "%s" does not exist in check mode' % (path)) return is_active = IfcfgUtil.connection_seems_active(name) @@ -2570,33 +2579,33 @@ class Cmd_initscripts(Cmd): if do_up: if is_active is True and not force_state_change and not is_modified: - AnsibleUtil.log_info(idx, 'up connection %s skipped because already active' % - (name)) + self.log_info(idx, 'up connection %s skipped because already active' % + (name)) return - AnsibleUtil.log_info(idx, 'up connection %s (%s)' % - (name, - 'not-active' if is_active is not True else \ - 'is-modified' if is_modified else \ - 'force-state-change')) + self.log_info(idx, 'up connection %s (%s)' % + (name, + 'not-active' if is_active is not True else \ + 'is-modified' if is_modified else \ + 'force-state-change')) cmd = 'ifup' else: if is_active is False and not force_state_change: - AnsibleUtil.log_info(idx, 'down connection %s skipped because not active' % - (name)) + self.log_info(idx, 'down connection %s skipped because not active' % + (name)) return - AnsibleUtil.log_info(idx, 'up connection %s (%s)' % - (name, - 'active' if is_active is not False else \ - 'force-state-change')) + self.log_info(idx, 'up connection %s (%s)' % + (name, + 'active' if is_active is not False else \ + 'force-state-change')) cmd = 'ifdown' if self.check_mode == CheckMode.REAL_RUN: rc, out, err = AnsibleUtil.module.run_command([cmd, name], encoding=None) - AnsibleUtil.log_info(idx, 'call `%s %s`: rc=%d, out="%s", err="%s"' % (cmd, name, rc, out, err)) + self.log_info(idx, 'call `%s %s`: rc=%d, out="%s", err="%s"' % (cmd, name, rc, out, err)) if rc != 0: - AnsibleUtil.log_error(idx, 'call `%s %s` failed with exit status %d' % (cmd, name, rc)) + self.log_error(idx, 'call `%s %s` failed with exit status %d' % (cmd, name, rc)) AnsibleUtil.run_results_changed(idx) From 14cc53f7cae2a202f1a01ca483e176f77a19e446 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 13:30:36 +0100 Subject: [PATCH 08/19] library: avoid calls to AnsibleUtil.fail_json() but raise exception --- library/network_connections.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 8653eb3..7166931 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -2013,8 +2013,7 @@ class _AnsibleUtil(RunEnvironment): try: c = ArgValidator_ListConnections().validate(self.params['connections']) except ValidationError as e: - self.fail_json('configuration error: %s' % (e), - warn_traceback = False) + raise MyError('configuration error: %s' % (e)) self._connections = c return c @@ -2290,7 +2289,7 @@ class Cmd_nm(Cmd): try: nmclient = Util.NM().Client.new(None) except Exception as e: - AnsibleUtil.fail_json('failure loading libnm library: %s' % (e)) + raise MyError('failure loading libnm library: %s' % (e)) self._nmutil = NMUtil(nmclient) return self._nmutil From 25497bfa63cb8eec2b721ca3c2891431e80bda90 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 13:36:13 +0100 Subject: [PATCH 09/19] library: remove unused function --- library/network_connections.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 7166931..57353bb 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -2037,11 +2037,6 @@ class _AnsibleUtil(RunEnvironment): changed = True self.run_results[idx]['changed'] = bool(changed) - def run_results_rc(self, idx, rc, msg): - assert(idx >= 0 and idx < len(self.run_results) - 1) - self.run_results[idx]['rc'].append((rc, msg)) - self.log(idx, LogLevel.INFO, 'command: %s (rc=%s)' % (msg, rc)) - def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): self._log_idx += 1 if idx == -1: From 080fb56bfe70dcbe925d335efd37901b1366eb3a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 13:43:42 +0100 Subject: [PATCH 10/19] library: don't call run_command() via AnsibleUtil --- library/network_connections.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/library/network_connections.py b/library/network_connections.py index 57353bb..fda0e43 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1965,6 +1965,9 @@ class RunEnvironment: def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): raise NotImplementedError() + def run_command(self, argv, encoding = None): + raise NotImplementedError() + class _AnsibleUtil(RunEnvironment): ARGS = { @@ -1993,6 +1996,9 @@ class _AnsibleUtil(RunEnvironment): self._module = module return module + def run_command(self, argv, encoding = None): + return self.module.run_command(argv, encoding = encoding) + @property def params(self): return self.module.params @@ -2114,6 +2120,9 @@ class Cmd: self._check_mode = CheckMode.PREPARE + def run_command(argv, encoding = None): + return self.run_env.run_command(argv, encoding = encoding) + def log_debug(self, idx, msg): self.log(idx, LogLevel.DEBUG, msg) @@ -2596,7 +2605,7 @@ class Cmd_initscripts(Cmd): cmd = 'ifdown' if self.check_mode == CheckMode.REAL_RUN: - rc, out, err = AnsibleUtil.module.run_command([cmd, name], encoding=None) + rc, out, err = self.run_env.run_command([cmd, name]) self.log_info(idx, 'call `%s %s`: rc=%d, out="%s", err="%s"' % (cmd, name, rc, out, err)) if rc != 0: self.log_error(idx, 'call `%s %s` failed with exit status %d' % (cmd, name, rc)) From 7e08386e7e24f7b5d07b2bf6a402a65eec7ccb68 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 13:57:14 +0100 Subject: [PATCH 11/19] library: handle ignore-errors parameter outside of AnsibleUtil --- library/network_connections.py | 38 +++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index fda0e43..9d88e58 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1962,7 +1962,13 @@ class NMUtil: class RunEnvironment: - def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): + def log(self, + idx, + severity, + msg, + ignore_errors = False, + warn_traceback = False, + force_fail = False): raise NotImplementedError() def run_command(self, argv, encoding = None): @@ -2003,15 +2009,6 @@ class _AnsibleUtil(RunEnvironment): def params(self): return self.module.params - def params_ignore_errors(self, connection, default_value = None): - v = connection['ignore_errors'] - if v is None: - try: - v = Util.boolean(self.params['ignore_errors']) - except: - v = default_value - return v - @property def connections(self): c = self._connections @@ -2043,7 +2040,13 @@ class _AnsibleUtil(RunEnvironment): changed = True self.run_results[idx]['changed'] = bool(changed) - def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): + def log(self, + idx, + severity, + msg, + ignore_errors = False, + warn_traceback = False, + force_fail = False): self._log_idx += 1 if idx == -1: idx = len(self.run_results) - 1 @@ -2052,7 +2055,7 @@ class _AnsibleUtil(RunEnvironment): self.run_results[idx]['log'].append((severity, msg, self._log_idx)) if severity == LogLevel.ERROR: if force_fail \ - or not self.params_ignore_errors(self.connections[idx], False): + or not ignore_errors: self.fail_json('error: %s' % (msg), warn_traceback = warn_traceback) def _complete_kwargs_loglines(self, rr, idx): @@ -2112,10 +2115,12 @@ class Cmd: run_env, connection_validator, is_check_mode = False, + ignore_errors = False, force_state_change = False): self.run_env = run_env self._connection_validator = connection_validator self._is_check_mode = is_check_mode + self._ignore_errors = Util.boolean(ignore_errors) self._force_state_change = Util.boolean(force_state_change) self._check_mode = CheckMode.PREPARE @@ -2139,9 +2144,11 @@ class Cmd: self.log(idx, LogLevel.ERROR, msg, warn_traceback = warn_traceback, force_fail = True) def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): + connection = AnsibleUtil.connections[idx] self.run_env.log(idx, severity, msg, + ignore_errors = self.connection_ignore_errors(connection), warn_traceback = warn_traceback, force_fail = force_fail) @@ -2159,6 +2166,12 @@ class Cmd: return v return self._force_state_change + def connection_ignore_errors(self, connection): + v = connection['ignore_errors'] + if v is not None: + return v + return self._ignore_errors + def connection_modified_earlier(self, idx): # for index @idx, check if any of the previous profiles [0..idx[ # modify the connection. @@ -2628,6 +2641,7 @@ if __name__ == '__main__': run_env = ansible_util, connection_validator = ArgValidator_ListConnections(), is_check_mode = ansible_util.module.check_mode, + ignore_errors = ansible_util.params['ignore_errors'], force_state_change = ansible_util.params['force_state_change']) cmd.run() except Exception as e: From e2c575cb84b3d3c34285a9f9e00774cbc0ec0096 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 14:26:46 +0100 Subject: [PATCH 12/19] library: refactor handling of run_results Make it more independent of AnsibleUtil.connections. Eventually, we want to move parts to Cmd. --- library/network_connections.py | 58 ++++++++++++++++------------------ 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 9d88e58..7c73072 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1986,8 +1986,7 @@ class _AnsibleUtil(RunEnvironment): def __init__(self): self._module = None self._connections = None - self._run_results = None - self._run_results_prepare = None + self._run_results = [] self._log_idx = 0 @property @@ -2020,24 +2019,28 @@ class _AnsibleUtil(RunEnvironment): self._connections = c return c + def run_results_push(self, n_connections = None): + c = [] + if n_connections is None: + n_connections = len(self.run_results) - 1 + for cc in range(0, n_connections + 1): + c.append({ + 'changed': False, + 'log': [], + 'rc': [], + }) + self._run_results.append(c) + + def run_results_reset(self): + n_connections = len(self.run_results) - 1 + del self._run_results[-1] + self.run_results_push(n_connections) + @property def run_results(self): - c = self._run_results - if c is None: - c = [] - for cc in range(0, len(self.connections) + 1): - c.append({ - 'changed': False, - 'log': [], - 'rc': [], - }) - self._run_results = c - return c + return self._run_results[-1] - def run_results_changed(self, idx, changed = None): - assert(idx >= 0 and idx < len(self.run_results) - 1) - if changed is None: - changed = True + def run_results_changed(self, idx, changed = True): self.run_results[idx]['changed'] = bool(changed) def log(self, @@ -2047,11 +2050,8 @@ class _AnsibleUtil(RunEnvironment): ignore_errors = False, warn_traceback = False, force_fail = False): + assert(idx >= -1) self._log_idx += 1 - if idx == -1: - idx = len(self.run_results) - 1 - else: - assert(idx >= 0 and idx < len(self.run_results) - 1) self.run_results[idx]['log'].append((severity, msg, self._log_idx)) if severity == LogLevel.ERROR: if force_fail \ @@ -2076,11 +2076,8 @@ class _AnsibleUtil(RunEnvironment): logs = [] l = [] - if self._run_results_prepare is not None: - for idx, rr in enumerate(self._run_results_prepare): - l.extend(self._complete_kwargs_loglines(rr, idx)) - if self._run_results is not None: - for idx, rr in enumerate(self._run_results): + for res in self._run_results: + for idx, rr in enumerate(res): l.extend(self._complete_kwargs_loglines(rr, idx)) l.sort(key = lambda x: x[0]) logs.extend([x[1] for x in l]) @@ -2091,10 +2088,11 @@ class _AnsibleUtil(RunEnvironment): def exit_json(self, **kwargs): changed = False - if self._run_results is not None: + if self._run_results: for rr in self.run_results: if rr['changed']: changed = True + break kwargs['changed'] = changed self.module.exit_json(**self._complete_kwargs(kwargs)) @@ -2212,15 +2210,14 @@ class Cmd: def check_mode_next(self): if self._check_mode == CheckMode.PREPARE: - AnsibleUtil._run_results_prepare = AnsibleUtil._run_results - AnsibleUtil._run_results = None + AnsibleUtil.run_results_push() if self._is_check_mode: self._check_mode = CheckMode.DRY_RUN else: self._check_mode = CheckMode.PRE_RUN return self._check_mode if self.check_mode == CheckMode.PRE_RUN: - AnsibleUtil._run_results = None + AnsibleUtil.run_results_reset() self._check_mode = CheckMode.REAL_RUN return CheckMode.REAL_RUN if self._check_mode != CheckMode.DONE: @@ -2229,6 +2226,7 @@ class Cmd: assert False def run(self): + AnsibleUtil.run_results_push(len(AnsibleUtil.connections)) for idx, connection in enumerate(AnsibleUtil.connections): try: self._connection_validator.validate_connection_one(self.validate_one_type, From c9bbb69d7e1bf1f35742d0776380b8a5ce3c9b8f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 15:05:49 +0100 Subject: [PATCH 13/19] library: track connections in Cmd instead of AnsibleUtil For now this looks more complicated then before. It will get better... --- library/network_connections.py | 101 ++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 46 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 7c73072..ea3c6ee 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1963,6 +1963,7 @@ class NMUtil: class RunEnvironment: def log(self, + connections, idx, severity, msg, @@ -1985,7 +1986,6 @@ class _AnsibleUtil(RunEnvironment): def __init__(self): self._module = None - self._connections = None self._run_results = [] self._log_idx = 0 @@ -2008,17 +2008,6 @@ class _AnsibleUtil(RunEnvironment): def params(self): return self.module.params - @property - def connections(self): - c = self._connections - if c is None: - try: - c = ArgValidator_ListConnections().validate(self.params['connections']) - except ValidationError as e: - raise MyError('configuration error: %s' % (e)) - self._connections = c - return c - def run_results_push(self, n_connections = None): c = [] if n_connections is None: @@ -2044,6 +2033,7 @@ class _AnsibleUtil(RunEnvironment): self.run_results[idx]['changed'] = bool(changed) def log(self, + connections, idx, severity, msg, @@ -2056,20 +2046,20 @@ class _AnsibleUtil(RunEnvironment): if severity == LogLevel.ERROR: if force_fail \ or not ignore_errors: - self.fail_json('error: %s' % (msg), warn_traceback = warn_traceback) + self.fail_json(connections, 'error: %s' % (msg), warn_traceback = warn_traceback) - def _complete_kwargs_loglines(self, rr, idx): - if idx == len(self.connections): + def _complete_kwargs_loglines(self, rr, connections, idx): + if idx == len(connections): prefix = '#' else: - c = self.connections[idx] + c = connections[idx] prefix = '#%s, state:%s' % (idx, c['state']) if c['state'] != 'wait': prefix = prefix + (', "%s"' % (c['name'])) for r in rr['log']: yield (r[2], '[%03d] %s %s: %s' % (r[2], LogLevel.fmt(r[0]), prefix, r[1])) - def _complete_kwargs(self, kwargs, traceback_msg = None): + def _complete_kwargs(self, connections, kwargs, traceback_msg = None): if 'warnings' in kwargs: logs = list(kwargs['warnings']) else: @@ -2078,7 +2068,7 @@ class _AnsibleUtil(RunEnvironment): l = [] for res in self._run_results: for idx, rr in enumerate(res): - l.extend(self._complete_kwargs_loglines(rr, idx)) + l.extend(self._complete_kwargs_loglines(rr, connections, idx)) l.sort(key = lambda x: x[0]) logs.extend([x[1] for x in l]) if traceback_msg is not None: @@ -2086,7 +2076,7 @@ class _AnsibleUtil(RunEnvironment): kwargs['warnings'] = logs return kwargs - def exit_json(self, **kwargs): + def exit_json(self, connections, **kwargs): changed = False if self._run_results: for rr in self.run_results: @@ -2094,14 +2084,14 @@ class _AnsibleUtil(RunEnvironment): changed = True break kwargs['changed'] = changed - self.module.exit_json(**self._complete_kwargs(kwargs)) + self.module.exit_json(**self._complete_kwargs(connections, kwargs)) - def fail_json(self, msg, warn_traceback = False, **kwargs): + def fail_json(self, connections, msg, warn_traceback = False, **kwargs): traceback_msg = None if warn_traceback: traceback_msg = 'exception: %s' % (traceback.format_exc()) kwargs['msg'] = msg - self.module.fail_json(**self._complete_kwargs(kwargs, traceback_msg)) + self.module.fail_json(**self._complete_kwargs(connections, kwargs, traceback_msg)) AnsibleUtil = _AnsibleUtil() @@ -2111,21 +2101,35 @@ class Cmd: def __init__(self, run_env, + connections_unvalidated, connection_validator, is_check_mode = False, ignore_errors = False, force_state_change = False): self.run_env = run_env + self._connections_unvalidated = connections_unvalidated self._connection_validator = connection_validator self._is_check_mode = is_check_mode self._ignore_errors = Util.boolean(ignore_errors) self._force_state_change = Util.boolean(force_state_change) + self._connections = None self._check_mode = CheckMode.PREPARE def run_command(argv, encoding = None): return self.run_env.run_command(argv, encoding = encoding) + @property + def connections(self): + c = self._connections + if c is None: + try: + c = self._connection_validator.validate(self._connections_unvalidated) + except ValidationError as e: + raise MyError('configuration error: %s' % (e)) + self._connections = c + return c + def log_debug(self, idx, msg): self.log(idx, LogLevel.DEBUG, msg) @@ -2142,8 +2146,9 @@ class Cmd: self.log(idx, LogLevel.ERROR, msg, warn_traceback = warn_traceback, force_fail = True) def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): - connection = AnsibleUtil.connections[idx] - self.run_env.log(idx, + connection = self.connections[idx] + self.run_env.log(connections, + idx, severity, msg, ignore_errors = self.connection_ignore_errors(connection), @@ -2174,7 +2179,7 @@ class Cmd: # for index @idx, check if any of the previous profiles [0..idx[ # modify the connection. - con = AnsibleUtil.connections[idx] + con = self.connections[idx] assert(con['state'] in ['up', 'down']) # also check, if the current profile is 'up' with a 'type' (which @@ -2185,7 +2190,7 @@ class Cmd: return True for i in reversed(range(idx)): - c = AnsibleUtil.connections[i] + c = self.connections[i] if 'name' not in c: continue if c['name'] != con['name']: @@ -2226,17 +2231,17 @@ class Cmd: assert False def run(self): - AnsibleUtil.run_results_push(len(AnsibleUtil.connections)) - for idx, connection in enumerate(AnsibleUtil.connections): + AnsibleUtil.run_results_push(len(self.connections)) + for idx, connection in enumerate(self.connections): try: self._connection_validator.validate_connection_one(self.validate_one_type, - AnsibleUtil.connections, + self.connections, idx) except ValidationError as e: self.log_fatal(idx, str(e)) self.run_prepare() while self.check_mode_next() != CheckMode.DONE: - for idx, connection in enumerate(AnsibleUtil.connections): + for idx, connection in enumerate(self.connections): try: state = connection['state'] if state == 'wait': @@ -2264,7 +2269,7 @@ class Cmd: raise def run_prepare(self): - for idx, connection in enumerate(AnsibleUtil.connections): + for idx, connection in enumerate(self.connections): if 'type' in connection and connection['check_iface_exists']: # when the profile is tied to a certain interface via 'interface_name' or 'mac', # check that such an interface exists. @@ -2311,7 +2316,7 @@ class Cmd_nm(Cmd): def run_prepare(self): Cmd.run_prepare(self) names = {} - for connection in AnsibleUtil.connections: + for connection in self.connections: if connection['state'] not in ['up', 'down', 'present', 'absent']: continue name = connection['name'] @@ -2339,11 +2344,11 @@ class Cmd_nm(Cmd): def run_state_absent(self, idx): changed = False seen = set() - name = AnsibleUtil.connections[idx]['name'] + name = self.connections[idx]['name'] black_list_names = None if not name: name = None - black_list_names = ArgUtil.connection_get_non_absent_names(AnsibleUtil.connections) + black_list_names = ArgUtil.connection_get_non_absent_names(self.connections) while True: connections = self.nmutil.connection_list(name = name, black_list_names = black_list_names, black_list = seen) if not connections: @@ -2361,9 +2366,9 @@ class Cmd_nm(Cmd): self.log_info(idx, 'no connection "%s"' % (name)) def run_state_present(self, idx): - connection = AnsibleUtil.connections[idx] + connection = self.connections[idx] con_cur = Util.first(self.nmutil.connection_list(name = connection['name'], uuid = connection['nm.uuid'])) - con_new = self.nmutil.connection_create(AnsibleUtil.connections, idx, con_cur) + con_new = self.nmutil.connection_create(self.connections, idx, con_cur) changed = False if con_cur is None: self.log_info(idx, 'add connection %s, %s' % (connection['name'], connection['nm.uuid'])) @@ -2405,7 +2410,7 @@ class Cmd_nm(Cmd): AnsibleUtil.run_results_changed(idx, changed) def run_state_up(self, idx): - connection = AnsibleUtil.connections[idx] + connection = self.connections[idx] con = Util.first(self.nmutil.connection_list(name = connection['name'], uuid = connection['nm.uuid'])) if not con: @@ -2447,7 +2452,7 @@ class Cmd_nm(Cmd): AnsibleUtil.run_results_changed(idx) def run_state_down(self, idx): - connection = AnsibleUtil.connections[idx] + connection = self.connections[idx] cons = self.nmutil.connection_list(name = connection['name']) changed = False @@ -2492,7 +2497,7 @@ class Cmd_initscripts(Cmd): def check_name(self, idx, name = None): if name is None: - name = AnsibleUtil.connections[idx]['name'] + name = self.connections[idx]['name'] try: f = IfcfgUtil.ifcfg_path(name) except MyError as e: @@ -2502,11 +2507,11 @@ class Cmd_initscripts(Cmd): def run_state_absent(self, idx): changed = False - n = AnsibleUtil.connections[idx]['name'] + n = self.connections[idx]['name'] name = n if not name: names = [] - black_list_names = ArgUtil.connection_get_non_absent_names(AnsibleUtil.connections) + black_list_names = ArgUtil.connection_get_non_absent_names(self.connections) for f in os.listdir('/etc/sysconfig/network-scripts'): if not f.startswith('ifcfg-'): continue @@ -2540,12 +2545,12 @@ class Cmd_initscripts(Cmd): if not self.check_name(idx): return - connection = AnsibleUtil.connections[idx] + connection = self.connections[idx] name = connection['name'] old_content = IfcfgUtil.content_from_file(name) - ifcfg_all = IfcfgUtil.ifcfg_create(AnsibleUtil.connections, idx, + ifcfg_all = IfcfgUtil.ifcfg_create(self.connections, idx, lambda msg: self.log_warn(idx, msg), old_content) @@ -2571,7 +2576,7 @@ class Cmd_initscripts(Cmd): if not self.check_name(idx): return - connection = AnsibleUtil.connections[idx] + connection = self.connections[idx] name = connection['name'] if connection['wait'] is not None: @@ -2634,15 +2639,19 @@ class Cmd_initscripts(Cmd): if __name__ == '__main__': ansible_util = AnsibleUtil + connections = None try: cmd = Cmd.create(ansible_util.params['provider'], run_env = ansible_util, + connections_unvalidated = ansible_util.params['connections'], connection_validator = ArgValidator_ListConnections(), is_check_mode = ansible_util.module.check_mode, ignore_errors = ansible_util.params['ignore_errors'], force_state_change = ansible_util.params['force_state_change']) + connections = cmd.connections cmd.run() except Exception as e: - ansible_util.fail_json('fatal error: %s' % (e), + ansible_util.fail_json(connections, + 'fatal error: %s' % (e), warn_traceback = not isinstance(e, MyError)) - ansible_util.exit_json() + ansible_util.exit_json(connections) From 4e6ace727eb7bfc11e0ecb92a8ec43dcf5906805 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Jan 2018 16:52:46 +0100 Subject: [PATCH 14/19] library: move change-tracking from AnsibleUtil to Cmd --- library/network_connections.py | 147 ++++++++++++++++++++++----------- 1 file changed, 97 insertions(+), 50 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index ea3c6ee..bf24dc3 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1962,11 +1962,15 @@ class NMUtil: class RunEnvironment: + def __init__(self): + self._check_mode = None + def log(self, connections, idx, severity, msg, + is_changed = False, ignore_errors = False, warn_traceback = False, force_fail = False): @@ -1975,7 +1979,21 @@ class RunEnvironment: def run_command(self, argv, encoding = None): raise NotImplementedError() -class _AnsibleUtil(RunEnvironment): + def _check_mode_changed(self, old_check_mode, new_check_mode, connections): + raise NotImplementedError() + + def check_mode_set(self, check_mode, connections = None): + c = self._check_mode + self._check_mode = check_mode + assert( (c is None and check_mode in [CheckMode.PREPARE]) \ + or (c == CheckMode.PREPARE and check_mode in [CheckMode.PRE_RUN, CheckMode.DRY_RUN]) \ + or (c == CheckMode.PRE_RUN and check_mode in [CheckMode.REAL_RUN]) \ + or (c == CheckMode.REAL_RUN and check_mode in [CheckMode.DONE]) \ + or (c == CheckMode.DRY_RUN and check_mode in [CheckMode.DONE])) + self._check_mode_changed(c, check_mode, connections) + + +class AnsibleUtil(RunEnvironment): ARGS = { 'ignore_errors': { 'required': False, 'default': False, 'type': 'str' }, @@ -1985,6 +2003,7 @@ class _AnsibleUtil(RunEnvironment): } def __init__(self): + RunEnvironment.__init__(self) self._module = None self._run_results = [] self._log_idx = 0 @@ -2004,19 +2023,13 @@ class _AnsibleUtil(RunEnvironment): def run_command(self, argv, encoding = None): return self.module.run_command(argv, encoding = encoding) - @property - def params(self): - return self.module.params - def run_results_push(self, n_connections = None): c = [] if n_connections is None: n_connections = len(self.run_results) - 1 for cc in range(0, n_connections + 1): c.append({ - 'changed': False, 'log': [], - 'rc': [], }) self._run_results.append(c) @@ -2029,14 +2042,20 @@ class _AnsibleUtil(RunEnvironment): def run_results(self): return self._run_results[-1] - def run_results_changed(self, idx, changed = True): - self.run_results[idx]['changed'] = bool(changed) + def _check_mode_changed(self, old_check_mode, new_check_mode, connections): + if old_check_mode is None: + self.run_results_push(len(connections)) + elif old_check_mode == CheckMode.PREPARE: + self.run_results_push() + elif old_check_mode == CheckMode.PRE_RUN: + self.run_results_reset() def log(self, connections, idx, severity, msg, + is_changed = False, ignore_errors = False, warn_traceback = False, force_fail = False): @@ -2046,7 +2065,7 @@ class _AnsibleUtil(RunEnvironment): if severity == LogLevel.ERROR: if force_fail \ or not ignore_errors: - self.fail_json(connections, 'error: %s' % (msg), warn_traceback = warn_traceback) + self.fail_json(connections, 'error: %s' % (msg), changed = is_changed, warn_traceback = warn_traceback) def _complete_kwargs_loglines(self, rr, connections, idx): if idx == len(connections): @@ -2076,25 +2095,18 @@ class _AnsibleUtil(RunEnvironment): kwargs['warnings'] = logs return kwargs - def exit_json(self, connections, **kwargs): - changed = False - if self._run_results: - for rr in self.run_results: - if rr['changed']: - changed = True - break + def exit_json(self, connections, changed = False, **kwargs): kwargs['changed'] = changed self.module.exit_json(**self._complete_kwargs(connections, kwargs)) - def fail_json(self, connections, msg, warn_traceback = False, **kwargs): + def fail_json(self, connections, msg, changed = False, warn_traceback = False, **kwargs): traceback_msg = None if warn_traceback: traceback_msg = 'exception: %s' % (traceback.format_exc()) kwargs['msg'] = msg + kwargs['changed'] = changed self.module.fail_json(**self._complete_kwargs(connections, kwargs, traceback_msg)) -AnsibleUtil = _AnsibleUtil() - ############################################################################### class Cmd: @@ -2114,11 +2126,17 @@ class Cmd: self._force_state_change = Util.boolean(force_state_change) self._connections = None + self._connections_data = None self._check_mode = CheckMode.PREPARE + self._is_changed = False def run_command(argv, encoding = None): return self.run_env.run_command(argv, encoding = encoding) + @property + def is_changed(self): + return self._is_changed + @property def connections(self): c = self._connections @@ -2130,6 +2148,31 @@ class Cmd: self._connections = c return c + @property + def connections_data(self): + c = self._connections_data + if c is None: + assert(self.check_mode in [CheckMode.DRY_RUN, CheckMode.PRE_RUN, CheckMode.REAL_RUN]) + c = [] + for idx in range(0, len(self.connections)): + c.append({ + 'changed': False, + }) + self._connections_data = c + return c + + def connections_data_reset(self): + for c in self.connections_data: + c['changed'] = False + self._is_changed = False + + def connections_data_set_changed(self, idx, changed = True): + if not changed: + return + self.connections_data[idx]['changed'] = changed + if changed: + self._is_changed = True + def log_debug(self, idx, msg): self.log(idx, LogLevel.DEBUG, msg) @@ -2146,12 +2189,12 @@ class Cmd: self.log(idx, LogLevel.ERROR, msg, warn_traceback = warn_traceback, force_fail = True) def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): - connection = self.connections[idx] self.run_env.log(connections, idx, severity, msg, - ignore_errors = self.connection_ignore_errors(connection), + is_changed = self.is_changed, + ignore_errors = self.connection_ignore_errors(connections[idx]), warn_traceback = warn_traceback, force_fail = force_fail) @@ -2186,7 +2229,7 @@ class Cmd: # possibly modifies the connection as well) if con['state'] == 'up' \ and 'type' in con \ - and AnsibleUtil.run_results[idx]['changed']: + and self.connections_data[idx]['changed']: return True for i in reversed(range(idx)): @@ -2204,7 +2247,7 @@ class Cmd: elif c_state == 'absent': return True elif c_state in ['present', 'up']: - if AnsibleUtil.run_results[i]['changed']: + if self.connections_data[idx]['changed']: return True return False @@ -2215,23 +2258,23 @@ class Cmd: def check_mode_next(self): if self._check_mode == CheckMode.PREPARE: - AnsibleUtil.run_results_push() if self._is_check_mode: - self._check_mode = CheckMode.DRY_RUN + c = CheckMode.DRY_RUN else: - self._check_mode = CheckMode.PRE_RUN - return self._check_mode - if self.check_mode == CheckMode.PRE_RUN: - AnsibleUtil.run_results_reset() - self._check_mode = CheckMode.REAL_RUN - return CheckMode.REAL_RUN - if self._check_mode != CheckMode.DONE: - self._check_mode = CheckMode.DONE - return CheckMode.DONE - assert False + c = CheckMode.PRE_RUN + elif self.check_mode == CheckMode.PRE_RUN: + self.connections_data_reset() + c = CheckMode.REAL_RUN + elif self._check_mode != CheckMode.DONE: + c = CheckMode.DONE + else: + assert False + self._check_mode = c + self.run_env.check_mode_set(c) + return c def run(self): - AnsibleUtil.run_results_push(len(self.connections)) + self.run_env.check_mode_set(CheckMode.PREPARE, self.connections) for idx, connection in enumerate(self.connections): try: self._connection_validator.validate_connection_one(self.validate_one_type, @@ -2355,7 +2398,7 @@ class Cmd_nm(Cmd): break c = connections[-1] seen.add(c) - AnsibleUtil.run_results_changed(idx) + self.connections_data_set_changed(idx) self.log_info(idx, 'delete connection %s, %s' % (c.get_id(), c.get_uuid())) if self.check_mode == CheckMode.REAL_RUN: try: @@ -2407,7 +2450,7 @@ class Cmd_nm(Cmd): self.log_error(idx, 'delete duplicate connection failed: %s' % (e)) seen.add(c) - AnsibleUtil.run_results_changed(idx, changed) + self.connections_data_set_changed(idx, changed) def run_state_up(self, idx): connection = self.connections[idx] @@ -2449,7 +2492,7 @@ class Cmd_nm(Cmd): except MyError as e: self.log_error(idx, 'up connection failed while waiting: %s' % (e)) - AnsibleUtil.run_results_changed(idx) + self.connections_data_set_changed(idx) def run_state_down(self, idx): connection = self.connections[idx] @@ -2484,7 +2527,7 @@ class Cmd_nm(Cmd): if not changed: self.log_info(idx, 'down connection %s failed: no connection' % (connection['name'])) - AnsibleUtil.run_results_changed(idx, changed) + self.connections_data_set_changed(idx, changed) ############################################################################### @@ -2539,7 +2582,7 @@ class Cmd_initscripts(Cmd): if not changed: self.log_info(idx, 'delete ifcfg-rh files for %s (no files present)' % ('"'+n+'"' if n else '*')) - AnsibleUtil.run_results_changed(idx, changed) + self.connections_data_set_changed(idx, changed) def run_state_present(self, idx): if not self.check_name(idx): @@ -2570,7 +2613,7 @@ class Cmd_initscripts(Cmd): except MyError as e: self.log_error(idx, '%s ifcfg-rh profile "%s" failed: %s' % (op, name, e)) - AnsibleUtil.run_results_changed(idx) + self.connections_data_set_changed(idx) def _run_state_updown(self, idx, do_up): if not self.check_name(idx): @@ -2626,7 +2669,7 @@ class Cmd_initscripts(Cmd): if rc != 0: self.log_error(idx, 'call `%s %s` failed with exit status %d' % (cmd, name, rc)) - AnsibleUtil.run_results_changed(idx) + self.connections_data_set_changed(idx) def run_state_up(self, idx): @@ -2638,20 +2681,24 @@ class Cmd_initscripts(Cmd): ############################################################################### if __name__ == '__main__': - ansible_util = AnsibleUtil + ansible_util = AnsibleUtil() connections = None + cmd = None try: - cmd = Cmd.create(ansible_util.params['provider'], + params = ansible_util.module.params + cmd = Cmd.create(params['provider'], run_env = ansible_util, - connections_unvalidated = ansible_util.params['connections'], + connections_unvalidated = params['connections'], connection_validator = ArgValidator_ListConnections(), is_check_mode = ansible_util.module.check_mode, - ignore_errors = ansible_util.params['ignore_errors'], - force_state_change = ansible_util.params['force_state_change']) + ignore_errors = params['ignore_errors'], + force_state_change = params['force_state_change']) connections = cmd.connections cmd.run() except Exception as e: ansible_util.fail_json(connections, 'fatal error: %s' % (e), + changed = (cmd is not None and cmd.is_changed), warn_traceback = not isinstance(e, MyError)) - ansible_util.exit_json(connections) + ansible_util.exit_json(connections, + changed = (cmd is not None and cmd.is_changed)) From 8db7f496b26d49729755a10e9773e6af330bf1f4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 19 Jan 2018 09:08:03 +0100 Subject: [PATCH 15/19] library: don't create AnsibleUtil.module lazily Now, that we only create the AnsibleUtil instance when we already know that we run under ansible (not from unit tests), we can avoid initializing the AnsibleModule lazily. --- library/network_connections.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index bf24dc3..82286c4 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -2004,21 +2004,15 @@ class AnsibleUtil(RunEnvironment): def __init__(self): RunEnvironment.__init__(self) - self._module = None self._run_results = [] self._log_idx = 0 - @property - def module(self): - module = self._module - if module is None: - from ansible.module_utils.basic import AnsibleModule - module = AnsibleModule( - argument_spec = self.ARGS, - supports_check_mode = True, - ) - self._module = module - return module + from ansible.module_utils.basic import AnsibleModule + module = AnsibleModule( + argument_spec = self.ARGS, + supports_check_mode = True, + ) + self.module = module def run_command(self, argv, encoding = None): return self.module.run_command(argv, encoding = encoding) @@ -2681,9 +2675,9 @@ class Cmd_initscripts(Cmd): ############################################################################### if __name__ == '__main__': - ansible_util = AnsibleUtil() connections = None cmd = None + ansible_util = AnsibleUtil() try: params = ansible_util.module.params cmd = Cmd.create(params['provider'], From 1ae5196a87d5ec147826a7b03af1b33490bddc3f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 19 Jan 2018 13:11:38 +0100 Subject: [PATCH 16/19] library/trivial: rename AnsibleUtil to RunEnvironmentAnsible --- library/network_connections.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 82286c4..8e9926c 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1993,7 +1993,7 @@ class RunEnvironment: self._check_mode_changed(c, check_mode, connections) -class AnsibleUtil(RunEnvironment): +class RunEnvironmentAnsible(RunEnvironment): ARGS = { 'ignore_errors': { 'required': False, 'default': False, 'type': 'str' }, @@ -2677,22 +2677,22 @@ class Cmd_initscripts(Cmd): if __name__ == '__main__': connections = None cmd = None - ansible_util = AnsibleUtil() + run_env_ansible = RunEnvironmentAnsible() try: - params = ansible_util.module.params + params = run_env_ansible.module.params cmd = Cmd.create(params['provider'], - run_env = ansible_util, + run_env = run_env_ansible, connections_unvalidated = params['connections'], connection_validator = ArgValidator_ListConnections(), - is_check_mode = ansible_util.module.check_mode, + is_check_mode = run_env_ansible.module.check_mode, ignore_errors = params['ignore_errors'], force_state_change = params['force_state_change']) connections = cmd.connections cmd.run() except Exception as e: - ansible_util.fail_json(connections, - 'fatal error: %s' % (e), - changed = (cmd is not None and cmd.is_changed), - warn_traceback = not isinstance(e, MyError)) - ansible_util.exit_json(connections, - changed = (cmd is not None and cmd.is_changed)) + run_env_ansible.fail_json(connections, + 'fatal error: %s' % (e), + changed = (cmd is not None and cmd.is_changed), + warn_traceback = not isinstance(e, MyError)) + run_env_ansible.exit_json(connections, + changed = (cmd is not None and cmd.is_changed)) From a5eb321ef329d1269d0c73e6b8ce157766c48417 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 19 Jan 2018 13:16:56 +0100 Subject: [PATCH 17/19] library: cleanup handling of run_results in RunEnvironmentAnsible --- library/network_connections.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 8e9926c..ed83dfe 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -2017,32 +2017,29 @@ class RunEnvironmentAnsible(RunEnvironment): def run_command(self, argv, encoding = None): return self.module.run_command(argv, encoding = encoding) - def run_results_push(self, n_connections = None): + def _run_results_push(self, n_connections): c = [] - if n_connections is None: - n_connections = len(self.run_results) - 1 for cc in range(0, n_connections + 1): c.append({ 'log': [], }) self._run_results.append(c) - def run_results_reset(self): - n_connections = len(self.run_results) - 1 - del self._run_results[-1] - self.run_results_push(n_connections) - @property def run_results(self): return self._run_results[-1] def _check_mode_changed(self, old_check_mode, new_check_mode, connections): if old_check_mode is None: - self.run_results_push(len(connections)) + self._run_results_push(len(connections)) elif old_check_mode == CheckMode.PREPARE: - self.run_results_push() + self._run_results_push(len(self.run_results) - 1) elif old_check_mode == CheckMode.PRE_RUN: - self.run_results_reset() + # when switching from RRE_RUN to REAL_RUN, we drop the run-results + # we just collected and reset to empty. The PRE_RUN succeeded. + n_connections = len(self.run_results) - 1 + del self._run_results[-1] + self._run_results_push(n_connections) def log(self, connections, From db7fc2b60e121d4ba7804d8968367b740879c51e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 19 Jan 2018 13:53:38 +0100 Subject: [PATCH 18/19] library: move ansible_managed header outside of IfcfgUtil --- library/network_connections.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index ed83dfe..77293d8 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1372,15 +1372,16 @@ class IfcfgUtil: ifcfg[val[0]] = val[1] return ifcfg - ANSIBLE_MANAGED = '# this file was created by ansible' - @classmethod - def content_from_dict(cls, ifcfg_all, file_type = None): + def content_from_dict(cls, ifcfg_all, file_type = None, header = None): content = {} for file_type in cls._file_types(file_type): h = ifcfg_all[file_type] if file_type == 'ifcfg': - s = cls.ANSIBLE_MANAGED + '\n' + if header is not None: + s = header + '\n' + else: + s = "" for key in sorted(h.keys()): value = h[key] if not cls.KeyValid(key): @@ -1965,6 +1966,10 @@ class RunEnvironment: def __init__(self): self._check_mode = None + @property + def ifcfg_header(self): + return None + def log(self, connections, idx, @@ -2014,6 +2019,10 @@ class RunEnvironmentAnsible(RunEnvironment): ) self.module = module + @property + def ifcfg_header(self): + return '# this file was created by ansible' + def run_command(self, argv, encoding = None): return self.module.run_command(argv, encoding = encoding) @@ -2588,7 +2597,8 @@ class Cmd_initscripts(Cmd): lambda msg: self.log_warn(idx, msg), old_content) - new_content = IfcfgUtil.content_from_dict(ifcfg_all) + new_content = IfcfgUtil.content_from_dict(ifcfg_all, + header = self.run_env.ifcfg_header) if old_content == new_content: self.log_info(idx, 'ifcfg-rh profile "%s" already up to date' % (name)) From 068db05080b461d1a3da52025bf1eb0520706ac4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 19 Jan 2018 14:32:11 +0100 Subject: [PATCH 19/19] library: fix handling of "changed" flag run() supports a --check mode (DRY_RUN) and a real mode, where the real mode consists of a PRE_RUN that only simulates the steps and a REAL_RUN. Actualy changes can only happen during REAL_RUN (and we pretend that they happen during DRY_RUN). Fix handling of the change flag, is was broken previously. Also, we need to set the is-changed flag to True before actually invoking the action. Because, if we fail, we might fail_json() right away, and need to correct changed flag. --- library/network_connections.py | 59 +++++++++++++++++----------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 77293d8..e73ea6c 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -2128,14 +2128,14 @@ class Cmd: self._connections = None self._connections_data = None self._check_mode = CheckMode.PREPARE - self._is_changed = False + self._is_changed_modified_system = False def run_command(argv, encoding = None): return self.run_env.run_command(argv, encoding = encoding) @property - def is_changed(self): - return self._is_changed + def is_changed_modified_system(self): + return self._is_changed_modified_system @property def connections(self): @@ -2164,14 +2164,18 @@ class Cmd: def connections_data_reset(self): for c in self.connections_data: c['changed'] = False - self._is_changed = False def connections_data_set_changed(self, idx, changed = True): + assert(self._check_mode in [CheckMode.PRE_RUN, CheckMode.DRY_RUN, CheckMode.REAL_RUN]) if not changed: return self.connections_data[idx]['changed'] = changed - if changed: - self._is_changed = True + if changed \ + and self._check_mode in [CheckMode.DRY_RUN, CheckMode.REAL_RUN]: + # we only do actual modifications during the REAL_RUN step. + # And as a special exception, during the DRY_RUN step, which + # is like REAL_RUN, except not not actually changing anything. + self._is_changed_modified_system = True def log_debug(self, idx, msg): self.log(idx, LogLevel.DEBUG, msg) @@ -2193,7 +2197,7 @@ class Cmd: idx, severity, msg, - is_changed = self.is_changed, + is_changed = self.is_changed_modified_system, ignore_errors = self.connection_ignore_errors(connections[idx]), warn_traceback = warn_traceback, force_fail = force_fail) @@ -2385,7 +2389,6 @@ class Cmd_nm(Cmd): connection['nm.uuid'] = uuid def run_state_absent(self, idx): - changed = False seen = set() name = self.connections[idx]['name'] black_list_names = None @@ -2398,8 +2401,8 @@ class Cmd_nm(Cmd): break c = connections[-1] seen.add(c) - self.connections_data_set_changed(idx) self.log_info(idx, 'delete connection %s, %s' % (c.get_id(), c.get_uuid())) + self.connections_data_set_changed(idx) if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_delete(c) @@ -2412,18 +2415,17 @@ class Cmd_nm(Cmd): connection = self.connections[idx] con_cur = Util.first(self.nmutil.connection_list(name = connection['name'], uuid = connection['nm.uuid'])) con_new = self.nmutil.connection_create(self.connections, idx, con_cur) - changed = False if con_cur is None: self.log_info(idx, 'add connection %s, %s' % (connection['name'], connection['nm.uuid'])) - changed = True - try: - if self.check_mode == CheckMode.REAL_RUN: + self.connections_data_set_changed(idx) + if self.check_mode == CheckMode.REAL_RUN: + try: con_cur = self.nmutil.connection_add(con_new) - except MyError as e: - self.log_error(idx, 'adding connection failed: %s' % (e)) + except MyError as e: + self.log_error(idx, 'adding connection failed: %s' % (e)) elif not self.nmutil.connection_compare(con_cur, con_new, normalize_a = True): - changed = True self.log_info(idx, 'update connection %s, %s' % (con_cur.get_id(), con_cur.get_uuid())) + self.connections_data_set_changed(idx) if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_update(con_cur, con_new) @@ -2442,7 +2444,7 @@ class Cmd_nm(Cmd): break c = connections[-1] self.log_info(idx, 'delete duplicate connection %s, %s' % (c.get_id(), c.get_uuid())) - changed = True + self.connections_data_set_changed(idx) if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_delete(c) @@ -2450,7 +2452,6 @@ class Cmd_nm(Cmd): self.log_error(idx, 'delete duplicate connection failed: %s' % (e)) seen.add(c) - self.connections_data_set_changed(idx, changed) def run_state_up(self, idx): connection = self.connections[idx] @@ -2477,6 +2478,7 @@ class Cmd_nm(Cmd): 'not-active' if not is_active else \ 'is-modified' if is_modified else \ 'force-state-change')) + self.connections_data_set_changed(idx) if self.check_mode == CheckMode.REAL_RUN: try: ac = self.nmutil.connection_activate (con) @@ -2492,8 +2494,6 @@ class Cmd_nm(Cmd): except MyError as e: self.log_error(idx, 'up connection failed while waiting: %s' % (e)) - self.connections_data_set_changed(idx) - def run_state_down(self, idx): connection = self.connections[idx] @@ -2505,9 +2505,10 @@ class Cmd_nm(Cmd): ac = Util.first(self.nmutil.active_connection_list(connections = cons, black_list = seen)) if ac is None: break - changed = True seen.add(ac) self.log_info(idx, 'down connection %s: %s' % (connection['name'], ac.get_path())) + changed = True + self.connections_data_set_changed(idx) if self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.active_connection_deactivate(ac) @@ -2527,7 +2528,6 @@ class Cmd_nm(Cmd): if not changed: self.log_info(idx, 'down connection %s failed: no connection' % (connection['name'])) - self.connections_data_set_changed(idx, changed) ############################################################################### @@ -2549,7 +2549,6 @@ class Cmd_initscripts(Cmd): return f def run_state_absent(self, idx): - changed = False n = self.connections[idx]['name'] name = n if not name: @@ -2568,12 +2567,15 @@ class Cmd_initscripts(Cmd): if not self.check_name(idx): return names = [name] + + changed = False for name in names: for path in IfcfgUtil.ifcfg_paths(name): if not os.path.isfile(path): continue changed = True self.log_info(idx, 'delete ifcfg-rh file "%s"' % (path)) + self.connections_data_set_changed(idx) if self.check_mode == CheckMode.REAL_RUN: try: os.unlink(path) @@ -2582,7 +2584,6 @@ class Cmd_initscripts(Cmd): if not changed: self.log_info(idx, 'delete ifcfg-rh files for %s (no files present)' % ('"'+n+'"' if n else '*')) - self.connections_data_set_changed(idx, changed) def run_state_present(self, idx): if not self.check_name(idx): @@ -2608,14 +2609,13 @@ class Cmd_initscripts(Cmd): self.log_info(idx, '%s ifcfg-rh profile "%s"' % (op, name)) + self.connections_data_set_changed(idx) if self.check_mode == CheckMode.REAL_RUN: try: IfcfgUtil.content_to_file(name, new_content) except MyError as e: self.log_error(idx, '%s ifcfg-rh profile "%s" failed: %s' % (op, name, e)) - self.connections_data_set_changed(idx) - def _run_state_updown(self, idx, do_up): if not self.check_name(idx): return @@ -2664,14 +2664,13 @@ class Cmd_initscripts(Cmd): 'force-state-change')) cmd = 'ifdown' + self.connections_data_set_changed(idx) if self.check_mode == CheckMode.REAL_RUN: rc, out, err = self.run_env.run_command([cmd, name]) self.log_info(idx, 'call `%s %s`: rc=%d, out="%s", err="%s"' % (cmd, name, rc, out, err)) if rc != 0: self.log_error(idx, 'call `%s %s` failed with exit status %d' % (cmd, name, rc)) - self.connections_data_set_changed(idx) - def run_state_up(self, idx): self._run_state_updown(idx, True) @@ -2699,7 +2698,7 @@ if __name__ == '__main__': except Exception as e: run_env_ansible.fail_json(connections, 'fatal error: %s' % (e), - changed = (cmd is not None and cmd.is_changed), + changed = (cmd is not None and cmd.is_changed_modified_system), warn_traceback = not isinstance(e, MyError)) run_env_ansible.exit_json(connections, - changed = (cmd is not None and cmd.is_changed)) + changed = (cmd is not None and cmd.is_changed_modified_system))