Skip to content

Commit

Permalink
Enable system auditd and ip loadshare hash downward compatibility (#441)
Browse files Browse the repository at this point in the history
* Enable system auditd and ip loadshare hash downward compatibility.

* Add a fragment file.

* Add mock handling for edit_config_catch

* Fix sanity errors.

* Fix mistakes in the fragment file.

* Modify the fix to operation on individual requests in the enterprise_sonic httpapi plugin.

* Enable conditional suppression of GET exceptions.

* Fix handling for an empty GET response.

* Fix an indentation in the PoE facts file and fix global PoE empty configuration handling.
  • Loading branch information
kerry-meyer authored Aug 29, 2024
1 parent 8b9811a commit 973e80b
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
bugfixes:
- sonic_system - Catch the ConnectionError exception caused by unconditional fetching of auditd and ip loadshare hash algorithm configuration, and return empty configuration instead of allowing the uncaught exception to abort all "system" operations on SONiC images older than version 4.4.0 (https://github.com/ansible-collections/dellemc.enterprise_sonic/pull/441).
9 changes: 7 additions & 2 deletions plugins/httpapi/sonic.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@

import json
import time
import re

from ansible.module_utils._text import to_text
from ansible.module_utils.connection import ConnectionError
Expand Down Expand Up @@ -67,7 +68,7 @@ def send_request(self, data, **message_kwargs):
def get(self, command):
return self.send_request(path=command, data=None, method='get')

def edit_config(self, requests):
def edit_config(self, requests, suppr_ntf_excp=True):
"""Send a list of http requests to remote device and return results
"""
if requests is None:
Expand All @@ -78,7 +79,11 @@ def edit_config(self, requests):
try:
response = self.send_request(**req)
except ConnectionError as exc:
raise ConnectionError(to_text(exc, errors='surrogate_then_replace'))
if suppr_ntf_excp and req.get('method') == 'get' and re.search("Not Found.*code': 404", str(exc)):
# 'code': 404, 'error-message': 'Resource not found'
response = [{}, {}]
else:
raise ConnectionError(to_text(exc, errors='surrogate_then_replace'))
responses.append(response)
return responses

Expand Down
22 changes: 7 additions & 15 deletions plugins/module_utils/network/sonic/facts/poe/poe.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,7 @@ def get_poe_info(self):
self._module.fail_json(msg=str(exc))

poe_config = {}
try:
poe_config = response[0][1]
if len(poe_config) > 0:
poe_config = poe_config["openconfig-poe:poe"]
except Exception:
raise Exception("response from getting poe facts not formed as expected")
poe_config = response[0][1].get("openconfig-poe:poe", {})

# get poe interface settings
try:
Expand All @@ -172,14 +167,11 @@ def get_poe_info(self):
self._module.fail_json(msg=str(exc))

interface_poe_settings = []
try:
interface_poe_settings = []
for interface in response[0][1]["openconfig-interfaces:interfaces"]["interface"]:
interface_settings = interface.get("openconfig-if-ethernet:ethernet", {}).get("openconfig-if-poe:poe", {})
if len(interface_settings) > 0:
interface_settings.update({"name": interface["name"]})
interface_poe_settings.append(interface_settings)
except Exception:
raise Exception("response from getting poe facts not formed as expected")
poe_interfaces = response[0][1].get("openconfig-interfaces:interfaces", {}).get("interface", [])
for interface in poe_interfaces:
interface_settings = interface.get("openconfig-if-ethernet:ethernet", {}).get("openconfig-if-poe:poe", {})
if len(interface_settings) > 0:
interface_settings.update({"name": interface["name"]})
interface_poe_settings.append(interface_settings)
formatted_specs = self.format_to_argspec(poe_config, interface_poe_settings)
return formatted_specs
24 changes: 14 additions & 10 deletions plugins/module_utils/network/sonic/facts/sflow/sflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,21 @@ def populate_facts(self, connection, ansible_facts, data=None):
if not data:
data = self.get_sflow_info()

data = self.format_to_argspec(data)

# validate can add null values for things missing from device config,
# so doing that before remove empties
cleaned_data = utils.remove_empties(
utils.validate_config(self.argument_spec, data)
)
# convert to argspec for ansible_facts
facts = {}
if data:
data = self.format_to_argspec(data)

# validate can add null values for things missing from device config,
# so doing that before remove empties
cleaned_data = utils.remove_empties(
utils.validate_config(self.argument_spec, data)
)
if cleaned_data:
facts["sflow"] = cleaned_data["config"]

ansible_facts['ansible_network_resources'].pop('sflow', None)
if cleaned_data:
ansible_facts['ansible_network_resources'].update({"sflow": cleaned_data["config"]})
ansible_facts['ansible_network_resources'].update(facts)

return ansible_facts

Expand Down Expand Up @@ -121,7 +125,7 @@ def get_sflow_info(self):

response_body = {}
try:
response_body = response[0][1][response_key]
response_body = response[0][1].get(response_key)
except Exception:
raise Exception("response from getting sflow facts not formed as expected")

Expand Down
3 changes: 3 additions & 0 deletions plugins/module_utils/network/sonic/facts/system/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
from ansible_collections.ansible.netcommon.plugins.module_utils.network.common import (
utils,
)

from ansible.module_utils.connection import ConnectionError

from ansible_collections.dellemc.enterprise_sonic.plugins.module_utils.network.sonic.sonic import (
to_request,
edit_config
Expand Down
7 changes: 4 additions & 3 deletions plugins/module_utils/network/sonic/facts/vrfs/vrfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,16 +92,17 @@ def render_config(self, spec, conf):

def get_all_vrf_interfaces(self):
"""Get all the interfaces available in chassis"""
all_network_instatnces = {}
all_network_instances = {}
request = [{"path": "data/openconfig-network-instance:network-instances", "method": GET}]
try:
response = edit_config(self._module, to_request(self._module, request))
except ConnectionError as exc:
self._module.fail_json(msg=str(exc), code=exc.code)

if "openconfig-network-instance:network-instances" in response[0][1]:
all_network_instatnces = response[0][1].get("openconfig-network-instance:network-instances", {})
return self.get_vrf_interfaces_from_network_instances(all_network_instatnces['network-instance'])
all_network_instances = response[0][1].get("openconfig-network-instance:network-instances", {})
network_instances = all_network_instances.get('network-instance', [])
return self.get_vrf_interfaces_from_network_instances(network_instances)

def get_vrf_interfaces_from_network_instances(self, network_instances):
vrf_interfaces = []
Expand Down
1 change: 1 addition & 0 deletions plugins/module_utils/network/sonic/facts/vxlans/vxlans.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def get_all_vxlans_vrf_list(self):
except ConnectionError as exc:
self._module.fail_json(msg=str(exc), code=exc.code)

vxlan_vrf_list = {}
if "sonic-vrf:VRF_LIST" in response[0][1]:
vxlan_vrf_list = response[0][1].get("sonic-vrf:VRF_LIST", {})

Expand Down
10 changes: 7 additions & 3 deletions plugins/module_utils/network/sonic/sonic.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,22 @@ def run_commands(module, commands, check_rc=True):
module.fail_json(msg=to_text(exc))


def edit_config(module, commands, skip_code=None):
def edit_config(module, commands, skip_code=None, suppr_ntf_excp=True):
connection = get_connection(module)

# Start: This is to convert interface name from Eth1/1 to Eth1%2f1
for request in commands:
# This check is to differenciate between requests and commands
# This check is to differentiate between requests and commands
if isinstance(request, dict):
url = request.get("path", None)
if url:
request["path"] = update_url(url)
# End
return connection.edit_config(commands)
if suppr_ntf_excp:
# Default: not used for cliconf
return connection.edit_config(commands)
else:
return connection.edit_config(commands, suppr_ntf_excp)


def edit_config_reboot(module, commands, skip_code=None):
Expand Down
2 changes: 1 addition & 1 deletion plugins/modules/sonic_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def initiate_request(module):
request = to_request(module, [{"path": url, "method": method, "data": body}])

try:
response = edit_config(module, request)
response = edit_config(module, request, suppr_ntf_excp=False)
except ConnectionError as exc:
module.fail_json(msg=to_text(exc))
return response
Expand Down
15 changes: 10 additions & 5 deletions tests/regression/roles/sonic_system/defaults/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ tests:
ipv4: false
ipv6: false
auto_breakout: ENABLE
load_share_hash_algo: JENKINS_HASH_HI
# Use only on switch models that support this.
#load_share_hash_algo: JENKINS_HASH_HI
audit_rules: BASIC

- name: test_case_02
Expand All @@ -21,7 +22,8 @@ tests:
input:
hostname: SONIC-new
interface_naming: standard_extended
load_share_hash_algo: JENKINS_HASH_LO
# Use only on switch models that support this.
#load_share_hash_algo: JENKINS_HASH_LO
audit_rules: DETAIL

- name: test_case_03
Expand All @@ -40,7 +42,8 @@ tests:
anycast_address:
ipv4: false
auto_breakout: ENABLE
load_share_hash_algo: JENKINS_HASH_LO
# Use only on switch models that support this.
#load_share_hash_algo: JENKINS_HASH_LO
audit_rules: BASIC

- name: test_case_05
Expand All @@ -60,7 +63,8 @@ tests:
ipv4: true
mac_address: 00:09:5B:EC:EE:F2
auto_breakout: ENABLE
load_share_hash_algo: CRC_XOR
# Use only on switch models that support this.
#load_share_hash_algo: CRC_XOR
audit_rules: BASIC

- name: test_case_07
Expand All @@ -83,7 +87,8 @@ tests:
anycast_address:
ipv4: true
auto_breakout: ENABLE
load_share_hash_algo: CRC_32HI
# Use only on switch models that support this.
#load_share_hash_algo: CRC_32HI
audit_rules: BASIC

- name: test_case_09
Expand Down

0 comments on commit 973e80b

Please sign in to comment.