From 90f867088f2f86bdb312b505cb96766c124656e9 Mon Sep 17 00:00:00 2001 From: Seddik Alaoui Ismaili Date: Wed, 12 Nov 2025 01:17:46 +0100 Subject: [PATCH 1/3] nmcli idempotency connection check --- plugins/modules/nmcli.py | 91 +++++++++++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 19 deletions(-) diff --git a/plugins/modules/nmcli.py b/plugins/modules/nmcli.py index 58519578c59..1078e4fbb49 100644 --- a/plugins/modules/nmcli.py +++ b/plugins/modules/nmcli.py @@ -2383,7 +2383,24 @@ def connection_exists(self): def down_connection(self): cmd = [self.nmcli_bin, "con", "down", self.conn_name] return self.execute_command(cmd) - + def get_connection_state(self): + """Get the current state of the connection""" + cmd = [self.nmcli_bin, "--terse", "--fields", "GENERAL.STATE", "con", "show", self.conn_name] + (rc, out, err) = self.execute_command(cmd) + if rc != 0: + raise None + + lines = out.strip().split('\n') + for line in lines: + if 'GENERAL.STATE' in line: + state = line.split(':')[-1].strip() + return state + return None + + def is_connection_active(self): + """Check if the connection is currently active""" + state = self.get_connection_state() + return state == "activated" def up_connection(self): cmd = [self.nmcli_bin, "con", "up", self.conn_name] return self.execute_command(cmd) @@ -2934,31 +2951,67 @@ def main(): elif nmcli.state == "up": if nmcli.connection_exists(): - if module.check_mode: - module.exit_json(changed=True) - if nmcli.conn_reload: - (rc, out, err) = nmcli.reload_connection() - (rc, out, err) = nmcli.up_connection() - if rc != 0: - module.fail_json(name=f"Error bringing up connection named {nmcli.conn_name}", msg=err, rc=rc) + is_active = nmcli.is_connection_active() + + if is_active and not nmcli.conn_reload: + result["changed"] = False + result["msg"] = f"Connection {nmcli.conn_name} is already active" + if module.check_mode: + module.exit_json(changed=False, **result) + else: + if module.check_mode: + module.exit_json(changed=True, **result) + + if nmcli.conn_reload: + (rc, out, err) = nmcli.reload_connection() + if rc != 0: + module.fail_json(name=f"Error bringing up connection named {nmcli.conn_name}", msg=err, rc=rc) + + if not is_active or nmcli.conn_reload: + (rc, out, err) = nmcli.up_connection() + if rc != 0: + module.fail_json(name=f"Error bringing up connection named {nmcli.conn_name}", msg=err, rc=rc) + result["changed"] = True + else: + module.fail_json( + name=nmcli.conn_name, + msg="Connection does not exist", + ) elif nmcli.state == "down": if nmcli.connection_exists(): - if module.check_mode: - module.exit_json(changed=True) - if nmcli.conn_reload: - (rc, out, err) = nmcli.reload_connection() - (rc, out, err) = nmcli.down_connection() - if rc != 0: - module.fail_json(name=f"Error bringing down connection named {nmcli.conn_name}", msg=err, rc=rc) + is_active = nmcli.is_connection_active() + + if not is_active and not nmcli.conn_reload: + result["changed"] = False + result["msg"] = f"Connection {nmcli.conn_name} is already inactive" + if module.check_mode: + module.exit_json(changed=False, **result) + else: + if module.check_mode: + module.exit_json(changed=True, **result) + + if nmcli.conn_reload: + (rc, out, err) = nmcli.reload_connection() + if rc != 0: + module.fail_json(name=f"Error reloading connection {nmcli.conn_name}", msg=err, rc=rc) + + if is_active or nmcli.conn_reload: + (rc, out, err) = nmcli.down_connection() + if rc != 0: + module.fail_json(name=f"Error bringing down connection named {nmcli.conn_name}", msg=err, rc=rc) + result["changed"] = True + else: + module.fail_json(msg=f"Connection {nmcli.conn_name} does not exist") except NmcliModuleError as e: module.fail_json(name=nmcli.conn_name, msg=str(e)) - if rc is None: - result["changed"] = False - else: - result["changed"] = True + if "changed" not in result: + if rc is None: + result["changed"] = False + else: + result["changed"] = True if out: result["stdout"] = out if err: From ca2f9e81d65d6085a008b341c44bb3163c607ba6 Mon Sep 17 00:00:00 2001 From: Seddik Alaoui Ismaili Date: Fri, 21 Nov 2025 23:18:52 +0100 Subject: [PATCH 2/3] Changelog fragment and ruff reformat --- .../fragments/11114-nmcli-idempotency.yml | 3 +++ plugins/modules/nmcli.py | 20 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/11114-nmcli-idempotency.yml diff --git a/changelogs/fragments/11114-nmcli-idempotency.yml b/changelogs/fragments/11114-nmcli-idempotency.yml new file mode 100644 index 00000000000..d60f2239383 --- /dev/null +++ b/changelogs/fragments/11114-nmcli-idempotency.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - nmcli - add idempotency check (https://github.com/ansible-collections/community.general/pull/11114). \ No newline at end of file diff --git a/plugins/modules/nmcli.py b/plugins/modules/nmcli.py index 1078e4fbb49..4be54267295 100644 --- a/plugins/modules/nmcli.py +++ b/plugins/modules/nmcli.py @@ -2383,6 +2383,7 @@ def connection_exists(self): def down_connection(self): cmd = [self.nmcli_bin, "con", "down", self.conn_name] return self.execute_command(cmd) + def get_connection_state(self): """Get the current state of the connection""" cmd = [self.nmcli_bin, "--terse", "--fields", "GENERAL.STATE", "con", "show", self.conn_name] @@ -2390,10 +2391,10 @@ def get_connection_state(self): if rc != 0: raise None - lines = out.strip().split('\n') + lines = out.strip().split("\n") for line in lines: - if 'GENERAL.STATE' in line: - state = line.split(':')[-1].strip() + if "GENERAL.STATE" in line: + state = line.split(":")[-1].strip() return state return None @@ -2401,6 +2402,7 @@ def is_connection_active(self): """Check if the connection is currently active""" state = self.get_connection_state() return state == "activated" + def up_connection(self): cmd = [self.nmcli_bin, "con", "up", self.conn_name] return self.execute_command(cmd) @@ -2965,12 +2967,16 @@ def main(): if nmcli.conn_reload: (rc, out, err) = nmcli.reload_connection() if rc != 0: - module.fail_json(name=f"Error bringing up connection named {nmcli.conn_name}", msg=err, rc=rc) + module.fail_json( + name=f"Error bringing up connection named {nmcli.conn_name}", msg=err, rc=rc + ) if not is_active or nmcli.conn_reload: (rc, out, err) = nmcli.up_connection() if rc != 0: - module.fail_json(name=f"Error bringing up connection named {nmcli.conn_name}", msg=err, rc=rc) + module.fail_json( + name=f"Error bringing up connection named {nmcli.conn_name}", msg=err, rc=rc + ) result["changed"] = True else: module.fail_json( @@ -2999,7 +3005,9 @@ def main(): if is_active or nmcli.conn_reload: (rc, out, err) = nmcli.down_connection() if rc != 0: - module.fail_json(name=f"Error bringing down connection named {nmcli.conn_name}", msg=err, rc=rc) + module.fail_json( + name=f"Error bringing down connection named {nmcli.conn_name}", msg=err, rc=rc + ) result["changed"] = True else: module.fail_json(msg=f"Connection {nmcli.conn_name} does not exist") From 0eb6ba8f4b14c649fa61690cf9886d1647688563 Mon Sep 17 00:00:00 2001 From: Seddik Alaoui Ismaili Date: Fri, 21 Nov 2025 23:42:45 +0100 Subject: [PATCH 3/3] Fix : change error handling --- plugins/modules/nmcli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/nmcli.py b/plugins/modules/nmcli.py index 4be54267295..a929dd3a211 100644 --- a/plugins/modules/nmcli.py +++ b/plugins/modules/nmcli.py @@ -2389,7 +2389,7 @@ def get_connection_state(self): cmd = [self.nmcli_bin, "--terse", "--fields", "GENERAL.STATE", "con", "show", self.conn_name] (rc, out, err) = self.execute_command(cmd) if rc != 0: - raise None + raise NmcliModuleError(err) lines = out.strip().split("\n") for line in lines: