Skip to content

Fix issue with breakout with multiple switch #484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 77 additions & 52 deletions plugins/modules/dcnm_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -1854,8 +1854,8 @@ def __init__(self, module):
self.diff_replace = []
self.diff_delete = [[], [], [], [], [], [], [], [], []]
self.diff_delete_deploy = [[], [], [], [], [], [], [], [], []]
self.breakout = []
self.want_breakout = []
self.have_breakout = []
self.diff_create_breakout = []
self.diff_delete_breakout = []
self.diff_deploy = []
Expand Down Expand Up @@ -2065,11 +2065,6 @@ def __init__(self, module):
"BREAKOUT": 8,
}

# Build breakout list in config
for interface in self.config:
if interface.get("type") and interface["type"] == "breakout":
self.breakout.append(interface["name"])

msg = "ENTERED DcnmIntf: "
self.log.debug(msg)

Expand All @@ -2087,19 +2082,32 @@ def dcnm_intf_breakout_format(self, if_name):
# Return False and None if the pattern does not match
return False, None

def dcnm_intf_get_parent(self, cfg, name):
# Extract the parent interface ID (e.g., "1/2" from "1/2/3")
def dcnm_intf_get_parent(self, name, switch):
"""
Given an interface name and switch serial number, determine if the parent breakout interface exists in the target config.

Args:
cfg (list): List of interface configuration dictionaries.
name (str): Interface name to check (e.g., "Ethernet1/100/1").
switch (str): Switch Mgmt IP Address.

Returns:
tuple: (True, type) if parent breakout interface exists, else (False, None).
"""
# Extract the parent interface ID (e.g., "1/100" from "Ethernet1/100/1")
match = re.search(r"(\d+/\d+)/\d+", name)
if not match:
return False, None # Return early if the pattern doesn't match

parent_port_id = f"ethernet{match.group(1)}"

# Search for the parent interface in the configuration
for interface in cfg:
if interface.get("name").lower() == parent_port_id:
parent_intf = f"Ethernet{match.group(1)}"
# Check if the parent interface exists in the config for the given switch
for interface in self.config:
if (
interface["name"].lower() == parent_intf.lower()
and interface.get("type", "").lower() == "breakout"
and interface["switch"][0] == switch
):
return True, interface.get("type", None)

return False, None

def dcnm_intf_dump_have_all(self):
Expand Down Expand Up @@ -3798,11 +3806,13 @@ def dcnm_intf_get_want(self):
# Process non-breakout_interface policies
if intf_payload not in self.want:
intf = intf_payload["interfaces"][0]["ifName"]
is_valid_format, formatted_interface = self.dcnm_intf_breakout_format(intf)

# Add to self.want only if the interface does not match invalid breakout conditions
if not (is_valid_format and formatted_interface not in self.breakout):

is_breakout_format, formatted_interface = self.dcnm_intf_breakout_format(intf)
parent_breakout_found, parent_type = self.dcnm_intf_get_parent(intf, sw)
# Add breakout interface to self.want only if breakout is configured in state overridden
# When state is replaced, we don't check if parent is configured.
if is_breakout_format is False or parent_breakout_found is True and self.params['state'] == "overridden":
self.want.append(intf_payload)
else:
self.want.append(intf_payload)

def dcnm_intf_get_intf_info(self, ifName, serialNumber, ifType):
Expand Down Expand Up @@ -3852,6 +3862,19 @@ def dcnm_intf_get_have_all_with_sno(self, sno):
if resp and "DATA" in resp and resp["DATA"]:
self.have_all.extend(resp["DATA"])

def dcnm_intf_get_have_all_breakout_interfaces(self, sno):
# This function will get policies for a given serial number and
# populate the breakout interfaces in self.have_breakout.
path = "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/control/policies/switches/{}".format(sno)
resp = dcnm_send(self.module, "GET", path)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccoueffe Can you get me the exact output that is saved to this resp variable? I will need this to fix the unit tests.


breakout = []
if resp and "DATA" in resp and resp["DATA"]:
for elem in resp["DATA"]:
if elem.get('templateName', None) == "breakout_interface":
breakout.append(elem['entityName'])
self.have_breakout.append({sno: breakout})

def dcnm_intf_get_have_all(self, sw):

# Check if you have already got the details for this switch
Expand All @@ -3868,6 +3891,7 @@ def dcnm_intf_get_have_all(self, sw):

self.have_all_list.append(sw)
self.dcnm_intf_get_have_all_with_sno(sno)
self.dcnm_intf_get_have_all_breakout_interfaces(sno)

def dcnm_intf_get_have(self):

Expand Down Expand Up @@ -4168,30 +4192,34 @@ def dcnm_intf_compare_want_and_have(self, state):
if state != "deleted":
for want_breakout in self.want_breakout:
want_intf = want_breakout["interfaces"][0]["ifName"]
want_serialnumber = want_breakout["interfaces"][0]["serialNumber"]
match_create = False
for have_intf in self.have_all:
is_valid, intf = self.dcnm_intf_breakout_format(have_intf['ifName'])
if is_valid and want_intf == intf:
match_create = True

