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

[dhcp-relay]: Launch DHCP Relay On L3 Vlan #6527

Conversation

tahmed-dev
Copy link
Contributor

Recent changes brought L2 vlan concept which do not have DHCP
clients behind them and so DHCP relay is not required. Also,
dhcpmon fails to launch on those vlans as their interfaces
lack IP addresses. This PR limit launch of both DHCP relay
and dhcpmon to L3 vlans only.

singed-off-by: Tamer Ahmed tamer.ahmed@microsoft.com

- Why I did it
Stop non functioning DHCP relay from being launched and also disable error being thrown by dhcpmon

- How I did it
Added extra check for presence of vlan in the vlan_interface table

- How to verify it
PR check passes

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

  • 201811
  • 201911
  • 202006
  • 202012

Recent changes brought l2 vlan concept which do not have DHCP
clients behind them and so DHCP relay is not required. Also,
dhcpmon fails to launch on those vlans as their interfaces
lack IP addresses. This PR limit launch of both DHCP relay
and dhcpmon to L3 vlans only.

singed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
@abdosi
Copy link
Contributor

abdosi commented Jan 21, 2021

we don't need this for 201811 ?

prsunny
prsunny previously approved these changes Jan 21, 2021
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.

lgtm.

@tahmed-dev
Copy link
Contributor Author

we don't need this for 201811 ?

No, L2 Vlans are not 201811

@lguohan
Copy link
Collaborator

lguohan commented Jan 24, 2021

where is the expected results?

@tahmed-dev
Copy link
Contributor Author

where is the expected results?

It is still one L3 vlan, however new L2 vlan does not have dhcprelay launched. It is a -ve test where without the change in the supervisord, the test fails with the following logs:

======================================================================
FAIL: test_dhcp_relay (tests.test_j2files.TestJ2Files)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/sonic/src/sonic-config-engine/tests/test_j2files.py", line 72, in test_dhcp_relay
    self.assertTrue(filecmp.cmp(os.path.join(self.test_dir, 'sample_output', utils.PYvX_DIR, 'docker-dhcp-relay.supervisord.conf'), self.output_file))
AssertionError: False is not true

----------------------------------------------------------------------
Ran 146 tests in 40.672s

FAILED (failures=1)

@tahmed-dev tahmed-dev merged commit 8d857fa into sonic-net:master Jan 25, 2021
@tahmed-dev tahmed-dev deleted the taahme/launch-dhcp-relay-for-l3-vlans-only branch January 25, 2021 18:49
abdosi pushed a commit that referenced this pull request Jan 25, 2021
Recent changes brought l2 vlan concept which do not have DHCP
clients behind them and so DHCP relay is not required. Also,
dhcpmon fails to launch on those vlans as their interfaces
lack IP addresses. This PR limit launch of both DHCP relay
and dhcpmon to L3 vlans only.

singed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
lguohan pushed a commit that referenced this pull request Jan 28, 2021
Recent changes brought l2 vlan concept which do not have DHCP
clients behind them and so DHCP relay is not required. Also,
dhcpmon fails to launch on those vlans as their interfaces
lack IP addresses. This PR limit launch of both DHCP relay
and dhcpmon to L3 vlans only.

singed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
tahmed-dev added a commit that referenced this pull request May 12, 2021
Recent changes brought l2 vlan concept which does not have DHCP
clients behind them and so DHCP relay is not required. Also,
dhcpmon fails to launch on those vlans as their interfaces
lack IP addresses. This PR backposts #6527 that limits launch
of both DHCP relay and dhcpmon to L3 vlans only.

original-pr: #6527
singed-off-by: Tamer Ahmed tamer.ahmed@microsoft.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants