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

[SNMP]: Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG #17045

Merged
merged 9 commits into from
Dec 6, 2023
Merged

[SNMP]: Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG #17045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
81a9e2d
[SNMP]: Modify minigraph parser to update SNMP_AGENT_ADDRESS_CONFIG
SuvarnaMeenakshi Oct 31, 2023
c41dd1f
Use docker0 ip for multi-asic platform.
SuvarnaMeenakshi Oct 31, 2023
e044a5a
Modify to update only for multi-asic
SuvarnaMeenakshi Oct 31, 2023
921c685
Remove zoneid usage as interface is not known
SuvarnaMeenakshi Nov 17, 2023
ce3a8de
Fix as per PR comment to use all Loopback IP address
SuvarnaMeenakshi Nov 28, 2023
4e35e11
Fix as per review comments.
SuvarnaMeenakshi Nov 29, 2023
a589bda
Use string type for port number variable to avoid conversion to str
SuvarnaMeenakshi Nov 29, 2023
3f7f4fe
Modify to loop through the original parsed data as per
SuvarnaMeenakshi Nov 29, 2023
ea3f368
Add additional new line
SuvarnaMeenakshi Nov 29, 2023
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
26 changes: 0 additions & 26 deletions dockers/docker-snmp/snmpd.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,6 @@
{% for (agentip, port, vrf) in SNMP_AGENT_ADDRESS_CONFIG %}
agentAddress {{ protocol(agentip) }}:[{{ agentip }}]{% if port %}:{{ port }}{% endif %}{% if vrf %}%{{ vrf }}{% endif %}{{ "" }}
{% endfor %}
{% elif NAMESPACE_COUNT is not defined or NAMESPACE_COUNT|int <= 1 %}
{% if MGMT_INTERFACE is defined %}
{% for intf, ip in MGMT_INTERFACE %}
{% set agentip = ip.split('/')[0]|lower %}
{% set zoneid = '' %}
# Use interface as zoneid for link local ipv6
{% if agentip.startswith('fe80') %}
{% set zoneid = '%' + intf %}
{% endif %}
agentAddress {{ protocol(agentip) }}:[{{ agentip }}{{ zoneid }}]:161
{% endfor %}
{% endif %}
{% if LOOPBACK_INTERFACE is defined %}
{% for lo in LOOPBACK_INTERFACE %}
{% if lo | length == 2 %}
{% set intf = lo[0] %}
{% set agentip = lo[1].split('/')[0]|lower %}
{% set zoneid = '' %}
# Use interface as zoneid for link local ipv6
{% if agentip.startswith('fe80') %}
{% set zoneid = '%' + intf %}
{% endif %}
agentAddress {{ protocol(agentip) }}:[{{ agentip }}{{ zoneid }}]:161
{% endif %}
{% endfor %}
{% endif %}
{% else %}
agentAddress udp:161
agentAddress udp6:161
Expand Down
3 changes: 0 additions & 3 deletions dockers/docker-snmp/start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,11 @@ mkdir -p /etc/ssw /etc/snmp
# Parse snmp.yml and insert the data in Config DB
/usr/bin/snmp_yml_to_configdb.py

ADD_PARAM=$(printf '%s {"NAMESPACE_COUNT":"%s"}' "-a" "$NAMESPACE_COUNT")

SONIC_CFGGEN_ARGS=" \
-d \
-y /etc/sonic/sonic_version.yml \
-t /usr/share/sonic/templates/sysDescription.j2,/etc/ssw/sysDescription \
-t /usr/share/sonic/templates/snmpd.conf.j2,/etc/snmp/snmpd.conf \
$ADD_PARAM \
"

sonic-cfggen $SONIC_CFGGEN_ARGS
Expand Down
16 changes: 16 additions & 0 deletions src/sonic-config-engine/minigraph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

from portconfig import get_port_config, get_fabric_port_config, get_fabric_monitor_config
from sonic_py_common.interface import backplane_prefix
from sonic_py_common.multi_asic import is_multi_asic

# TODO: Remove this once we no longer support Python 2
if sys.version_info.major == 3:
Expand Down Expand Up @@ -1718,6 +1719,21 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw

results['MGMT_VRF_CONFIG'] = mvrf

# Update SNMP_AGENT_ADDRESS_CONFIG with Management IP and Loopback IP
# if available.
if not is_multi_asic() and asic_name is None:
Copy link
Collaborator

@qiluo-msft qiluo-msft Nov 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

asic_name

why checking asic_name? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For host namespace, asic_name will be None and SNMP_AGENT_ADDRESS_CONFIG is only required for host namespace in case of multi-asic devices. For single asic device asic_name will always be None, but added this check to ensure that we only generate for host namespace.
Minigraph parser generates config for host and each namespace separately.
This is done by passing the right asic_name/

Copy link
Collaborator

@qiluo-msft qiluo-msft Nov 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

if

If the condition does not meet, please initialize an empty results['SNMP_AGENT_ADDRESS_CONFIG']. #Closed

Copy link
Collaborator

@qiluo-msft qiluo-msft Nov 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

not

I guess you want to check is_multi_asic, not single asic. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to check if the device is single asic, then update SNMP_AGENT_ADDRESS_CONFIG.
Hence checking "not is_multi_asic()".
No change required for multi-asic.

results['SNMP_AGENT_ADDRESS_CONFIG'] = {}
port = '161'
for mgmt_intf in mgmt_intf.keys():
snmp_key = mgmt_intf[1].split('/')[0] + '|' + port + '|'
results['SNMP_AGENT_ADDRESS_CONFIG'][snmp_key] = {}
# Add Loopback IP as agent address for single asic
for loip in lo_intfs.keys():
snmp_key = loip[1].split('/')[0] + '|' + port + '|'
results['SNMP_AGENT_ADDRESS_CONFIG'][snmp_key] = {}
else:
results['SNMP_AGENT_ADDRESS_CONFIG'] = {}

phyport_intfs = {}
vlan_intfs = {}
pc_intfs = {}
Expand Down
8 changes: 8 additions & 0 deletions src/sonic-config-engine/tests/sample_graph.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@
</a:Prefix>
<a:PrefixStr>100.0.0.6/32</a:PrefixStr>
</a:LoopbackIPInterface>
<a:LoopbackIPInterface>
<Name>HostIP</Name>
<AttachTo>Loopback1</AttachTo>
<a:Prefix xmlns:b="Microsoft.Search.Autopilot.NetMux">
<b:IPPrefix>100.0.0.7/32</b:IPPrefix>
</a:Prefix>
<a:PrefixStr>100.0.0.7/32</a:PrefixStr>
</a:LoopbackIPInterface>
</LoopbackIPInterfaces>
<ManagementIPInterfaces xmlns:a="http://schemas.datacontract.org/2004/07/Microsoft.Search.Autopilot.Evolution">
<a:ManagementIPInterface>
Expand Down
7 changes: 6 additions & 1 deletion src/sonic-config-engine/tests/test_cfggen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,4 +1145,9 @@ def test_minigraph_cisco_400g_to_100G_speed_no_lane_change(self):
)
)


def testsnmp_agent_address_config(self):
argument = ['-m', self.sample_graph, '-p', self.port_config, '-v', 'SNMP_AGENT_ADDRESS_CONFIG.keys()|list']
output = self.run_script(argument)
self.assertEqual(
utils.liststr_to_dict(output.strip()),
utils.liststr_to_dict("['192.168.200.15|161|', '100.0.0.6|161|', '100.0.0.7|161|']"))