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_server] Add dhcprelayd for dhcp_server feature #16947

Merged
merged 11 commits into from
Nov 2, 2023

Conversation

yaqiangz
Copy link
Contributor

@yaqiangz yaqiangz commented Oct 19, 2023

Why I did it

Add support in dhcp_relay container for dhcp_server_ipv4 feature. HLD: sonic-net/SONiC#1282

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

How I did it

  • Inside dhcp_relay container:
  1. Add install psutil and install sonic_dhcp_server python wheels in Dockerfile.j2
  2. For dhcp_relay cli, add restrictions for disabling add/del/show ipv4 dhcp_relay while dhcp_server is enabled.
  3. Add dhcprelayd process, to start/restart dhcpmon (dhcpv4 relay counter) and dchrelay processes. And monitor config_db change.
  4. Modify template to generate supervisord.conf file. If dhcp_server feature is disabled, generate configuration file as before. If dhcp_server feature is enabled, make supervisor not run dhcpmon (dhcpv4 relay counter) and dchrelay process, instead run dhcprelayd process.
  5. Add DhcpRelaydDbMonitor to monitor db changes
  • Inside dhcp_server container:
  1. Add support for dhcp_lease to remove expired lease found in lease file.
  2. After init, add dhcp_server_ip (eth0 inside dhcp_server container) to state_db
  • Inside sonic-config-engine wheel
  1. Add/Modify UTs for configuration j2 for dhcp_relay changes.
    1.1 For dhcp_server disabled, remove repeated blank lines in expected output file.
    1.2 For dhcp_server enabled, add new UT.
  • For sonic-dhcp-server python wheel
  1. Add dhcp_db_monitor.py which is used to monitor db change.
  2. Refactor folder structure inside src/sonic-dhcp-server/dhcp_server

How to verify it

  • UTs in sonic-config-engine all passed, to make sure that supervisor configuration file for dhcp_relay container is generated correct.
  • UTs in sonic-dhcp-server all passed
=========================================================== test session starts ============================================================
platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0
rootdir: /sonic/src/sonic-dhcp-server, configfile: setup.cfg, testpaths: tests
plugins: pyfakefs-5.2.4, cov-2.10.1
collected 140 items                                                                                                                        

tests/test_dhcp_cfggen.py ..........                                                                                                 [  7%]
tests/test_dhcp_db_monitor.py .......................                                                                                [ 23%]
tests/test_dhcp_lease.py .....                                                                                                       [ 27%]
tests/test_dhcprelayd.py ........................................................................................                    [ 90%]
tests/test_dhcpservd.py ......                                                                                                       [ 94%]
tests/test_utils.py ........                                                                                                         [100%]Clearing the cache


----------- coverage: platform linux, python 3.9.2-final-0 -----------
Name                                    Stmts   Miss Branch BrPart     Cover   Missing
--------------------------------------------------------------------------------------
dhcp_server/dhcprelayd/dhcprelayd.py      120      8     48      0    91.67%   73-82
dhcp_server/dhcpservd/dhcp_cfggen.py      169     16     72      1    92.95%   48-65, 148-152, 238->240, 240
dhcp_server/dhcpservd/dhcp_lease.py        95      3     30      4    94.40%   57->59, 59, 60->61, 61, 74->76, 112->134, 153
dhcp_server/common/utils.py                40      3     14      0    94.44%   35, 55-56
dhcp_server/common/dhcp_db_monitor.py      81      1     30      1    98.20%   69->70, 70
dhcp_server/dhcpservd/dhcpservd.py         49      1     10      0    98.31%   73
--------------------------------------------------------------------------------------
TOTAL                                     554     32    204      6    94.20%

1 file skipped due to complete coverage.

Required test coverage of 80.0% reached. Total coverage: 94.20%

=========================================================== slowest 5 durations ============================================================
0.07s call     tests/test_dhcp_lease.py::test_update_kea_lease
0.02s call     tests/test_dhcprelayd.py::test_start_dhcpmon_process[zombie-2-new_dhcp_interfaces_list0]
0.01s call     tests/test_dhcp_cfggen.py::test_render_config[True]
0.01s call     tests/test_dhcpservd.py::test_dump_dhcp4_config
0.01s call     tests/test_dhcp_cfggen.py::test_render_config[False]

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 force-pushed the master_dhcprelayd branch 6 times, most recently from 04d9912 to c0b9a32 Compare October 20, 2023 09:22
rules/config Outdated Show resolved Hide resolved
@yaqiangz yaqiangz force-pushed the master_dhcprelayd branch 3 times, most recently from 91192e4 to 8027951 Compare October 29, 2023 12:09
@yaqiangz yaqiangz marked this pull request as ready for review October 30, 2023 01:01
@yaqiangz yaqiangz force-pushed the master_dhcprelayd branch 6 times, most recently from 8ae3baa to 085076c Compare October 30, 2023 08:38
@yaqiangz
Copy link
Contributor Author

yaqiangz commented Nov 2, 2023

@yxieca Could you pls help to signoff and merge this PR?

@yaqiangz yaqiangz requested a review from yxieca November 2, 2023 03:03
@yxieca yxieca merged commit 274d320 into sonic-net:master Nov 2, 2023
19 checks passed
lixiaoyuner pushed a commit to lixiaoyuner/sonic-buildimage that referenced this pull request Feb 6, 2024
…sonic-buildimage into internal

1. Resolve conflicts (introduced by this public PR: [sonic-net#16947](sonic-net#16947)) in `dockers/docker-dhcp-relay/Dockerfile.j2`:
```
<<<<<<< HEAD
RUN apt-get install -y dnsmasq gcc python3-dev

RUN pip3 install psutil

RUN apt-get purge -y gcc python3-dev
=======
{% if docker_dhcp_relay_whls.strip() %}
# Copy locally-built Python wheel dependencies
{{ copy_files("python-wheels/", docker_dhcp_relay_whls.split(' '), "/python-wheels/") }}

# Install locally-built Python wheel dependencies
{{ install_python_wheels(docker_dhcp_relay_whls.split(' ')) }}
{% endif %}
>>>>>>> 274d320
```

Accepted both changes, after resolved:
```
RUN apt-get install -y dnsmasq gcc python3-dev

RUN pip3 install psutil

RUN apt-get purge -y gcc python3-dev

{% if docker_dhcp_relay_whls.strip() %}
# Copy locally-built Python wheel dependencies
{{ copy_files("python-wheels/", docker_dhcp_relay_whls.split(' '), "/python-wheels/") }}

# Install locally-built Python wheel dependencies
{{ install_python_wheels(docker_dhcp_relay_whls.split(' ')) }}
{% endif %}
```

2. Because dnsmasq (used for ipv4 dhcp_server previously) exist in internal repo but not exist in public master repo, which would cause new added test case added by [sonic-net#16947](sonic-net#16947) in sonic-config-engine would fail. Also, we expect that dnsmasq is not running if "dhcp_server" feature is enabled.
So I **add support for new added test case** and **modify template to avoid run dnsmasq when dhcp_server feature is enabled** in below commit:
![image (2).png](https://dev.azure.com/msazure/b32aa71e-8ed2-41b2-9d77-5bc261222004/_apis/git/repositories/8721f84d-7905-4f85-b5f1-0e19e8eac66b/pullRequests/9025464/attachments/image%20%282%29.png)

Related work items: sonic-net#350, sonic-net#803, sonic-net#2858, sonic-net#16945, sonic-net#16996, sonic-net#17047, sonic-net#17053, sonic-net#17056, sonic-net#17057, sonic-net#17068
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.

3 participants