From 4c1d8447d0a669bec424543dd43e889eba4a585c Mon Sep 17 00:00:00 2001 From: Vinay M <63404819+roverflow@users.noreply.github.com> Date: Thu, 9 Jan 2025 13:04:28 +0530 Subject: [PATCH 1/9] Fix `nxos_facts` integration test (#917) * Add/Test Integration * test - 2 * fix facts tests and changes in integation * fix * remove file_copy ignore * file copy fix * fixes fstring error * added changelog * file copy * remove vpc merging this with upstream ci fail as these are related to cml --- changelogs/fragments/tests_fix.yaml | 5 +++++ plugins/action/nxos.py | 3 +-- tests/integration/targets/nxos_facts/vars/main.yml | 2 ++ .../targets/nxos_file_copy/tests/cli/input_validation.yaml | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/tests_fix.yaml diff --git a/changelogs/fragments/tests_fix.yaml b/changelogs/fragments/tests_fix.yaml new file mode 100644 index 000000000..bd6d86e79 --- /dev/null +++ b/changelogs/fragments/tests_fix.yaml @@ -0,0 +1,5 @@ +--- +bugfixes: + - Fixes mixed usage of f-string and format string in action plugin for consistency. +trivial: + - Added tests fixes for nxos_facts. diff --git a/plugins/action/nxos.py b/plugins/action/nxos.py index e57c08aeb..c636c636d 100644 --- a/plugins/action/nxos.py +++ b/plugins/action/nxos.py @@ -60,8 +60,7 @@ def run(self, tmp=None, task_vars=None): "msg": ( f"Connection type must be fully qualified name for " f"network_cli connection type, got {self._play_context.connection}" - ) - % self._play_context.connection, + ), } conn = Connection(self._connection.socket_path) diff --git a/tests/integration/targets/nxos_facts/vars/main.yml b/tests/integration/targets/nxos_facts/vars/main.yml index f010eb0ac..65cf3ce18 100644 --- a/tests/integration/targets/nxos_facts/vars/main.yml +++ b/tests/integration/targets/nxos_facts/vars/main.yml @@ -28,4 +28,6 @@ available_network_resources: - static_routes - telemetry - vlans + - vrf_address_family - vrf_global + - vrf_interfaces diff --git a/tests/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml b/tests/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml index bcda7b4fa..e05edc3fb 100644 --- a/tests/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml +++ b/tests/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml @@ -21,7 +21,7 @@ - ansible.builtin.assert: that: - - result is search("argument 'file_pull_timeout' is of type .* and we were unable to convert to int") + - result.msg is ansible.builtin.search("argument 'file_pull_timeout' is of type .* and we were unable to convert to int", multiline=true) - name: Input validation - param should be type register: result From f03aaeef1c588829c8e5c5c87139f62dfdfec162 Mon Sep 17 00:00:00 2001 From: Aayush Anand Date: Thu, 9 Jan 2025 16:26:32 +0530 Subject: [PATCH 2/9] added overridden state for cisco.nxos_telemetry (#918) * added overridden state * chore: auto fixes from pre-commit.com hooks * added changelog * Add changelog for telemetry overridden state * Delete changelogs/fragments/telemetry_overridden.yaml --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Ruchi Pakhle <72685035+Ruchip16@users.noreply.github.com> --- .../fragments/nxos_telemetry_overridden.yaml | 2 + docs/cisco.nxos.nxos_telemetry_module.rst | 28 +++++- .../nxos/argspec/telemetry/telemetry.py | 2 +- .../nxos/config/telemetry/telemetry.py | 4 +- plugins/modules/nxos_telemetry.py | 28 +++++- .../network/nxos/test_nxos_telemetry.py | 95 +++++++++++++++++++ 6 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/nxos_telemetry_overridden.yaml diff --git a/changelogs/fragments/nxos_telemetry_overridden.yaml b/changelogs/fragments/nxos_telemetry_overridden.yaml new file mode 100644 index 000000000..c9a366efc --- /dev/null +++ b/changelogs/fragments/nxos_telemetry_overridden.yaml @@ -0,0 +1,2 @@ +minor_changes: + - "nxos_telemetry - Added support for 'overridden' state to provide complete configuration override capabilities." diff --git a/docs/cisco.nxos.nxos_telemetry_module.rst b/docs/cisco.nxos.nxos_telemetry_module.rst index d1394ccf0..9d6e58535 100644 --- a/docs/cisco.nxos.nxos_telemetry_module.rst +++ b/docs/cisco.nxos.nxos_telemetry_module.rst @@ -565,6 +565,7 @@ Parameters
  • replaced
  • deleted
  • gathered
    • +
  • overridden
    • @@ -653,7 +654,7 @@ Examples # This action will replace telemetry configuration on the device with the # telemetry configuration defined in the playbook. - - name: Override Telemetry Configuration + - name: Replace Telemetry Configuration cisco.nxos.nxos_telemetry: config: certificate: @@ -674,6 +675,31 @@ Examples destination_group: 55 state: replaced + # Using overridden + # This action will override all telemetry configuration on the device with the + # telemetry configuration defined in the playbook. + + - name: Override Telemetry Configuration + cisco.nxos.nxos_telemetry: + config: + certificate: + key: /bootflash/server.key + hostname: localhost + compression: gzip + source_interface: Ethernet1/1 + vrf: management + destination_groups: + - id: 2 + destination: + ip: 192.168.0.2 + port: 50001 + protocol: gRPC + encoding: GPB + subscriptions: + - id: 5 + destination_group: 55 + state: overridden + Return Values diff --git a/plugins/module_utils/network/nxos/argspec/telemetry/telemetry.py b/plugins/module_utils/network/nxos/argspec/telemetry/telemetry.py index 7da72979b..b6b6c6ef3 100644 --- a/plugins/module_utils/network/nxos/argspec/telemetry/telemetry.py +++ b/plugins/module_utils/network/nxos/argspec/telemetry/telemetry.py @@ -108,7 +108,7 @@ class TelemetryArgs(object): # pylint: disable=R0903 "type": "dict", }, "state": { - "choices": ["merged", "replaced", "deleted", "gathered"], + "choices": ["merged", "replaced", "deleted", "gathered", "overridden"], "default": "merged", "type": "str", }, diff --git a/plugins/module_utils/network/nxos/config/telemetry/telemetry.py b/plugins/module_utils/network/nxos/config/telemetry/telemetry.py index da3f77439..9f3e109ed 100644 --- a/plugins/module_utils/network/nxos/config/telemetry/telemetry.py +++ b/plugins/module_utils/network/nxos/config/telemetry/telemetry.py @@ -82,8 +82,6 @@ def execute_module(self): warnings = list() state = self._module.params["state"] - if "overridden" in state: - self._module.fail_json(msg="State is invalid for this module.") # When state is 'deleted', the module_params should not contain data # under the 'config' key if "deleted" in state and self._module.params.get("config"): @@ -153,7 +151,7 @@ def set_state(self, want, have): # and does not require any processing using NxosCmdRef objects. if state == "deleted": return self._state_deleted(want, have) - elif state == "replaced": + elif state in ["replaced", "overridden"]: if want == have: return [] return self._state_replaced(want, have) diff --git a/plugins/modules/nxos_telemetry.py b/plugins/modules/nxos_telemetry.py index 89a58e2d2..4020c80e0 100644 --- a/plugins/modules/nxos_telemetry.py +++ b/plugins/modules/nxos_telemetry.py @@ -199,6 +199,7 @@ - replaced - deleted - gathered + - overridden default: merged """ @@ -267,7 +268,7 @@ # This action will replace telemetry configuration on the device with the # telemetry configuration defined in the playbook. -- name: Override Telemetry Configuration +- name: Replace Telemetry Configuration cisco.nxos.nxos_telemetry: config: certificate: @@ -287,6 +288,31 @@ - id: 5 destination_group: 55 state: replaced + +# Using overridden +# This action will override all telemetry configuration on the device with the +# telemetry configuration defined in the playbook. + +- name: Override Telemetry Configuration + cisco.nxos.nxos_telemetry: + config: + certificate: + key: /bootflash/server.key + hostname: localhost + compression: gzip + source_interface: Ethernet1/1 + vrf: management + destination_groups: + - id: 2 + destination: + ip: 192.168.0.2 + port: 50001 + protocol: gRPC + encoding: GPB + subscriptions: + - id: 5 + destination_group: 55 + state: overridden """ RETURN = """ before: diff --git a/tests/unit/modules/network/nxos/test_nxos_telemetry.py b/tests/unit/modules/network/nxos/test_nxos_telemetry.py index 2c8027443..3e64d93b0 100644 --- a/tests/unit/modules/network/nxos/test_nxos_telemetry.py +++ b/tests/unit/modules/network/nxos/test_nxos_telemetry.py @@ -1997,6 +1997,101 @@ def test_tms_names_idempotent(self): ) self.execute_module(changed=False, commands=[]) + def test_tms_overridden_n9k(self): + # Assumes feature telemetry is enabled + # Similar to replaced state: + # - Modify vrf global config, remove default all other global config. + # - destination-group 2 destination '192.168.0.1' idempotent + # - destination-group 2 destination '192.168.0.2' remove + # - remove all other destination-groups + # - Modify sensor-group 55 and delete all others + # - Modify subscription 7, add 10 and delete all others + self.execute_show_command.return_value = load_fixture( + "nxos_telemetry", + "N9K.cfg", + ) + self.get_platform_shortname.return_value = "N9K" + set_module_args( + { + "state": "overridden", + "config": { + "vrf": "blue", + "destination_groups": [ + { + "id": 2, + "destination": { + "ip": "192.168.0.1", + "port": 50001, + "protocol": "GRPC", + "encoding": "GPB", + }, + }, + ], + "sensor_groups": [ + { + "id": 55, + "data_source": "NX-API", + "path": { + "name": "sys/bgp", + "depth": 0, + "query_condition": "query_condition_xyz", + "filter_condition": "filter_condition_xyz", + }, + }, + ], + "subscriptions": [ + { + "id": 7, + "destination_group": 10, + "sensor_group": { + "id": 55, + "sample_interval": 1000, + }, + }, + { + "id": 10, + "destination_group": 2, + "sensor_group": { + "id": 55, + "sample_interval": 1000, + }, + }, + ], + }, + }, + ignore_provider_arg, + ) + self.execute_module( + changed=True, + commands=[ + "telemetry", + "no subscription 3", + "no subscription 5", + "no subscription 4", + "subscription 7", + "no snsr-grp 2 sample-interval 1000", + "no subscription 6", + "no sensor-group 56", + "no sensor-group 2", + "no destination-group 10", + "destination-group 2", + "no ip address 192.168.0.2 port 60001 protocol grpc encoding gpb", + "sensor-group 55", + "data-source NX-API", + "path sys/bgp depth 0 query-condition query_condition_xyz filter-condition filter_condition_xyz", + "subscription 10", + "dst-grp 2", + "snsr-grp 55 sample-interval 1000", + "subscription 7", + "snsr-grp 55 sample-interval 1000", + "no certificate /bootflash/server.key localhost", + "destination-profile", + "no use-compression gzip", + "no source-interface loopback55", + "use-vrf blue", + ], + ) + def build_args(data, type, state=None, check_mode=None): if state is None: From f361495b5554585a844ed9ac1b669d341ce3c3fc Mon Sep 17 00:00:00 2001 From: Aayush Anand Date: Thu, 9 Jan 2025 16:36:17 +0530 Subject: [PATCH 3/9] fix None value handling in l2 interfaces (#921) * fix handling of None state in l2_interfaces * added_changelogs --------- Co-authored-by: Sagar Paul --- changelogs/fragments/nxos_l2_interfaces.yaml | 3 +++ .../config/l2_interfaces/l2_interfaces.py | 15 +++++++++++---- .../tests/common/_populate_config.yaml | 9 +++++++++ .../tests/common/replaced.yaml | 19 +++++++++++++++++-- 4 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/nxos_l2_interfaces.yaml diff --git a/changelogs/fragments/nxos_l2_interfaces.yaml b/changelogs/fragments/nxos_l2_interfaces.yaml new file mode 100644 index 000000000..e7b055781 --- /dev/null +++ b/changelogs/fragments/nxos_l2_interfaces.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - "nxos_l2_interfaces - Fixed handling of 'none' value in allowed_vlans to properly set trunk VLAN none" diff --git a/plugins/module_utils/network/nxos/config/l2_interfaces/l2_interfaces.py b/plugins/module_utils/network/nxos/config/l2_interfaces/l2_interfaces.py index 913f8ecfb..11e8ec3d3 100644 --- a/plugins/module_utils/network/nxos/config/l2_interfaces/l2_interfaces.py +++ b/plugins/module_utils/network/nxos/config/l2_interfaces/l2_interfaces.py @@ -145,9 +145,13 @@ def expand_trunk_allowed_vlans(self, d): return None if "trunk" in d and d["trunk"]: if "allowed_vlans" in d["trunk"]: - allowed_vlans = vlan_range_to_list(d["trunk"]["allowed_vlans"]) - vlans_list = [str(line) for line in sorted(allowed_vlans)] - d["trunk"]["allowed_vlans"] = ",".join(vlans_list) + if d["trunk"]["allowed_vlans"]: + if d["trunk"]["allowed_vlans"].lower() == "none": + return + else: + allowed_vlans = vlan_range_to_list(d["trunk"]["allowed_vlans"]) + vlans_list = [str(line) for line in sorted(allowed_vlans)] + d["trunk"]["allowed_vlans"] = ",".join(vlans_list) def set_state(self, want, have): """Select the appropriate function based on the state provided @@ -347,5 +351,8 @@ def _reconstruct_commands(self, cmds): ) if match: data = match.groupdict() - unparsed = vlan_list_to_range(data["vlans"].split(",")) + if data["vlans"].lower() != "none": + unparsed = vlan_list_to_range(data["vlans"].split(",")) + else: + unparsed = "none" cmds[idx] = data["cmd"] + " " + unparsed diff --git a/tests/integration/targets/nxos_l2_interfaces/tests/common/_populate_config.yaml b/tests/integration/targets/nxos_l2_interfaces/tests/common/_populate_config.yaml index 7f2724c9f..9451733c3 100644 --- a/tests/integration/targets/nxos_l2_interfaces/tests/common/_populate_config.yaml +++ b/tests/integration/targets/nxos_l2_interfaces/tests/common/_populate_config.yaml @@ -17,3 +17,12 @@ parents: "interface {{ nxos_int2 }}" vars: ansible_connection: ansible.netcommon.network_cli + +- name: Set VLAN trunking properties with "none" + cisco.nxos.nxos_config: + lines: + - "switchport" + - "switchport trunk allowed vlan none" + parents: "interface {{ nxos_int3 }}" + vars: + ansible_connection: ansible.netcommon.network_cli diff --git a/tests/integration/targets/nxos_l2_interfaces/tests/common/replaced.yaml b/tests/integration/targets/nxos_l2_interfaces/tests/common/replaced.yaml index c37061c05..3720d0a47 100644 --- a/tests/integration/targets/nxos_l2_interfaces/tests/common/replaced.yaml +++ b/tests/integration/targets/nxos_l2_interfaces/tests/common/replaced.yaml @@ -2,10 +2,11 @@ - ansible.builtin.debug: msg: Start nxos_l2_interfaces replaced integration tests connection={{ ansible_connection }} -- name: Set a fact for 'test_int1' and 'test_int2' +- name: Set a fact for 'test_int1','test_int2'name and 'test_int3' ansible.builtin.set_fact: test_int1: "{{ nxos_int1 }}" test_int2: "{{ nxos_int2 }}" + test_int3: "{{ nxos_int3 }}" - name: Setup1 ignore_errors: true @@ -13,6 +14,7 @@ lines: - "default interface {{ test_int1 }}" - "default interface {{ test_int2 }}" + - "default interface {{ test_int3 }}" - block: - name: Setup2 @@ -30,6 +32,13 @@ - "switchport trunk allowed vlan 25-27" parents: "interface {{ test_int2 }}" + - name: Setup4 + cisco.nxos.nxos_config: + lines: + - "switchport" + - "switchport trunk allowed vlan 100-200" + parents: "interface {{ test_int3 }}" + - name: Gather l2_interfaces facts cisco.nxos.nxos_facts: &id001 gather_subset: @@ -50,6 +59,10 @@ - name: "{{ test_int2 }}" trunk: allowed_vlans: 25-27 + + - name: "{{ test_int3 }}" + trunk: + allowed_vlans: none state: replaced - ansible.builtin.assert: @@ -60,7 +73,9 @@ - "'switchport trunk allowed vlan 10-12' in result.commands" - "'interface {{ test_int2 }}' in result.commands" - "'no switchport trunk native vlan' in result.commands" - - result.commands|length == 5 + - "'interface {{ test_int3 }}' in result.commands" + - "'switchport trunk allowed vlan none' in result.commands" + - result.commands|length == 7 - name: Gather l2_interfaces post facts cisco.nxos.nxos_facts: *id001 From b05e63faadf9cb8dc397ca106c1a0678429ba94f Mon Sep 17 00:00:00 2001 From: Aayush Anand Date: Thu, 9 Jan 2025 16:42:23 +0530 Subject: [PATCH 4/9] Fix hardware subsets for nxos 9.3 version (#919) * fixed onemin percentage * added changelog * chore: auto fixes from pre-commit.com hooks * Update base.py * chore: auto fixes from pre-commit.com hooks * included vrf_interfaces and vrf_address_family in tests var * fix sanity * chore: auto fixes from pre-commit.com hooks * Update base.py --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Nilashish Chakraborty --- changelogs/fragments/nxos_facts_cpu_utilization.yaml | 2 ++ plugins/module_utils/network/nxos/facts/legacy/base.py | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/nxos_facts_cpu_utilization.yaml diff --git a/changelogs/fragments/nxos_facts_cpu_utilization.yaml b/changelogs/fragments/nxos_facts_cpu_utilization.yaml new file mode 100644 index 000000000..fe93ca2d5 --- /dev/null +++ b/changelogs/fragments/nxos_facts_cpu_utilization.yaml @@ -0,0 +1,2 @@ +bugfixes: + - "Fixed hardware fact gathering failure for CPU utilization parsing on NX-OS 9.3(3) by handling both list and single value formats of onemin_percent" diff --git a/plugins/module_utils/network/nxos/facts/legacy/base.py b/plugins/module_utils/network/nxos/facts/legacy/base.py index aa39ea734..296539d79 100644 --- a/plugins/module_utils/network/nxos/facts/legacy/base.py +++ b/plugins/module_utils/network/nxos/facts/legacy/base.py @@ -169,6 +169,10 @@ def populate(self): self.facts["cpu_utilization"] = self.parse_cpu_utilization(data) def parse_cpu_utilization(self, data): + onemin = data.get("onemin_percent", ["0"]) + if not isinstance(onemin, list): + onemin = [str(onemin)] + onemin_value = onemin[0] return { "core": { "five_minutes": int(data.get("fivemin_percent", 0)), @@ -178,7 +182,7 @@ def parse_cpu_utilization(self, data): "five_seconds_interrupt": int( data.get("fivesec_intr_percent", 0), ), - "one_minute": int(data.get("onemin_percent", 0)), + "one_minute": int(onemin_value), }, } From bb20f31533dbdfff39e356616a779dfd45d04e7f Mon Sep 17 00:00:00 2001 From: Vinay M <63404819+roverflow@users.noreply.github.com> Date: Mon, 13 Jan 2025 14:21:23 +0530 Subject: [PATCH 5/9] Fix downstream tests (#922) * Fix downstream tests * remove comment * fix sapce * skip test via inventory var * revert existing logic * change nxos_skip location --- .../integration/targets/nxos_vrf_address_family/tasks/main.yml | 1 + .../targets/nxos_vrf_interfaces/tests/common/deleted.yaml | 3 +++ tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml | 2 ++ 3 files changed, 6 insertions(+) diff --git a/tests/integration/targets/nxos_vrf_address_family/tasks/main.yml b/tests/integration/targets/nxos_vrf_address_family/tasks/main.yml index adc4c6075..3a624e256 100644 --- a/tests/integration/targets/nxos_vrf_address_family/tasks/main.yml +++ b/tests/integration/targets/nxos_vrf_address_family/tasks/main.yml @@ -1,5 +1,6 @@ --- - name: Main task for vrf_global module + when: not nxos_skip_marked | default(false) ansible.builtin.include_tasks: cli.yaml tags: - network_cli diff --git a/tests/integration/targets/nxos_vrf_interfaces/tests/common/deleted.yaml b/tests/integration/targets/nxos_vrf_interfaces/tests/common/deleted.yaml index 16338228f..432527332 100644 --- a/tests/integration/targets/nxos_vrf_interfaces/tests/common/deleted.yaml +++ b/tests/integration/targets/nxos_vrf_interfaces/tests/common/deleted.yaml @@ -11,6 +11,9 @@ cisco.nxos.nxos_vrf_interfaces: &id001 config: - name: Ethernet1/2 + vrf_name: test + - name: Ethernet1/6 + vrf_name: test2 state: deleted - name: Assert that correct set of commands were generated diff --git a/tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml b/tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml index be5c063ba..68b71dbfe 100644 --- a/tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml +++ b/tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml @@ -77,6 +77,8 @@ deleted: commands: - interface Ethernet1/2 - no vrf member test + - interface Ethernet1/6 + - no vrf member test2 before: - name: "Ethernet1/1" - name: "Ethernet1/2" From af722facbee36ee3f928d1a1af1810af03794c8d Mon Sep 17 00:00:00 2001 From: Sean Sullivan Date: Wed, 15 Jan 2025 01:46:41 -0500 Subject: [PATCH 6/9] lag_interfaces - Fix bug where lag interfaces was not erroring on command failure (#923) * lag_interfaces - Fix bug where lag interfaces was not erroring on command failure. * linting * added pr to changelog --- changelogs/fragments/lag_interfaces_error.yaml | 3 +++ galaxy.yml | 2 +- .../network/nxos/config/lag_interfaces/lag_interfaces.py | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/lag_interfaces_error.yaml diff --git a/changelogs/fragments/lag_interfaces_error.yaml b/changelogs/fragments/lag_interfaces_error.yaml new file mode 100644 index 000000000..bb607bd58 --- /dev/null +++ b/changelogs/fragments/lag_interfaces_error.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - "lag_interfaces - Fix bug where lag interfaces was not erroring on command failure. (https://github.com/ansible-collections/cisco.nxos/pull/923)" diff --git a/galaxy.yml b/galaxy.yml index 494fdf171..c5d8ff2a8 100644 --- a/galaxy.yml +++ b/galaxy.yml @@ -10,4 +10,4 @@ readme: README.md repository: https://github.com/ansible-collections/cisco.nxos issues: https://github.com/ansible-collections/cisco.nxos/issues tags: [cisco, nxos, networking, nxapi, netconf] -version: 9.2.1 +version: 9.2.2-devel diff --git a/plugins/module_utils/network/nxos/config/lag_interfaces/lag_interfaces.py b/plugins/module_utils/network/nxos/config/lag_interfaces/lag_interfaces.py index fc3202a7f..433a5a950 100644 --- a/plugins/module_utils/network/nxos/config/lag_interfaces/lag_interfaces.py +++ b/plugins/module_utils/network/nxos/config/lag_interfaces/lag_interfaces.py @@ -86,6 +86,8 @@ def execute_module(self): err_str = item if err_str.lower().startswith("cannot add"): self._module.fail_json(msg=err_str) + elif err_str.lower().startswith("command failed"): + self._module.fail_json(msg=err_str) result["changed"] = True if self.state in self.ACTION_STATES: From d86df3d1ad0b992242b3617bb5edece09afdf0ea Mon Sep 17 00:00:00 2001 From: Vinay M <63404819+roverflow@users.noreply.github.com> Date: Thu, 16 Jan 2025 11:31:33 +0530 Subject: [PATCH 7/9] Fix vrf_interfaces test cases (#924) * Fix vrf_interfaces test cases * remove gathered * Fix downstream vrf interfaces * fix gatherd --- .../tests/common/gathered.yaml | 8 ++++- .../tests/common/merged.yaml | 16 ++++++++-- .../tests/common/overridden.yaml | 16 ++++++++-- .../tests/common/replaced.yaml | 16 ++++++++-- .../nxos_vrf_interfaces/tests/common/rtt.yaml | 2 +- .../nxos_vrf_interfaces/vars/main.yaml | 30 ++++++++----------- 6 files changed, 62 insertions(+), 26 deletions(-) diff --git a/tests/integration/targets/nxos_vrf_interfaces/tests/common/gathered.yaml b/tests/integration/targets/nxos_vrf_interfaces/tests/common/gathered.yaml index 2c96fe709..28d46f965 100644 --- a/tests/integration/targets/nxos_vrf_interfaces/tests/common/gathered.yaml +++ b/tests/integration/targets/nxos_vrf_interfaces/tests/common/gathered.yaml @@ -16,7 +16,13 @@ ansible.builtin.assert: that: - not result.changed - - "{{ gathered['config'] | symmetric_difference(result['gathered']) |length == 0 }}" + - > + {{ + result['gathered'] + | selectattr('name', 'in', 'Ethernet1/2,Ethernet1/6') + | symmetric_difference(gathered['config']) + | length == 0 + }} always: - ansible.builtin.include_tasks: _remove_config.yaml diff --git a/tests/integration/targets/nxos_vrf_interfaces/tests/common/merged.yaml b/tests/integration/targets/nxos_vrf_interfaces/tests/common/merged.yaml index 3bec76054..1f01ba06d 100644 --- a/tests/integration/targets/nxos_vrf_interfaces/tests/common/merged.yaml +++ b/tests/integration/targets/nxos_vrf_interfaces/tests/common/merged.yaml @@ -23,12 +23,24 @@ - name: Assert that before dicts are correctly generated ansible.builtin.assert: that: - - "{{ merged['before'] | symmetric_difference(result['before']) | length == 0 }}" + - > + {{ + result['before'] + | selectattr('name', 'in', 'Ethernet1/2,Ethernet1/6') + | symmetric_difference(merged['before']) + | length == 0 + }} - name: Assert that after dict is correctly generated ansible.builtin.assert: that: - - "{{ merged['after'] | symmetric_difference(result['after']) | length == 0 }}" + - > + {{ + result['after'] + | selectattr('name', 'in', 'Ethernet1/2,Ethernet1/6') + | symmetric_difference(merged['after']) + | length == 0 + }} - name: Merge provided configuration with device configuration (idempotent) register: result diff --git a/tests/integration/targets/nxos_vrf_interfaces/tests/common/overridden.yaml b/tests/integration/targets/nxos_vrf_interfaces/tests/common/overridden.yaml index 01800c8d5..0816b2b13 100644 --- a/tests/integration/targets/nxos_vrf_interfaces/tests/common/overridden.yaml +++ b/tests/integration/targets/nxos_vrf_interfaces/tests/common/overridden.yaml @@ -23,12 +23,24 @@ - name: Assert that before dicts are correctly generated ansible.builtin.assert: that: - - overridden['before'] == result['before'] + - > + {{ + result['before'] + | selectattr('name', 'in', 'Ethernet1/2,Ethernet1/6') + | symmetric_difference(overridden['before']) + | length == 0 + }} - name: Assert that after dict is correctly generated ansible.builtin.assert: that: - - overridden['after'] == result['after'] + - > + {{ + result['after'] + | selectattr('name', 'in', 'Ethernet1/2,Ethernet1/6') + | symmetric_difference(overridden['after']) + | length == 0 + }} - name: Override provided configuration with device configuration (idempotent) register: result diff --git a/tests/integration/targets/nxos_vrf_interfaces/tests/common/replaced.yaml b/tests/integration/targets/nxos_vrf_interfaces/tests/common/replaced.yaml index 3a3240a08..ecad626cb 100644 --- a/tests/integration/targets/nxos_vrf_interfaces/tests/common/replaced.yaml +++ b/tests/integration/targets/nxos_vrf_interfaces/tests/common/replaced.yaml @@ -24,12 +24,24 @@ - name: Assert that before dicts are correctly generated ansible.builtin.assert: that: - - replaced['before'] == result['before'] + - > + {{ + result['before'] + | selectattr('name', 'in', 'Ethernet1/2,Ethernet1/6') + | symmetric_difference(replaced['before']) + | length == 0 + }} - name: Assert that after dict is correctly generated ansible.builtin.assert: that: - - replaced['after'] == result['after'] + - > + {{ + result['after'] + | selectattr('name', 'in', 'Ethernet1/2,Ethernet1/6') + | symmetric_difference(replaced['after']) + | length == 0 + }} - name: Replace provided VRF interfaces configuration (idempotent) register: result diff --git a/tests/integration/targets/nxos_vrf_interfaces/tests/common/rtt.yaml b/tests/integration/targets/nxos_vrf_interfaces/tests/common/rtt.yaml index f5890e29a..1f7e85dc8 100644 --- a/tests/integration/targets/nxos_vrf_interfaces/tests/common/rtt.yaml +++ b/tests/integration/targets/nxos_vrf_interfaces/tests/common/rtt.yaml @@ -36,7 +36,7 @@ - ansible.builtin.assert: that: - result.changed == true - - result.commands|symmetric_difference(overridden.commands) == [] + - result.commands|symmetric_difference(rtt.commands_no_revert) == [] - name: Revert back to base configuration using facts round trip register: revert diff --git a/tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml b/tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml index 68b71dbfe..685fbec44 100644 --- a/tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml +++ b/tests/integration/targets/nxos_vrf_interfaces/vars/main.yaml @@ -1,10 +1,10 @@ --- gathered: config: - - name: "Ethernet1/1" - name: "Ethernet1/2" - - name: "Ethernet1/3" - - name: "Ethernet1/4" + vrf_name: "test" + - name: "Ethernet1/6" + vrf_name: "test2" merged: commands: @@ -13,15 +13,11 @@ merged: - interface Ethernet1/6 - vrf member test2 before: - - name: "Ethernet1/1" - name: "Ethernet1/2" - - name: "Ethernet1/3" - name: "Ethernet1/6" after: - - name: "Ethernet1/1" - name: "Ethernet1/2" vrf_name: "test" - - name: "Ethernet1/3" - name: "Ethernet1/6" vrf_name: "test2" @@ -32,16 +28,13 @@ overridden: - interface Ethernet1/6 - vrf member VRF9 before: - - name: "Ethernet1/1" - name: "Ethernet1/2" - vrf_name: "vrf_B" - - name: "Ethernet1/3" + vrf_name: "test" - name: "Ethernet1/6" + vrf_name: "test2" after: - - name: "Ethernet1/1" - name: "Ethernet1/2" vrf_name: "VRF8" - - name: "Ethernet1/3" - name: "Ethernet1/6" vrf_name: "VRF9" @@ -49,7 +42,7 @@ parsed: after: - name: "Ethernet1/2" vrf_name: "VRF1" - - name: "GigabitEthernet1/6" + - name: "Ethernet1/6" vrf_name: "TEST_VRF" replaced: @@ -59,17 +52,13 @@ replaced: - interface Ethernet1/6 - vrf member TEST_VRF3 before: - - name: "Ethernet1/1" - name: "Ethernet1/2" vrf_name: "test" - - name: "Ethernet1/3" - name: "Ethernet1/6" vrf_name: "test2" after: - - name: "Ethernet1/1" - name: "Ethernet1/2" vrf_name: "TEST_VRF2" - - name: "Ethernet1/3" - name: "Ethernet1/6" vrf_name: "TEST_VRF3" @@ -93,7 +82,12 @@ deleted: rtt: commands: - - interface "Ethernet1/6" + - interface Ethernet1/6 - vrf member test - interface Ethernet1/2 - no vrf member test2 + commands_no_revert: + - interface Ethernet1/2 + - vrf member test2 + - interface Ethernet1/6 + - no vrf member test From a6f433e28d9baee3b77affd5ba7813374fdd7344 Mon Sep 17 00:00:00 2001 From: Vinay M <63404819+roverflow@users.noreply.github.com> Date: Thu, 16 Jan 2025 19:42:33 +0530 Subject: [PATCH 8/9] Disable VRF Address family role (#926) --- .../integration/targets/nxos_vrf_address_family/tasks/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/targets/nxos_vrf_address_family/tasks/main.yml b/tests/integration/targets/nxos_vrf_address_family/tasks/main.yml index 3a624e256..336ff2661 100644 --- a/tests/integration/targets/nxos_vrf_address_family/tasks/main.yml +++ b/tests/integration/targets/nxos_vrf_address_family/tasks/main.yml @@ -1,6 +1,6 @@ --- - name: Main task for vrf_global module - when: not nxos_skip_marked | default(false) + when: not nxos_skip_marked | default(true) ansible.builtin.include_tasks: cli.yaml tags: - network_cli From 56ad042760a4c76ae9c0ef438b21db9901f5b6f1 Mon Sep 17 00:00:00 2001 From: Aayush Anand Date: Thu, 16 Jan 2025 20:33:43 +0530 Subject: [PATCH 9/9] Fix nxos_user purge deleting non-locally configured users. (#903) * fixes_nxos_user purge behaviour * adding changelog * chore: auto fixes from pre-commit.com hooks * added unit test for preserving non-local users while purging * chore: auto fixes from pre-commit.com hooks * Update nxos_user.py * chore: auto fixes from pre-commit.com hooks --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Ruchi Pakhle <72685035+Ruchip16@users.noreply.github.com> Co-authored-by: Nilashish Chakraborty --- changelogs/fragments/purged_user.yaml | 3 + plugins/modules/nxos_user.py | 24 +++++++- .../modules/network/nxos/test_nxos_user.py | 55 +++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/purged_user.yaml diff --git a/changelogs/fragments/purged_user.yaml b/changelogs/fragments/purged_user.yaml new file mode 100644 index 000000000..7fed83013 --- /dev/null +++ b/changelogs/fragments/purged_user.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - Fixes nxos_user purge deleting non-local users,ensuring only local users are removed. diff --git a/plugins/modules/nxos_user.py b/plugins/modules/nxos_user.py index d3e847359..e966326df 100644 --- a/plugins/modules/nxos_user.py +++ b/plugins/modules/nxos_user.py @@ -412,6 +412,19 @@ def update_objects(want, have): return updates +def get_configured_usernames(module): + config_output = run_commands( + module, + [{"command": "show running-config | section ^username", "output": "text"}], + ) + usernames = set() + for line in config_output[0].splitlines(): + if line.startswith("username "): + username = line.split()[1] + usernames.add(username) + return usernames + + def main(): """main entry point for module execution""" element_spec = dict( @@ -457,9 +470,14 @@ def main(): commands = map_obj_to_commands(update_objects(want, have), module) if module.params["purge"]: - want_users = [x["name"] for x in want] - have_users = [x["name"] for x in have] - for item in set(have_users).difference(want_users): + want_users = set([x["name"] for x in want]) + have_users = set([x["name"] for x in have]) + + configured_usernames = get_configured_usernames(module) + + non_local_users = have_users.difference(want_users).difference(configured_usernames) + + for item in configured_usernames.difference(non_local_users): if item != "admin": item = item.replace("\\", "\\\\") commands.append("no username %s" % item) diff --git a/tests/unit/modules/network/nxos/test_nxos_user.py b/tests/unit/modules/network/nxos/test_nxos_user.py index ba0d43756..fea3995f3 100644 --- a/tests/unit/modules/network/nxos/test_nxos_user.py +++ b/tests/unit/modules/network/nxos/test_nxos_user.py @@ -116,3 +116,58 @@ def test_nxos_hashed_password(self): "username ansible password 5 $5$JFHICC$u.zXRUgprAkkYLiEns8VrhsNEIOj7FzVrn67tuJdtKB", ], ) + + def test_purge_with_non_local_users(self): + self.run_commands.side_effect = [ + [ + { + "TABLE_template": { + "ROW_template": [ + { + "usr_name": "admin", + "expire_date": "this user account has no expiry date", + "TABLE_role": {"ROW_role": {"role": "network-admin"}}, + }, + { + "usr_name": "ansible-test-1", + "expire_date": "this user account has no expiry date", + "TABLE_role": {"ROW_role": [{"role": "network-operator"}]}, + }, + { + "usr_name": "ansible-test-2", + "expire_date": "this user account has no expiry date", + "TABLE_role": {"ROW_role": [{"role": "network-operator"}]}, + }, + { + "usr_name": "domain\\remote-user", + "expire_date": "this user account has no expiry date", + "TABLE_role": {"ROW_role": [{"role": "network-operator"}]}, + }, + ], + }, + }, + ], + [ + "username admin password 5 $5$JFHICC$QwE password\n" + "username ansible-test-1 password 5 $5$JFHICC$abc password\n" + "username ansible-test-2 password 5 $5$JFHICC$def password\n", + ], + ] + set_module_args(dict(name="new-user", configured_password="ansible123", purge=True)) + result = self.execute_module( + changed=True, + commands=[ + "no username ansible-test-1", + "no username ansible-test-2", + "username new-user", + "username new-user password ansible123", + # domain\remote-user should NOT be in the commands list as it's preserved + ], + ) + expected_commands = [ + "no username ansible-test-1", + "no username ansible-test-2", + "username new-user", + "username new-user password ansible123", + ] + self.assertEqual(sorted(result["commands"]), sorted(expected_commands))