# If interface in want_breakout and interface e1/x/y not present
# add to diff_create_breakout
# Search if interface is in have_breakout
for elem in self.have_breakout:
if want_serialnumber == list(elem.keys())[0]:
for interface in elem[want_serialnumber]:
if interface.lower() == want_intf.lower():
# If the breakout interface is already present in have,
# skip adding it to diff_create_breakout
match_create = True
break
if not match_create:
want_breakout["interfaces"][0].pop("fabricName")
want_breakout["interfaces"][0].pop("interfaceType")
self.diff_create_breakout.append(want_breakout["interfaces"])

if state == "deleted" or state == "overridden":
for have in self.have_all:
have_intf = have['ifName']
if re.search(r"\d+\/\d+\/\d+", have_intf):
found, parent_type = self.dcnm_intf_get_parent(self.config, have_intf)
# If have not in want breakout and if match to E1/x/1 add to dict
# Else if match E1/x/2, etc. silently ignore, because we delete the breakout
# with the first sub if.
if re.search(r"\d+\/\d+\/1$", have_intf) and not found:
payload = {'serialNumber': have['serialNo'],
'ifName': have['ifName']}
for have_breakout in self.have_breakout:
for interface in list(have_breakout.values())[0]:
match_delete_interface = True
for want_breakout in self.want_breakout:
if list(have_breakout.keys())[0] == want_breakout["interfaces"][0]["serialNumber"]:
if want_breakout["interfaces"][0]["ifName"] == interface:
match_delete_interface = False
break
if match_delete_interface:
payload = {
"serialNumber": list(have_breakout.keys())[0],
"ifName": interface + "/1"
}
self.diff_delete_breakout.append(payload)

for want in self.want:
Expand All @@ -4214,7 +4242,6 @@ def dcnm_intf_compare_want_and_have(self, state):
and (sno == d["interfaces"][0]["serialNumber"])
)
]

if not match_have:
changed_dict = copy.deepcopy(want)

Expand Down Expand Up @@ -4362,12 +4389,11 @@ def dcnm_intf_compare_want_and_have(self, state):
if action == "add":
# If E1/x/y do not create. Interface is created with breakout
if re.search(r"\d+\/\d+\/\d+", name):
found, parent_type = self.dcnm_intf_get_parent(self.config, name)
if found and parent_type == "breakout":
if want.get("interfaceType", None) is not None:
want.pop("interfaceType")
self.diff_replace.append(want)
self.changed_dict[0][state].append(changed_dict)
if want.get("interfaceType", None) is not None:
want.pop("interfaceType")
self.dcnm_intf_merge_intf_info(want, self.diff_replace)
self.changed_dict[0][state].append(changed_dict)
intf_changed = True
continue
self.dcnm_intf_merge_intf_info(want, self.diff_create)
# Add the changed_dict to self.changed_dict
Expand All @@ -4386,7 +4412,7 @@ def dcnm_intf_compare_want_and_have(self, state):
if str(deploy).lower() == "true":
# Add to diff_deploy,
# 1. if intf_changed is True
# 2. if intf_changed is Flase, then if 'complianceStatus is
# 2. if intf_changed is False, then if 'complianceStatus is
# False then add to diff_deploy.
# 3. Do not add otherwise

Expand Down Expand Up @@ -4753,8 +4779,7 @@ def dcnm_intf_get_diff_overridden(self, cfg):
# If a match for Ethernet1/x/y is found, verify if the parent interface Ethernet1/x exists in cfg.
# If the parent doesn't exist or isn't of type "breakout", remove the breakout interface.
if re.search(r"\d+\/\d+\/\d+", name):
found, parent_type = self.dcnm_intf_get_parent(cfg, name)

found, parent_type = self.dcnm_intf_get_parent(name, have['mgmtIpAddress'])
if not (found and parent_type == "breakout"):
payload = {
"serialNumber": have["serialNo"],
Expand Down Expand Up @@ -4962,7 +4987,7 @@ def dcnm_intf_get_diff_deleted(self):
for have in self.have_all:
have_intf = have['ifName']
if re.search(r"\d+\/\d+\/\d+", have_intf):
found, parent_type = self.dcnm_intf_get_parent(self.config, have_intf)
found, parent_type = self.dcnm_intf_get_parent(have_intf, have['mgmtIpAddress'])
# If have in want breakout and if match to E1/x/1 add to dict
# Else if match E1/x/2, etc. silently ignore, because we delete the breakout
# with the first sub if.
Expand Down Expand Up @@ -5420,7 +5445,7 @@ def dcnm_intf_send_message_to_dcnm(self):
# index 8 is used for breakout interface
if delete_index == 8:
path = "/appcenter/cisco/ndfc/api/v1/lan-fabric/rest/interface"

break
json_payload = json.dumps(delem)

resp = dcnm_send(self.module, "DELETE", path, json_payload)
Expand Down Expand Up @@ -5511,7 +5536,7 @@ def dcnm_intf_send_message_to_dcnm(self):
resp["CHANGED"] = self.changed_dict
self.module.fail_json(msg=resp)
else:
replace = True
create = True

# Update interfaces
path = self.paths["INTERFACE"]
Expand Down Expand Up @@ -5544,7 +5569,7 @@ def dcnm_intf_send_message_to_dcnm(self):
resp["CHANGED"] = self.changed_dict
self.module.fail_json(msg=resp)
else:
replace = True
delete = changed

resp = None
path = self.paths["GLOBAL_IF"]
Expand Down
Loading