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.

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

[minigraph] Add support for ipv4 dhcp_server in minigraph parser #16313

Closed

Conversation

yaqiangz
Copy link
Contributor

Why I did it

Add minigraph parser support for dhcp_server in BmcMgmtToRRouter device followed sonic-net/SONiC#1282. Parse dhcp_server related info from minigraph.

Work item tracking
  • Microsoft ADO (number only): 17695979

How I did it

Add minigraph parser support for dhcp_server in BmcMgmtToRRouter device.

How to verify it

Build image, ut passed.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@yaqiangz yaqiangz changed the title Add support for ipv4 dhcp_server in minigraph parser [minigraph] Add support for ipv4 dhcp_server in minigraph parser Aug 29, 2023
@yaqiangz yaqiangz force-pushed the azure-master_dhcp_server_ipv4_parser branch from 7c8b1c4 to 8575641 Compare August 29, 2023 06:27
@yaqiangz yaqiangz force-pushed the azure-master_dhcp_server_ipv4_parser branch from 8575641 to 5f0d5bb Compare August 29, 2023 07:53
@yaqiangz yaqiangz marked this pull request as ready for review August 31, 2023 08:52
@yaqiangz
Copy link
Contributor Author

yaqiangz commented Sep 5, 2023

@yxieca Could you pls help to merge this PR?

@lguohan
Copy link
Collaborator

lguohan commented Sep 5, 2023

@qiluo-msft , i thought we are gong to block all minigraph changes now?

results['DHCP_SERVER_IPV4'] = dhcp_server_ipv4
if dhcp_server_ipv4_customized_options:
results['DHCP_SERVER_IPV4_CUSTOMIZED_OPTIONS'] = dhcp_server_ipv4_customized_options
results['DHCP_SERVER_IPV4_PORT'] = dhcp_server_ipv4_port
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a new table in ConfigDB. Please onboard it with golden config generation pipeline. We are blocking new tables coming into minigraph parser.

Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

as comment

@@ -2068,6 +2140,18 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
'client_crt_cname': 'client.restapi.sonic'
}
}
if devices[hostname]["type"] == "BmcMgmtToRRouter":
dhcp_server_ipv4, dhcp_server_ipv4_port, dhcp_server_ipv4_customized_options = \
generate_ipv4_dhcp_server_config(devices, link_ports, port_alias_map, vlan_intfs, vlan_members,
Copy link
Collaborator

Choose a reason for hiding this comment

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

generate_ipv4_dhcp_server_config

Why generate_ipv4_dhcp_server_config() is needed. It just manipulated existing ConfigDB, does not parse minigraph at all, no extra info added into ConfigDB. Seems like everything you needed is already in ConfigDB or as default values.

@yaqiangz yaqiangz closed this Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants