Skip to content

Commit

Permalink
Fix VRRP regression failures (#455)
Browse files Browse the repository at this point in the history 
* Fix vrrp regression failure and standard interface naming changes in update url

* Fix ansible-lint failures

* Add changelog fragment
  • Loading branch information
@santhosh-kt
santhosh-kt authored Sep 26, 2024
1 parent 6eafbcd commit 3aa8ae5
Show file tree
Hide file tree
Showing 11 changed files with 469 additions and 462 deletions.
4 changes: 4 additions & 0 deletions changelogs/fragments/455-fix-vrrp-regression-failure.yaml
View file
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
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
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

0 comments on commit 3aa8ae5

Please sign in to comment.