From 366aeff44d531acd6720677085fb7c8e8262b259 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Aug 2017 16:23:30 +0200 Subject: [PATCH 1/5] library: remove unused argument @mainloop_iterate from list functions If the caller wants to iterate the mainloop, he shall just do it. --- library/network_connections.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index f6f2c3c..e321ccd 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1209,9 +1209,7 @@ class NMUtil: return True return False - def active_connection_list(self, connections = None, black_list = None, mainloop_iterate = True): - if mainloop_iterate: - Util.GMainLoop_iterate_all() + def active_connection_list(self, connections = None, black_list = None): active_cons = self.nmclient.get_active_connections() if connections: connections = set(connections) @@ -1221,9 +1219,7 @@ class NMUtil: active_cons = list(active_cons) return active_cons; - def connection_list(self, name = None, uuid = None, black_list = None, black_list_names = None, black_list_uuids = None, mainloop_iterate = True): - if mainloop_iterate: - Util.GMainLoop_iterate_all() + def connection_list(self, name = None, uuid = None, black_list = None, black_list_names = None, black_list_uuids = None): cons = self.nmclient.get_connections() if name is not None: cons = [c for c in cons if c.get_id() == name] From e0675f7a313a917b01b56e43784476edf878dd33 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Aug 2017 17:56:24 +0200 Subject: [PATCH 2/5] library: avoid suppressing duplicate logging messages We hack the "warnings" JSON result to return logging messages. However, ansible will suppress duplicate warnings. That is inconvenient. Avoid that by prefixing each logging message with an index. --- library/network_connections.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index e321ccd..7bea062 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1636,6 +1636,7 @@ class _AnsibleUtil: self._run_results = None self._run_results_prepare = None self._check_mode = CheckMode.PREPARE + self._log_idx = 0 @property def check_mode(self): @@ -1726,7 +1727,8 @@ class _AnsibleUtil: 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_results[idx]['log'].append((severity, msg)) + self._log_idx += 1 + self.run_results[idx]['log'].append((severity, msg, self._log_idx)) if severity == LogLevel.ERROR: # ignore_errors can be specified per profile. In absense of a # per-profile setting, a global parameter is consulted. @@ -1748,7 +1750,7 @@ class _AnsibleUtil: if c['state'] != 'wait': prefix = prefix + (', "%s"' % (c['name'])) for r in rr['log']: - yield '%s #%s, %s: %s' % (LogLevel.fmt(r[0]), idx, prefix, r[1]) + yield '[%03d] %s #%s, %s: %s' % (r[2], LogLevel.fmt(r[0]), idx, prefix, r[1]) def _complete_kwargs(self, kwargs, traceback_msg = None): if 'warnings' in kwargs: From 71102f925301b7510777a087c6b85c02de6bf06d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Aug 2017 18:00:12 +0200 Subject: [PATCH 3/5] library: ensure chronological order of logging messages We group logging messages by their connection (@idx). Finally, when constructing the overall result, reorder the messages to ensure that their are still in chronological order. Usually they are anyway, because we handle connections one-by-one, and wouldn't log messages for previous connections. --- library/network_connections.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 7bea062..92850e6 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1750,19 +1750,23 @@ class _AnsibleUtil: if c['state'] != 'wait': prefix = prefix + (', "%s"' % (c['name'])) for r in rr['log']: - yield '[%03d] %s #%s, %s: %s' % (r[2], LogLevel.fmt(r[0]), idx, prefix, r[1]) + yield (r[2], '[%03d] %s #%s, %s: %s' % (r[2], LogLevel.fmt(r[0]), idx, prefix, r[1])) def _complete_kwargs(self, kwargs, traceback_msg = None): if 'warnings' in kwargs: logs = list(kwargs['warnings']) else: logs = [] + + l = [] if self._run_results_prepare is not None: for idx, rr in enumerate(self._run_results_prepare): - logs.extend(self._complete_kwargs_loglines(rr, idx)) + l.extend(self._complete_kwargs_loglines(rr, idx)) if self._run_results is not None: for idx, rr in enumerate(self._run_results): - logs.extend(self._complete_kwargs_loglines(rr, idx)) + l.extend(self._complete_kwargs_loglines(rr, idx)) + l.sort(key = lambda x: x[0]) + logs.extend([x[1] for x in l]) if traceback_msg is not None: logs.append(traceback_msg) kwargs['warnings'] = logs From ab21a417489f98b3da4b3d680d3396e3d5194121 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Aug 2017 18:14:41 +0200 Subject: [PATCH 4/5] library: allow logging of general messages, not tied to a connection Mainly for printf debugging of the module, where we don't have a connection index handy. --- library/network_connections.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 92850e6..68f075a 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1693,7 +1693,7 @@ class _AnsibleUtil: c = self._run_results if c is None: c = [] - for cc in self.connections: + for cc in range(0, len(self.connections) + 1): c.append({ 'changed': False, 'log': [], @@ -1703,11 +1703,13 @@ class _AnsibleUtil: return c def run_results_changed(self, idx, changed = None): + assert(idx >= 0 and idx < len(self.run_results) - 1) if changed is None: 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)) @@ -1728,6 +1730,10 @@ class _AnsibleUtil: def log(self, idx, severity, msg, warn_traceback = False, force_fail = False): 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: # ignore_errors can be specified per profile. In absense of a @@ -1745,12 +1751,15 @@ class _AnsibleUtil: self.fail_json('error: %s' % (msg), warn_traceback = warn_traceback) def _complete_kwargs_loglines(self, rr, idx): - c = self.connections[idx] - prefix = 'state:%s' % (c['state']) - if c['state'] != 'wait': - prefix = prefix + (', "%s"' % (c['name'])) + if idx == len(self.connections): + prefix = '#' + else: + c = self.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: %s' % (r[2], LogLevel.fmt(r[0]), idx, prefix, r[1])) + yield (r[2], '[%03d] %s %s: %s' % (r[2], LogLevel.fmt(r[0]), prefix, r[1])) def _complete_kwargs(self, kwargs, traceback_msg = None): if 'warnings' in kwargs: From 5d09a75fe1b76bade2aac480c25d0cec3b5ab746 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 24 Aug 2017 18:01:15 +0200 Subject: [PATCH 5/5] library: wait for connection to disappear from libnm cache during delete We delete the connection asynchrnonously using libnm. Even after the asynchronous request completes, the connection may still in the cache. That is a bit odd, but maybe not a bug in libnm. Because it can happen that the connection was active, so it takes time to bring it down. Maybe the asynchronous request should not complete in libnm before the connection is truly gone. But note that deactivating a connection can take arbitrary long, so it's not clear that his would be the best behavior. Anyway, work around it by waiting. The effect of this bug is if you have "state: absent" followed by "state:up", then the intermediate "present" state will be skipped. That is, because it appears that the connection still exists, although it's about to be deleted. The subsequent "up" will then fail as the connection is gone in the meantime. https://github.com/linux-system-roles/network/issues/7 https://bugzilla.redhat.com/show_bug.cgi?id=1478910 --- library/network_connections.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/library/network_connections.py b/library/network_connections.py index 68f075a..09ca7ed 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1433,6 +1433,8 @@ class NMUtil: def connection_delete(self, connection, timeout = 10): + c_uuid = connection.get_uuid() + def delete_cb(connection, result, cb_args): success = False try: @@ -1453,6 +1455,22 @@ class NMUtil: if not cb_args.get('success', False): raise MyError('failure to delete connection: %s' % (cb_args.get('error', 'unknown error'))) + # workaround libnm oddity. The connection may not yet be gone if the + # connection was active and is deactivating. Wait. + wait_count = 0 + while True: + connections = self.connection_list(uuid = c_uuid) + if not connections: + return + wait_count += 1 + if wait_count > 10: + break; + import time + time.sleep(1) + Util.GMainLoop_iterate_all() + + raise MyError('connection %s was supposedly deleted successfully, but it\'s still here' % (c_uuid)) + def connection_activate(self, connection, timeout = 15, wait_time = None): already_retried = False;