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

Modify dhcp relay to pick primary address #17012

Conversation

shbalaku-microsoft
Copy link
Contributor

@shbalaku-microsoft shbalaku-microsoft commented Oct 25, 2023

Why I did it

This is change taken as part of the HLD: sonic-net/SONiC#1470 and this is a follow up on the PR #16827 where in the docker-dhcp we pick the value of primary gateway of the interface from the VLAN_Interface table which has "secondary" flag set in the config_db

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

How I did it

1: Changes in the j2 file to add a new "-pg" parameter in the dhcpv4-relay.agents.j2, the ip would be retrieved from the config db's vlan_interface table such that the interface which are picked will have secondary field set.
2: Changes in isc-dhcp to re-order the addresses of the discovered interface and which has the ip which has the passed parameter.

How to verify it

Validated using the existing test cases.

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

Modify dhcp relay to pick primary address

Link to config_db schema for YANG module changes

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

@prsunny
Copy link
Contributor

prsunny commented Nov 13, 2023

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

Can you also add unit tests?

src/sonic-config-engine/tests/test_j2files.py Outdated Show resolved Hide resolved
@@ -0,0 +1,114 @@
diff --git a/common/discover.c b/common/discover.c
Copy link
Contributor

Choose a reason for hiding this comment

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

@kellyyeh , could you please review this?

src/sonic-config-engine/sonic-cfggen Outdated Show resolved Hide resolved
@prsunny prsunny requested a review from kellyyeh November 18, 2023 01:32
prsunny
prsunny previously approved these changes Nov 22, 2023
@shbalaku-microsoft
Copy link
Contributor Author

Can you also add unit tests?

I have added test cases in the following PRs: sonic-net/sonic-mgmt#10828 and #17263

The second one is expected to fail as the vlan_interface changes in #16827 and the current PR changes are not in.

@lguohan lguohan merged commit 8b192a1 into sonic-net:master Nov 22, 2023
yxieca pushed a commit that referenced this pull request Dec 4, 2023
This is change taken as part of the HLD: sonic-net/SONiC#1470 and this is a follow up on the PR #16827 where in the docker-dhcp we pick the value of primary gateway of the interface from the VLAN_Interface table which has "secondary" flag set in the config_db

Microsoft ADO (number only): 16784946

How did I do it
-  Changes in the j2 file to add a new "-pg" parameter in the dhcpv4-relay.agents.j2, the ip would be retrieved from the config db's vlan_interface table such that the interface which are picked will have secondary field set.

- Changes in isc-dhcp to re-order the addresses of the discovered interface and which has the ip which has the passed parameter.
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