Skip to content

Commit

Permalink
Modify SONiC 4.4.0 IPv6 feature support code to eliminate failures on…
Browse files Browse the repository at this point in the history 
… older versions of SONiC. (#474)

* Modify changes for support of SONiC 4.4.0 IPv6 features to eliminate failures on older versions of SONiC.

* Add a changelog fragment file for this PR.

* Remove UT expectation for sending of the unnecessary and downward incompatible default autoconf REST api.

* Fix sanity errors.

* Fix idempotency problems for defaults in overridden and replaced state.

* Add missing changes to filtered "want" for "replaced" state.

* Fix a sanity error.

* Update plugins/module_utils/network/sonic/config/l3_interfaces/l3_interfaces.py

In 'replaced' state, remove empties before removing default entries to prevent extraneous diffs.

Co-authored-by: Arun Saravanan Balachandran <52521751+ArunSaravananBalachandran@users.noreply.github.com>

* Update plugins/module_utils/network/sonic/config/l3_interfaces/l3_interfaces.py

Use the "unfiltered" input "want" dictionary instead of the filtered "want" with defaults removed when determining which interfaces need to have their configuration "replaced".

This change allows "replaced" handling to proceed  on interfaces for which the "unfiltered" want includes only default values.

Co-authored-by: Arun Saravanan Balachandran <52521751+ArunSaravananBalachandran@users.noreply.github.com>

---------

Co-authored-by: Arun Saravanan Balachandran <52521751+ArunSaravananBalachandran@users.noreply.github.com>
  • Loading branch information
@kerry-meyer @ArunSaravananBalachandran
kerry-meyer and ArunSaravananBalachandran authored Oct 28, 2024
1 parent 06e9d99 commit 7b4b6f8
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 95 deletions.
3 changes: 3 additions & 0 deletions changelogs/fragments/474-l3_interfaces-ipv6-autoconf-downward-compatibility.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
bugfixes:
- sonic_l3_interfaces - Eliminate unconditional sending of the new autoconf REST API option during replaced and overridden state handling (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/474).
159 changes: 82 additions & 77 deletions plugins/module_utils/network/sonic/config/l3_interfaces/l3_interfaces.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,21 +114,21 @@ def execute_module(self):
existing_l3_intf_facts = remove_empties_from_list(existing_l3_interfaces_facts)
cmmnds = remove_empties_from_list(commands)

old_config = self.remove_default_entries(existing_l3_intf_facts, False)
cmds = self.remove_default_entries(cmmnds, True)
old_config = self.remove_default_entries(existing_l3_intf_facts)
cmds = self.remove_default_entries(cmmnds)

new_config = get_new_config(cmds, old_config,
TEST_KEYS_formatted_diff)
new_config = remove_empties_from_list(new_config)
new_config = self.remove_default_entries(new_config, False)
new_config = self.remove_default_entries(new_config)
result['after(generated)'] = new_config

if self._module._diff:
old_conf = remove_empties_from_list(old_config)
old_conf = self.remove_default_entries(old_conf, False)
old_conf = self.remove_default_entries(old_conf)

new_conf = remove_empties_from_list(new_config)
new_conf = self.remove_default_entries(new_conf, False)
new_conf = self.remove_default_entries(new_conf)

self.sort_lists_in_config(old_conf)
self.sort_lists_in_config(new_conf)
Expand Down Expand Up @@ -183,8 +183,10 @@ def _state_replaced(self, want, have):
"""
ret_requests = list()
commands = list()
new_want = self.update_object(want)
new_have = self.remove_default_entries(have, False)
new_have = remove_empties_from_list(have)
new_have = self.remove_default_entries(new_have)
new_want = remove_empties_from_list(want)
new_want = self.remove_default_entries(new_want)
get_replace_interfaces_list = self.get_interface_object_for_replaced(new_have, want)

diff_del = get_diff(get_replace_interfaces_list, new_want, TEST_KEYS)
Expand All @@ -194,9 +196,9 @@ def _state_replaced(self, want, have):
delete_l3_interfaces_requests = self.get_delete_all_requests(diff_del)
ret_requests.extend(delete_l3_interfaces_requests)
commands.extend(update_states(diff_del, "deleted"))
l3_interfaces_to_create_requests = self.get_create_l3_interfaces_requests(want)
l3_interfaces_to_create_requests = self.get_create_l3_interfaces_requests(new_want)
ret_requests.extend(l3_interfaces_to_create_requests)
commands.extend(update_states(want, "replaced"))
commands.extend(update_states(new_want, "replaced"))
elif diff_add:
l3_interfaces_to_create_requests = self.get_create_l3_interfaces_requests(diff_add)
ret_requests.extend(l3_interfaces_to_create_requests)
Expand All @@ -212,21 +214,19 @@ def _state_overridden(self, want, have):
"""
commands = list()
ret_requests = list()
commands = list()
new_want = self.update_object(want)
new_want = remove_empties_from_list(new_want)
new_want = self.remove_default_entries(new_want, True)
new_want = remove_empties_from_list(want)
new_want = self.remove_default_entries(new_want)
new_have = remove_empties_from_list(have)
new_have = self.remove_default_entries(new_have, False)
new_have = self.remove_default_entries(new_have)
get_override_interfaces = self.get_interface_object_for_overridden(new_have)
diff = get_diff(get_override_interfaces, new_want, TEST_KEYS)
diff2 = get_diff(new_want, get_override_interfaces, TEST_KEYS)

if diff or diff2:
delete_interfaces_requests = self.get_delete_all_requests(have)
delete_interfaces_requests = self.get_delete_all_requests(new_have)
ret_requests.extend(delete_interfaces_requests)
commands.extend(update_states(diff, "deleted"))
interfaces_to_create_requests = self.get_create_l3_interfaces_requests(want)
interfaces_to_create_requests = self.get_create_l3_interfaces_requests(new_want)
ret_requests.extend(interfaces_to_create_requests)
commands.extend(update_states(want, "overridden"))

Expand Down Expand Up @@ -258,62 +258,85 @@ def _state_deleted(self, want, have):
"""
commands = list()
if not want:
commands = have
commands = self.remove_default_entries(have)
requests = self.get_delete_all_completely_requests(commands)
else:
commands = want
requests = self.get_delete_l3_interfaces_requests(commands, have)
commands = self.remove_default_entries(want)
filtered_have = self.remove_default_entries(have)
requests = self.get_delete_l3_interfaces_requests(commands, filtered_have)
if len(requests) == 0:
commands = []
if commands:
commands = update_states(commands, "deleted")
return commands, requests

def remove_default_entries(self, config, input_cmds):
def remove_default_entries(self, config):
new_config = list()
state = self._module.params['state']
for obj in config:
new_obj = dict()
if obj.get('ipv4', None) and \
(obj['ipv4'].get('addresses', None) or
obj['ipv4'].get('anycast_addresses', None)):
obj['ipv4'].get('anycast_addresses', None) or
state == 'deleted'):
new_obj['ipv4'] = obj['ipv4']
if obj.get('ipv6', None) and \
(obj['ipv6'].get('addresses', None) or
obj['ipv6'].get('dad', None) or
obj['ipv6'].get('autoconf', None) is not None or
obj['ipv6'].get('enabled', None) is not None):
obj['ipv6'].get('enabled', None) is not None or
state == 'deleted'):

new_obj['ipv6'] = obj['ipv6'].copy()
if new_obj['ipv6'].get('dad', None) == "DISABLE":

# Because 'dad' is shown in the device IPv6 config as "DISABLE" when
# 'dad' configuration has been deleted, enable correct handling
# for all states by filtering out 'dad' == "DISABLE" unless one of the
# following conditions is true:
#
# (1) The 'config' parameter to this function represents input commands
# (from the executing user playbook) and the specified target state is a
# value other than 'deleted' ("positive" configuration). This is to
# enable idempotent handling for 'deleted' state when 'deleting' a 'dad'
# value of 'DISABLE' (a no-op that should not generate a request), while
# preserving the ability to 'merge' a 'dad' value of 'DISABLE'
# when that is requested by a playbook state of 'merged', 'overridden',
# or 'replaced'.
#
# or
#
# (2) The 'config' parameter to this function does not represent input
# commands (because it is from the current device configuration) and the
# specified target state is 'merged'. (This exclusion is to enable
# idempotent handling for 'merged' state when 'merging' a 'dad' value of
# 'DISABLE'.)
if ((input_cmds and state == "deleted") or
((not input_cmds) and state != "merged")):

# The following options have defult values in the device IPv6
# configuration when they have been "deleted":
#
# enable => False,
# dad => "DISABLE",
# autoconf => False
#
# Enable correct handling for all states by filtering out these
# options when they have default values unless the target state
# for the currently executing playbook task is "merged" state.
# This is to enable idempotent handling for all states given the
# following coniderations:
#
# - In 'merged' state, the input playbook value and the configured value
# of each of these "defaulted" options is needed to enable the
# correct "diff" calculation for the changes to be applied to the
# device.
# - For 'deleted' state, the "deletion" of any of the "defaulted" options
# is a no-op. If any of these options currently has the "default" value
# configured, it is already "deleted" and no further action is needed to
# execute a request to "delete" the default value. If it is configured to
# a value other than the default value, then the request to "delete" the
# default value has no effect. Options having the "default" value should
# therefore be deleted from both the input playbook and the device
# configuration to be used for processing the playbook task.
# - For 'replaced' and 'overridden' states, absence of an option in the
# input playbook task causes deletion of that option from the device
# configuration if it has a non-default value. For the "defaulted" options,
# this is the same as configuring the option to the default value. For
# this reason, removal of a default value option from the input playbook
# task and also from the "filtered" device configuration results in the
# desired end result and allows correct idempotent handling for these options.
# This is because the states of these options in the input playbook
# task and filtered device configuration will match when the defaulted option
# is configured to the "default" value (or "deleted", which is the same thing
# for these options).
#
if state != "merged":
if new_obj['ipv6'].get('enabled', None) is False:
del new_obj['ipv6']['enabled']
if new_obj['ipv6'].get('dad', None) == "DISABLE":
del new_obj['ipv6']['dad']
if new_obj['ipv6'] == {}:
del new_obj['ipv6']
if new_obj['ipv6'].get('autoconf', None) is False:
del new_obj['ipv6']['autoconf']
if new_obj.get('ipv6', None) == {}:
del new_obj['ipv6']

if new_obj:
if new_obj or state == "deleted":
key_set = set(obj.keys())
key_set.discard('ipv4')
key_set.discard('ipv6')
Expand All @@ -332,27 +355,6 @@ def get_interface_object_for_replaced(self, have, want):
objects.append(obj.copy())
return objects

def update_object(self, want):
objects = list()
for obj in want:
new_obj = {}
if 'name' in obj:
new_obj['name'] = obj['name']
if obj['ipv4'] is None:
new_obj['ipv4'] = {'addresses': None, 'anycast_addresses': None}
else:
new_obj['ipv4'] = obj['ipv4']

if obj['ipv6'] is None:
new_obj['ipv6'] = {'addresses': None, 'enabled': False, 'autoconf': False, 'dad': None}
else:
new_obj['ipv6'] = obj['ipv6']
if new_obj['ipv6'].get('autoconf') is None:
new_obj['ipv6']['autoconf'] = False

objects.append(new_obj)
return objects

def get_interface_object_for_overridden(self, have):
objects = list()
for obj in have:
Expand Down Expand Up @@ -446,10 +448,12 @@ def get_delete_l3_interfaces_requests(self, want, have):
if name and ipv4 is None and ipv6 is None:
is_del_ipv4 = True
is_del_ipv6 = True
elif ipv4 and not ipv4.get('addresses') and not ipv4.get('anycast_addresses'):
is_del_ipv4 = True
elif ipv6 and not ipv6.get('addresses') and ipv6.get('enabled') is None and ipv6.get('autoconf') is None and ipv6.get('dad') is None:
is_del_ipv6 = True
else:
if ipv4 and not ipv4.get('addresses') and not ipv4.get('anycast_addresses'):
is_del_ipv4 = True
if (ipv6 and not ipv6.get('addresses') and ipv6.get('enabled') is None and
ipv6.get('autoconf') is None and ipv6.get('dad') is None):
is_del_ipv6 = True

if is_del_ipv4:
if have_ipv4_addrs and len(have_ipv4_addrs) != 0:
Expand All @@ -458,7 +462,8 @@ def get_delete_l3_interfaces_requests(self, want, have):
if have_ipv4_anycast_addrs and len(have_ipv4_anycast_addrs) != 0:
for ip in have_ipv4_anycast_addrs:
ip = ip.replace('/', '%2f')
anycast_delete_request = {"path": ipv4_anycast_url.format(intf_name=name, sub_intf_name=sub_intf, anycast_ip=ip), "method": DELETE}
anycast_delete_request = {"path": ipv4_anycast_url.format(intf_name=name, sub_intf_name=sub_intf, anycast_ip=ip),
"method": DELETE}
requests.append(anycast_delete_request)
else:
ipv4_addrs = []
Expand Down Expand Up @@ -583,7 +588,7 @@ def get_delete_all_requests(self, configs):
ipv6_enabled = l3['ipv6']['enabled']
if 'autoconf' in l3['ipv6']:
ipv6_autoconf = l3['ipv6']['autoconf']
if 'dad' in l3['ipv6'] and l3['ipv6']['dad'] is not None and l3['ipv6']['dad'] != "DISABLE":
if 'dad' in l3['ipv6']:
ipv6_dad = l3['ipv6']['dad']

sub_intf = self.get_sub_interface_name(name)
Expand Down
7 changes: 6 additions & 1 deletion tests/regression/roles/sonic_l3_interfaces/defaults/main.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ tests:
addresses:
- address: 2041::1/128
- address: 2042::1/128
autoconf: True
- name: PortChannel101
ipv4:
addresses:
Expand Down Expand Up @@ -296,7 +297,8 @@ tests:
addresses:
- address: 105.2.2.2/16
ipv6:
enabled: true
enabled: False
autoconf: False
dad: DISABLE_IPV6_ON_FAILURE
addresses:
- address: 1050::/64
Expand Down Expand Up @@ -336,6 +338,9 @@ tests:
- address: 152.1.1.1/32
- address: 153.1.1.1/32
secondary: true
ipv6:
autoconf: False
enabled: False
- name: vlan 100
ipv4:
anycast_addresses:
Expand Down
21 changes: 4 additions & 17 deletions tests/unit/modules/network/sonic/fixtures/sonic_l3_interfaces.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ replaced_01:
anycast_addresses:
- 11.12.13.14/12
ipv6:
enabled: true
autoconf: true
enabled: false
autoconf: false
dad: DISABLE_IPV6_ON_FAILURE
- name: Eth1/1
ipv4:
Expand Down Expand Up @@ -521,16 +521,6 @@ replaced_01:
data:
openconfig-interfaces-ext:static-anycast-gateway:
- 11.12.13.14/12
- path: "data/openconfig-interfaces:interfaces/interface=Vlan13/openconfig-vlan:routed-vlan/openconfig-if-ip:ipv6/config"
method: "patch"
data:
config:
enabled: True
- path: "data/openconfig-interfaces:interfaces/interface=Vlan13/openconfig-vlan:routed-vlan/openconfig-if-ip:ipv6/config"
method: "patch"
data:
config:
ipv6_autoconfig: True
- path: "data/openconfig-interfaces:interfaces/interface=Vlan13/openconfig-vlan:routed-vlan/openconfig-if-ip:ipv6/config"
method: "patch"
data:
Expand Down Expand Up @@ -561,6 +551,8 @@ overridden_01:
eui64: True
- address: 31::1/64
- address: 32::1/64
enabled: False
autoconf: False
existing_l3_interfaces_config:
- path: "data/openconfig-interfaces:interfaces/interface"
response:
Expand Down Expand Up @@ -680,11 +672,6 @@ overridden_01:
openconfig-if-ip:config:
ip: 32::1
prefix-length: 64.0
- path: "data/openconfig-interfaces:interfaces/interface=Eth1%2f1/subinterfaces/subinterface=0/openconfig-if-ip:ipv6/config"
method: "patch"
data:
config:
ipv6_autoconfig: False
- path: "data/openconfig-interfaces:interfaces/interface=Vlan13/openconfig-vlan:routed-vlan/openconfig-if-ip:ipv4/openconfig-interfaces-ext:sag-ipv4/config/static-anycast-gateway"
method: "patch"
data:
Expand Down

0 comments on commit 7b4b6f8

Please sign in to comment.