Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix VRRP regression failures #455

Merged
merged 4 commits into from
Sep 26, 2024
Merged

Fix VRRP regression failures #455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b7bf90f
Fix vrrp regression failure and standard interface naming changes in …
santhosh-kt Sep 25, 2024
bd8aa00
Fix ansible-lint failures
santhosh-kt Sep 25, 2024
9596929
Add changelog fragment
santhosh-kt Sep 25, 2024
24d3d53
Merge branch 'ansible-collections:main' into vrrp_bug_fix
santhosh-kt Sep 26, 2024
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
4 changes: 4 additions & 0 deletions changelogs/fragments/455-fix-vrrp-regression-failure.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
bugfixes:
- sonic_vrrp - Update delete handling to fix regression failure (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/455).
- Update 'update_url' method to handle multiple interface names (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/455).
58 changes: 29 additions & 29 deletions plugins/module_utils/network/sonic/config/vrrp/vrrp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,12 +388,18 @@ def _get_replaced_overridden_config(self, want, have, state):
match_group = next((g for g in want_groups if g['virtual_router_id'] == vr_id and g['afi'] == afi), None)
if not match_group:
del_cfg.setdefault('group', []).append({'virtual_router_id': vr_id, 'afi': afi})
commands, requests = self.get_delete_vrrp_group_command_request(name, vr_id, afi)
commands, requests = self.get_delete_vrrp_group_command_request(name, group)
if commands:
del_requests.extend(requests)
if del_cfg:
del_cfg['name'] = name
del_config.append(del_cfg)
if not any(cfg.get('name') == name for cfg in del_config):
del_cfg['name'] = name
del_config.append(del_cfg)
else:
for cfg in del_config:
if cfg['name'] == name:
cfg['group'].append(del_cfg)
break
return add_config, del_config, del_requests

def get_create_vrrp_requests(self, commands):
Expand All @@ -407,15 +413,14 @@ def get_create_vrrp_requests(self, commands):

for cmd in commands:
name = cmd.get('name', None)
intf_name = name.replace('/', '%2f')
group_list = cmd.get('group', [])
if group_list:
for group in group_list:
virtual_router_id = group.get('virtual_router_id', None)
if 'Vlan' in intf_name:
keypath = self.vrrp_vlan_path.format(intf_name=intf_name)
if 'Vlan' in name:
keypath = self.vrrp_vlan_path.format(intf_name=name)
else:
parent_intf, sub_intf = intf_name.split('.') if '.' in intf_name else (intf_name, 0)
parent_intf, sub_intf = name.split('.') if '.' in name else (name, 0)
keypath = self.vrrp_intf_path.format(intf_name=parent_intf, intf_index=sub_intf)
requests.extend(self.get_create_specific_vrrp_param_requests(virtual_router_id, group, keypath))

Expand Down Expand Up @@ -503,14 +508,13 @@ def get_delete_vrrp_commands_requests(self, want, have, is_delete_all):
for cmd in want:
del_cmd = {}
name = cmd.get('name')
intf_name = name.replace('/', '%2f')
match_have = next((cnf for cnf in have if cnf['name'] == name), None)
group_list = [] if cmd.get('group') is None else cmd['group']

if is_delete_all:
if match_have:
if match_have.get('group'):
del_group, request = self.get_delete_all_vrrp_groups_commands_requests(match_have['group'], intf_name)
del_group, request = self.get_delete_all_vrrp_groups_commands_requests(match_have['group'], name)
if del_group:
commands.append({'name': name})
requests.extend(request)
Expand All @@ -525,18 +529,18 @@ def get_delete_vrrp_commands_requests(self, want, have, is_delete_all):
match_group = next((g for g in match_group_list if g['virtual_router_id'] == virtual_router_id and g['afi'] == afi), None)
if match_group:
del_group = None
if len(match_group.keys()) == 2:
del_group, request = self.get_delete_vrrp_group_command_request(intf_name, virtual_router_id, afi)
if len(group.keys()) == 2:
del_group, request = self.get_delete_vrrp_group_command_request(name, group)
else:
del_group, request = self.get_delete_specific_vrrp_param_commands_requests(match_group, virtual_router_id, group, intf_name)
del_group, request = self.get_delete_specific_vrrp_param_commands_requests(match_group, virtual_router_id, group, name)

if del_group:
del_groups.append(del_group)
requests.extend(request)
if del_groups:
del_cmd['group'] = del_groups
elif match_group_list:
del_group, request = self.get_delete_all_vrrp_groups_commands_requests(match_group_list, intf_name)
del_group, request = self.get_delete_all_vrrp_groups_commands_requests(match_group_list, name)
if del_group:
commands.append({'name': name})
requests.extend(request)
Expand All @@ -551,11 +555,10 @@ def get_delete_all_vrrp_groups_commands_requests(self, groups, intf_name):
groups = [] if groups is None else groups
for group in groups:
virtual_router_id = group.get('virtual_router_id')
afi = group.get('afi')
# VRRP with VRRP ID 1 can be removed only if other VRRP
# groups are removed first
# Hence the check
command, request = self.get_delete_vrrp_group_command_request(intf_name, virtual_router_id, afi)
command, request = self.get_delete_vrrp_group_command_request(intf_name, group)
if command:
commands.append(command)
if virtual_router_id == 1:
Expand All @@ -567,11 +570,13 @@ def get_delete_all_vrrp_groups_commands_requests(self, groups, intf_name):

return commands, requests

def get_delete_vrrp_group_command_request(self, intf_name, virtual_router_id, afi):
def get_delete_vrrp_group_command_request(self, intf_name, group):
""" Get list of requests to delete the entire VRRP and VRRP6 group configurations
based on the specified interface
"""
command, request = [], []
virtual_router_id = group.get('virtual_router_id')
afi = group.get('afi')
if not virtual_router_id or not afi:
return command, request

Expand All @@ -582,6 +587,14 @@ def get_delete_vrrp_group_command_request(self, intf_name, virtual_router_id, af
parent_intf, sub_intf = intf_name.split('.') if '.' in intf_name else (intf_name, 0)
keypath = self.vrrp_intf_path.format(intf_name=parent_intf, intf_index=sub_intf)
url = '{0}{1}vrrp/vrrp-group={2}'.format(keypath, ip_path, virtual_router_id)

track_interfaces = self.get_track_interfaces(group.get('track_interface'))

for track_intf in track_interfaces:
if track_intf.get('interface'):
track_url = url + "/openconfig-interfaces-ext:vrrp-track/vrrp-track-interface={0}".format(track_intf.get('interface'))
request.append({'path': track_url, 'method': DELETE})

command = {'virtual_router_id': virtual_router_id, 'afi': afi}
request.append({'path': url, 'method': DELETE})

Expand All @@ -595,12 +608,7 @@ def get_delete_specific_vrrp_param_commands_requests(self, cfg_group, virtual_ro

afi = group['afi']
vip_addresses = self.get_vip_addresses(group.get('virtual_address'))
preempt = group.get('preempt')
ip_path = IPV4_PATH if afi == 'ipv4' else IPV6_PATH
adv_interval = group.get('advertisement_interval')
priority = group.get('priority')
version = group.get('version')
use_v2_checksum = group.get('use_v2_checksum')
track_interfaces = self.get_track_interfaces(group.get('track_interface'))

if not virtual_router_id or not afi:
Expand All @@ -615,12 +623,6 @@ def get_delete_specific_vrrp_param_commands_requests(self, cfg_group, virtual_ro
parent_intf, sub_intf = intf_name.split('.') if '.' in intf_name else (intf_name, 0)
keypath = self.vrrp_intf_path.format(intf_name=parent_intf, intf_index=sub_intf)

if not (vip_addresses or preempt or adv_interval or priority or version or use_v2_checksum or track_interfaces):
url = keypath + ip_path + 'vrrp/vrrp-group={vrid}'.format(vrid=virtual_router_id)
commands = {'virtual_router_id': virtual_router_id, 'afi': afi}
requests.append({'path': url, 'method': DELETE})
return commands, requests

if vip_addresses and cfg_vip_addresses:
del_vip_list = []
for addr in set(vip_addresses).intersection(set(cfg_vip_addresses)):
Expand All @@ -639,10 +641,8 @@ def get_delete_specific_vrrp_param_commands_requests(self, cfg_group, virtual_ro
del_track_list = []
for track in track_interfaces:
interface = track['interface']
interface = interface.replace('/', '%2f')
for cfg_track in cfg_track_interfaces:
cfg_interface = cfg_track['interface']
cfg_interface = cfg_interface.replace('/', '%2f')
if interface == cfg_interface:
track_url = self.vrrp_config_path['track_interface'].format(vrid=virtual_router_id) + '/vrrp-track-interface=' + interface
requests.append({'path': keypath + ip_path + track_url, 'method': DELETE})
Expand Down
12 changes: 6 additions & 6 deletions plugins/module_utils/network/sonic/sonic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@
from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.config import NetworkConfig, ConfigLine

_DEVICE_CONFIGS = {}
STANDARD_ETH_REGEXP = r"Eth\d+(/\d+)+"
PATTERN = re.compile(STANDARD_ETH_REGEXP)
STANDARD_ETH_REGEXP = r"(Eth\d+(/\d+)+)"


def get_connection(module):
Expand Down Expand Up @@ -158,12 +157,13 @@ def edit_config_reboot(module, commands, skip_code=None):


def update_url(url):
match = re.search(STANDARD_ETH_REGEXP, url)
match = re.findall(STANDARD_ETH_REGEXP, url)
ret_url = url
if match:
interface_name = match.group()
interface_name = interface_name.replace("/", "%2f")
ret_url = PATTERN.sub(interface_name, url)
for item in match:
interface_name = item[0]
update_interface_name = interface_name.replace("/", "%2f")
ret_url = ret_url.replace(interface_name, update_interface_name)
return ret_url


Expand Down
Loading
Loading