Added ndo_fabric_span_session_source_filter module to manage the SPAN session source filter (DCNE-376) #677

sajagana · 2025-05-29T16:25:11Z

Added parent ndo_fabric_span_session_source module changes for the testing purpose.

plugins/modules/ndo_fabric_span_session_source.py

akinross · 2025-06-20T12:17:08Z

plugins/module_utils/template.py

+        """
+        existing_objects = self.template.get("monitoringTemplate", {}).get("template", {}).get("spanSessions", [])
+        if uuid or name:  # Query a specific object
+            return self.get_object_by_key_value_pairs("SPAN Sessions", existing_objects, [KVPair("uuid", uuid) if uuid else KVPair("name", name)], fail_module)


is plural form correct here?

Suggested change

return self.get_object_by_key_value_pairs("SPAN Sessions", existing_objects, [KVPair("uuid", uuid) if uuid else KVPair("name", name)], fail_module)

return self.get_object_by_key_value_pairs("SPAN Session", existing_objects, [KVPair("uuid", uuid) if uuid else KVPair("name", name)], fail_module)

applies to all 3

akinross · 2025-06-20T12:17:29Z

plugins/module_utils/template.py

+                KVPair("destPortTo", int(PORT_MAPPING.get(filter_config.get("destination_port_to")))),
+                KVPair("ipProtocol", IP_PROTOCOL_MAPPING.get(filter_config.get("ip_protocol"))),
+            ]
+            return self.get_object_by_key_value_pairs("SPAN Sessions Source", search_list, KVPairs, fail_module)


Suggested change

return self.get_object_by_key_value_pairs("SPAN Sessions Source", search_list, KVPairs, fail_module)

return self.get_object_by_key_value_pairs("SPAN Sessions Source Filter", search_list, KVPairs, fail_module)

plugins/module_utils/template.py

anvitha-jain · 2025-06-20T23:27:27Z

plugins/module_utils/template.py

+            return self.get_object_by_key_value_pairs("SPAN Session Source", search_list, [KVPair("name", name)], fail_module)
+        return search_list  # Query all objects
+
+    def get_fabric_span_session_source_filter(self, filter_config, search_list, fail_module=False):


If this function is used only in one module, this should be moved inside the module.

anvitha-jain · 2025-06-20T23:35:49Z

tests/integration/targets/ndo_fabric_span_session_source/tasks/main.yml

+        state: present
+      register: update_filter_l3out_ansible_test_source_2
+
+    - name: Assertion check for update SPAN Session source with different options


This assesrtion is task is too big, it has to split based on add, remove, update according to the tasks

akinross · 2025-06-21T07:13:47Z

plugins/modules/ndo_fabric_span_session_source.py

+        - This parameter or O(filter_epg.epg) is required.
+        type: str
+        aliases: [ epg_uuid ]
+      epg:


Do we want to name this epg or something like reference? I personally find the filter_epg.epg a bit redundant. In my PR I have a port_channel dict, which would then be port_channel.port_channel.some_key.

akinross · 2025-06-21T07:14:46Z

plugins/modules/ndo_fabric_span_session_source.py

+    type: list
+    elements: dict
+    suboptions:
+      access_path_type:


should we use an alias of type? or is this a reserved key in ansible?

akinross · 2025-06-21T07:15:56Z

plugins/modules/ndo_fabric_span_session_source.py

+    choices: [ incoming, outgoing, both ]
+  span_drop_packets:
+    description:
+    - The SPAN Drop Packets of the SPAN Session source.


should we add a explanation of behaviour here?

akinross · 2025-06-21T07:20:40Z

plugins/modules/ndo_fabric_span_session_source.py

+    description:
+    - The Filter L3Out of the SPAN Session source.
+    - When the Filter L3Out is specified in the configuration, the Filter EPG will be removed.
+    - Filter L3Out and Filter EPG cannot be configured simultaneously.


so we are adding this line now as standard? Should this also be added then to the OR description for all other mutual exclusives? uuid/name because or would not indicate that they cannot be provided together?

We do not have a standard sentence for mutual exclusives condition. But changed the current description to match the current format.

akinross · 2025-06-21T07:21:31Z

plugins/modules/ndo_fabric_span_session_source.py

+      template_id:
+        description:
+        - The ID of the Fabric Resource Policy template.
+        - This parameter or O(access_paths.template) is required to configure the 'Port Channel' or 'Virtual Port Channel' Access Path.


should we use O(access_path_type= port_channel) for these references?

akinross · 2025-06-27T08:11:27Z

plugins/modules/ndo_fabric_span_session_source.py

+    - The SPAN Drop Packets of the SPAN Session source.
+    - SPAN Drop Packets are packets that get dropped when the SPAN destination port cannot handle the volume of mirrored traffic from the source ports.
+    - Defaults to O(span_drop_packets=false) when unset during creation.
+    - The O(filter_epg) and O(filter_l3out) is not configurable when this parameter O(span_drop_packets=true) set to true.


Suggested change

- The O(filter_epg) and O(filter_l3out) is not configurable when this parameter O(span_drop_packets=true) set to true.

- The O(filter_epg) and O(filter_l3out) are not configurable when O(span_drop_packets=true).

akinross · 2025-06-27T08:30:05Z

plugins/modules/ndo_fabric_span_session_source.py

+      uuid:
+        description:
+        - The UUID of the EPG used to configure the Filter EPG.
+        - This parameter or O(filter_epg.epg_reference) is required.


I personally do not like this form. The original comment was to avoid double usage of a object naming in nesting of dictionaries. Originally you proposed for name filter_epg.epg.name and now this becomes filter_epg.epg_reference.name where we already now that the reference is for a epg. I proposed filter_epg.reference.name, to avoid this double usage of epg. In my case it would become virtual_port_channel.virtual_port_channel_reference.name. I think if we prefer epg_reference we might as well use epg.

I understand that the team is not a fan of the reference wording (which is/was used in MSO/NDO to point to objects within a schema). Thus far I proposed reference, object, instance which were all not fitting, but I do not see any alternatives. @anvitha-jain, @samiib, @shrsr, @gmicol, @lhercot, can you provide other proposals or your preference for this to @sajagana?

I am happy with using reference. I see how it can become quite verbose when the object name is long eg. virtual_port_channel.

Yes i also think alias can help to shorten this even further, alias ref for reference for instance and vpc for virtual_port_channel.

This way virtual_port_channel.reference.name could become vpc.ref.name

I'm good with reference as well!

Based on the discussion, "reference" was chosen as the primary attribute name, with "ref" used as its alias.

…ssion sources

…ess_paths logic

…n the ndo_fabric_span_session_source module

…est file

…nce in ndo_fabric_span_session_source module

…e with reference

akinross

LGTM

shrsr · 2025-07-02T15:25:12Z

plugins/modules/ndo_fabric_span_session_source_filter.py

+  source_port_from:
+    description:
+    - The starting source port number for the SPAN Session source filter.
+    - This parameter is required to query/delete a specific SPAN session source filter when it is configured with a non-default value.


Does this mean when it is not configured with "unspecified"?

Yes, some attributes default to "unspecified" if not provided during creation, update, query, or deletion. If a filter is created without a default value, I need the non-default value to query or delete that specific object.

Shall we be more specific then saying when not configured with "unspecified"?

shrsr · 2025-07-02T15:25:28Z

plugins/modules/ndo_fabric_span_session_source_filter.py

+  source_port_to:
+    description:
+    - The ending source port number for the SPAN Session source filter.
+    - This parameter is required to query/delete a specific SPAN session source filter when it is configured with a non-default value.


Same question as above?

Yes, some attributes default to "unspecified" if not provided during creation, update, query, or deletion. If a filter is created without a default value, I need the non-default value to query or delete that specific object.

shrsr

LGTM

shrsr

LGTM

akinross

LGTM

samiib · 2025-07-07T05:57:54Z

plugins/module_utils/mso.py

@@ -1891,8 +1891,8 @@ def listener_rules_spec():
    )


-def epg_object_reference_spec():
-    return dict(
+def epg_object_reference_spec(aliases=None):


Should aliases here be alias as its currently only defined as a single string? Or should this be list of strings and the below code would be: epg_reference_spec["aliases"] = aliases

…o list of strings

…manage the SPAN session source filter

…emplate functions

…mplate.py to ndo_fabric_span_session_source_filter.py

…umentation to query/delete a specific object using default value

akinross

LGTM

samiib

LGTM

shrsr

LGTM

gmicol

LGTM

anvitha-jain

LGTM

lhercot

LGTM

sajagana requested review from akinross, anvitha-jain, gmicol, lhercot, samiib and shrsr as code owners May 29, 2025 16:25

sajagana force-pushed the 376_fabric_span_session_source_filters branch from 16a41fc to 37bc01c Compare May 29, 2025 16:26

sajagana force-pushed the 376_fabric_span_session_source_filters branch from 37bc01c to d04bc8e Compare June 18, 2025 11:17

akinross requested changes Jun 20, 2025

View reviewed changes

sajagana requested a review from akinross June 20, 2025 13:51

anvitha-jain requested changes Jun 20, 2025

View reviewed changes

akinross requested changes Jun 21, 2025

View reviewed changes

sajagana force-pushed the 376_fabric_span_session_source_filters branch 3 times, most recently from a45edda to 7ce2355 Compare June 27, 2025 07:34

sajagana requested review from akinross and anvitha-jain June 27, 2025 07:45

akinross requested changes Jun 27, 2025

View reviewed changes

sajagana added 8 commits July 1, 2025 10:59

[minor_change] Added ndo_fabric_span_session_source to manage SPAN se…

e77233e

…ssion sources

[ignore] Added L3Out object details in the output and changed the acc…

fe1b394

…ess_paths logic

[ignore] Removed Filter L3Out argument spec validate function

bcd1bc4

[ignore] Changed the Filter L3Out input structure

611abe8

[ignore] Removed enabled attribute from filter_l3out and filter_epg i…

f2fb03b

…n the ndo_fabric_span_session_source module

[ignore] Updated ndo_fabric_span_session_source module document and t…

dedeefe

…est file

[ignore] Changed the nested epg, l3out to epg_reference, l3out_refere…

be6667f

…nce in ndo_fabric_span_session_source module

[ignore] Replaced the attribute name l3out_reference and epg_referenc…

dc914c0

…e with reference

sajagana force-pushed the 376_fabric_span_session_source_filters branch from 7ce2355 to f7b6f74 Compare July 1, 2025 05:48

sajagana requested a review from akinross July 1, 2025 05:58

akinross previously approved these changes Jul 2, 2025

View reviewed changes

shrsr requested changes Jul 2, 2025

View reviewed changes

sajagana requested a review from shrsr July 3, 2025 07:10

sajagana dismissed akinross’s stale review via 3b654ac July 3, 2025 16:06

sajagana requested a review from akinross July 3, 2025 16:06

shrsr approved these changes Jul 3, 2025

View reviewed changes

shrsr previously approved these changes Jul 3, 2025

View reviewed changes

akinross previously approved these changes Jul 4, 2025

View reviewed changes

samiib requested changes Jul 7, 2025

View reviewed changes

sajagana added 5 commits July 7, 2025 13:50

[ignore] Changed the epg_object_reference_spec function input value t…

c2a5846

…o list of strings

[minor_change] Added ndo_fabric_span_session_source_filter module to …

b593777

…manage the SPAN session source filter

[ignore] Removed the plural form words from the fabric_span_session t…

2e39b66

…emplate functions

[ignore] Moved get_fabric_span_session_source_filter function from te…

c437dd6

…mplate.py to ndo_fabric_span_session_source_filter.py

[ignore] Changed the ndo_fabric_span_session_source_filter module doc…

131c14f

…umentation to query/delete a specific object using default value

sajagana dismissed stale reviews from akinross and shrsr via 131c14f July 7, 2025 08:22

sajagana force-pushed the 376_fabric_span_session_source_filters branch from 3b654ac to 131c14f Compare July 7, 2025 08:22

sajagana requested review from samiib, shrsr and akinross July 7, 2025 08:22

akinross approved these changes Jul 7, 2025

View reviewed changes

samiib approved these changes Jul 7, 2025

View reviewed changes

shrsr approved these changes Jul 7, 2025

View reviewed changes

gmicol approved these changes Jul 7, 2025

View reviewed changes

anvitha-jain approved these changes Jul 10, 2025

View reviewed changes

lhercot approved these changes Jul 11, 2025

View reviewed changes

lhercot merged commit 6b9504c into CiscoDevNet:master Jul 11, 2025
22 of 23 checks passed

sajagana mentioned this pull request Jul 14, 2025

Added ndo_fabric_span_session_source to manage SPAN session sources (DCNE-376) #675

Closed

	return self.get_object_by_key_value_pairs("SPAN Sessions", existing_objects, [KVPair("uuid", uuid) if uuid else KVPair("name", name)], fail_module)
	return self.get_object_by_key_value_pairs("SPAN Session", existing_objects, [KVPair("uuid", uuid) if uuid else KVPair("name", name)], fail_module)

	- The O(filter_epg) and O(filter_l3out) is not configurable when this parameter O(span_drop_packets=true) set to true.
	- The O(filter_epg) and O(filter_l3out) are not configurable when O(span_drop_packets=true).

Added ndo_fabric_span_session_source_filter module to manage the SPAN session source filter (DCNE-376) #677

Added ndo_fabric_span_session_source_filter module to manage the SPAN session source filter (DCNE-376) #677

Uh oh!

Conversation

sajagana commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinross Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinross Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinross left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shrsr left a comment

Choose a reason for hiding this comment

Uh oh!

shrsr left a comment

Choose a reason for hiding this comment

Uh oh!

akinross left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akinross left a comment

sajagana commented May 29, 2025 •

edited

Loading

akinross Jun 21, 2025 •

edited

Loading

akinross Jun 27, 2025 •

edited

Loading