From c4439471509a5dd88b6c9f334334a198aa71f7f3 Mon Sep 17 00:00:00 2001 From: Till Maas Date: Thu, 13 Jun 2019 09:03:57 +0200 Subject: [PATCH 1/2] Utils: Add helper to call sync NM methods --- module_utils/network_lsr/utils.py | 37 +++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/module_utils/network_lsr/utils.py b/module_utils/network_lsr/utils.py index 4475358..bd1887d 100644 --- a/module_utils/network_lsr/utils.py +++ b/module_utils/network_lsr/utils.py @@ -121,6 +121,43 @@ class Util: c += 1 return c + @classmethod + def call_async_method(cls, object_, action, args, mainloop_timeout=10): + """ Asynchronously call a NetworkManager method """ + cancellable = cls.create_cancellable() + async_action = action + "_async" + # NM does not use a uniform naming for the async methods, + # for checkpoints it is: + # NMClient.checkpoint_create() and NMClient.checkpoint_create_finish(), + # but for reapply it is: + # NMDevice.reapply_async() and NMDevice.reapply_finish() + # NMDevice.reapply() is a synchronous version + # Therefore check if there is a method if an `async` suffix and use the + # one without the suffix otherwise + if not hasattr(object_, async_action): + async_action = action + finish = action + "_finish" + user_data = {} + + fullargs = [] + fullargs += args + fullargs += (cancellable, cls.create_callback(finish), user_data) + + getattr(object_, async_action)(*fullargs) + + if not cls.GMainLoop_run(mainloop_timeout): + cancellable.cancel() + raise MyError("failure to call %s.%s(): timeout" % object_, async_action) + + success = user_data.get("success", None) + if success is not None: + return success + + raise MyError( + "failure to %s checkpoint: %s: %r" + % (action, user_data.get("error", "unknown error"), user_data) + ) + @classmethod def create_cancellable(cls): return cls.Gio().Cancellable.new() From 225ba70a43473de3fd874bc62982cabb28cbfe35 Mon Sep 17 00:00:00 2001 From: Till Maas Date: Tue, 11 Jun 2019 12:57:40 +0200 Subject: [PATCH 2/2] Modules/NM: Wrap changes in checkpoint Create a checkpoint before changing NetworkManager profiles and rollback on failures (destroy it on success). --- library/network_connections.py | 93 ++++++++++++++++++++++++++++++++-- 1 file changed, 88 insertions(+), 5 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 6b20dad..25a7882 100644 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -39,6 +39,8 @@ options: Documentation needs to be written. Note that the network_connections ############################################################################### +DEFAULT_ACTIVATION_TIMEOUT = 90 + class CheckMode: PREPARE = "prepare" @@ -1061,6 +1063,36 @@ class NMUtil: % (cb_args.get("error", "unknown error"), cb_args) ) + def create_checkpoint(self, timeout): + """ Create a new checkpoint """ + checkpoint = Util.call_async_method( + self.nmclient, + "checkpoint_create", + [ + [], # devices, empty list is all devices + timeout, + Util.NM().CheckpointCreateFlags.DELETE_NEW_CONNECTIONS + | Util.NM().CheckpointCreateFlags.DISCONNECT_NEW_DEVICES, + ], + ) + + if checkpoint: + return checkpoint.get_path() + return None + + def destroy_checkpoint(self, path): + """ Destroy the specified checkpoint """ + Util.call_async_method(self.nmclient, "checkpoint_destroy", [path]) + + def rollback_checkpoint(self, path): + """ Rollback the specified checkpoint """ + Util.call_async_method( + self.nmclient, + "checkpoint_rollback", + [path], + mainloop_timeout=DEFAULT_ACTIVATION_TIMEOUT, + ) + def wait_till_connection_is_gone(self, uuid, timeout=10): """ Wait until a connection is gone or until the timeout elapsed @@ -1667,6 +1699,9 @@ class Cmd: self.log_fatal(idx, str(e)) self.run_prepare() while self.check_mode_next() != CheckMode.DONE: + if self.check_mode == CheckMode.REAL_RUN: + self.start_transaction() + for idx, connection in enumerate(self.connections): try: for action in connection["actions"]: @@ -1680,12 +1715,14 @@ class Cmd: self.run_action_down(idx) else: assert False - except Exception as e: - self.log_warn( - idx, "failure: %s [[%s]]" % (e, traceback.format_exc()) - ) + except Exception as error: + if self.check_mode == CheckMode.REAL_RUN: + self.rollback_transaction(idx, action, error) raise + if self.check_mode == CheckMode.REAL_RUN: + self.finish_transaction() + def run_prepare(self): for idx, connection in enumerate(self.connections): if "type" in connection and connection["check_iface_exists"]: @@ -1734,6 +1771,28 @@ class Cmd: % (connection["interface_name"], connection["mac"]), ) + def start_transaction(self): + """ Hook before making changes """ + + def finish_transaction(self): + """ Hook for after all changes where made successfuly """ + + def rollback_transaction(self, idx, action, error): + """ Hook if configuring a profile results in an error + + :param idx: Index of the connection that triggered the error + :param action: Action that triggered the error + :param error: The error + + :type idx: int + :type action: str + :type error: Exception + + """ + self.log_warn( + idx, "failure: %s (%s) [[%s]]" % (error, action, traceback.format_exc()) + ) + def run_action_absent(self, idx): raise NotImplementedError() @@ -1755,6 +1814,7 @@ class Cmd_nm(Cmd): Cmd.__init__(self, **kwargs) self._nmutil = None self.validate_one_type = ArgValidator_ListConnections.VALIDATE_ONE_MODE_NM + self._checkpoint = None @property def nmutil(self): @@ -1768,6 +1828,7 @@ class Cmd_nm(Cmd): def run_prepare(self): Cmd.run_prepare(self) + names = {} for connection in self.connections: name = connection["name"] @@ -1789,6 +1850,28 @@ class Cmd_nm(Cmd): connection["nm.exists"] = exists connection["nm.uuid"] = uuid + def start_transaction(self): + Cmd.start_transaction(self) + self._checkpoint = self.nmutil.create_checkpoint( + len(self.connections) * DEFAULT_ACTIVATION_TIMEOUT + ) + + def rollback_transaction(self, idx, action, error): + Cmd.rollback_transaction(self, idx, action, error) + if self._checkpoint: + try: + self.nmutil.rollback_checkpoint(self._checkpoint) + finally: + self._checkpoint = None + + def finish_transaction(self): + Cmd.finish_transaction(self) + if self._checkpoint: + try: + self.nmutil.destroy_checkpoint(self._checkpoint) + finally: + self._checkpoint = None + def run_action_absent(self, idx): seen = set() name = self.connections[idx]["name"] @@ -1941,7 +2024,7 @@ class Cmd_nm(Cmd): wait_time = connection["wait"] if wait_time is None: - wait_time = 90 + wait_time = DEFAULT_ACTIVATION_TIMEOUT try: self.nmutil.connection_activate_wait(ac, wait_time)