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

Jump to bottom

T6948: Keep DHCP server leases in sync with hostd records #4237

Merged
merged 6 commits into from
Feb 27, 2025
Merged

T6948: Keep DHCP server leases in sync with hostd records #4237

merged 6 commits into from
Feb 27, 2025

Conversation

indrajitr
Copy link
Contributor

@indrajitr indrajitr commented Dec 13, 2024 •
edited
Loading

Change Summary

Keep DHCP server leases in sync with vyos-hostd records via helper script invoked with ExecStartPost directive in kea-dhcp4-server.service.

The helper script updates VyOS hostd records from DHCP server leases. This ensures that hostd records with the DHCP server leases are kept in sync with VyOS hostd records right after DHCP server is started.

This is similar to the capability exposed via kea hook libdhcp_run_script.so which is invoked internally by kea when a single lease changes state.

Since the kea hook is currently implemented for DHCPv4 only, this helper script is implemented for DHCPv4 only as well.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

#4307

Component(s) name

dhcp server

Proposed changes

How to test

  1. Enable DHCP server and have active server leases
  2. Ensure that relevant host entries exist in /etc/hosts and/or /run/vyos-hostsd/vyos-hostsd.state
  3. Restart VyOS
  4. Verify DHCP server leases are still active (show dhcp server leases)
  5. Verify that the leases have NOT been applied to /etc/hosts
  6. Restart dhcp server (restart dhcp server)
  7. Verify that the leases have been applied to /etc/hosts

Smoketest result

vyos@test-1216:~$ /usr/libexec/vyos/tests/smoke/cli/test_service_dhcp-server.py
test_dhcp_exclude_in_range (__main__.TestServiceDHCPServer.test_dhcp_exclude_in_range) ... ok
test_dhcp_exclude_not_in_range (__main__.TestServiceDHCPServer.test_dhcp_exclude_not_in_range) ... ok
test_dhcp_high_availability (__main__.TestServiceDHCPServer.test_dhcp_high_availability) ... ok
test_dhcp_high_availability_standby (__main__.TestServiceDHCPServer.test_dhcp_high_availability_standby) ... ok
test_dhcp_hostsd_lease_sync (__main__.TestServiceDHCPServer.test_dhcp_hostsd_lease_sync) ... ok
test_dhcp_multiple_pools (__main__.TestServiceDHCPServer.test_dhcp_multiple_pools) ... ok
test_dhcp_on_interface_with_vrf (__main__.TestServiceDHCPServer.test_dhcp_on_interface_with_vrf) ... ok
test_dhcp_relay_server (__main__.TestServiceDHCPServer.test_dhcp_relay_server) ... ok
test_dhcp_single_pool_options (__main__.TestServiceDHCPServer.test_dhcp_single_pool_options) ... ok
test_dhcp_single_pool_options_scoped (__main__.TestServiceDHCPServer.test_dhcp_single_pool_options_scoped) ... ok
test_dhcp_single_pool_range (__main__.TestServiceDHCPServer.test_dhcp_single_pool_range) ... ok
test_dhcp_single_pool_static_mapping (__main__.TestServiceDHCPServer.test_dhcp_single_pool_static_mapping) ... ok

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 13, 2024 •
edited
Loading

👍
No issues in PR Title / Commit Title

@indrajitr
Copy link
Contributor Author

Keeping it as draft till I have a working test environment.

@indrajitr indrajitr marked this pull request as draft December 13, 2024 18:01
@indrajitr indrajitr force-pushed the hostd-update branch 2 times, most recently from 5e444ca to 748310c Compare December 19, 2024 00:24
@indrajitr indrajitr marked this pull request as ready for review December 19, 2024 00:29
@indrajitr
Copy link
Contributor Author

Update: Ready for review.
This has been working fine for me in the local environment for the last few days.

sarthurdev
sarthurdev approved these changes Jan 9, 2025
View reviewed changes
Copy link
Member

@sarthurdev sarthurdev left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding the smoketest.

@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions GitHub Actions
Copy link

Conflicts have been resolved. A maintainer will review the pull request shortly.

@indrajitr indrajitr force-pushed the hostd-update branch 2 times, most recently from 7c2f986 to 2aed4d7 Compare January 21, 2025 04:40
@indrajitr indrajitr requested a review from sarthurdev January 21, 2025 04:43
@indrajitr indrajitr force-pushed the hostd-update branch 4 times, most recently from 6c14589 to 033ae6a Compare January 27, 2025 16:53
@indrajitr indrajitr requested a review from a team as a code owner February 10, 2025 22:17
@indrajitr indrajitr force-pushed the hostd-update branch 2 times, most recently from b143cd7 to 4ea98e2 Compare February 11, 2025 02:03
c-po
c-po approved these changes Feb 17, 2025
View reviewed changes
Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

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

This looks good to me. I asked @sarthurdev as our VyOS most experienced KEA user for his opinion, too.

sarthurdev
sarthurdev requested changes Feb 17, 2025
View reviewed changes
Copy link
Member

@sarthurdev sarthurdev left a comment

Choose a reason for hiding this comment

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

LGTM, just this last issue

@indrajitr
dhcp: T6948: kea: Add kea helpers and enrich leases with domain name
d1bb249 
Add helpers:
- `kea_add_lease` to add a lease to the running kea server
- `kea_get_domain_from_subnet_id` to get the domain name
  from subnet id

Also, enrich leases with domain name from subnet id
@indrajitr
dhcp: T6948: Keep DHCP server leases in sync with hostd records
aa44f17 
Keep DHCP server leases in sync with vyos-hostd records
via helper script invoked with `ExecStartPost` directive
in kea-dhcp4-server.service.

The helper script updates VyOS hostd records from
DHCP server leases. This ensures that hostd records
with the DHCP server leases are kept in sync with
VyOS hostd records right after DHCP server is started.

This is similar to the capability exposed via kea hook
`libdhcp_run_script.so` which is invoked internally
by kea when a single lease changes state.

Since the kea hook is currently implemented for DHCPv4
only, this helper script is implemented for DHCPv4
only as well.
@indrajitr
dhcp: T6948: systemd-service adjustment to invoke helper script
caaa35b 
The helper script updates VyOS hostd records from
DHCP server leases. This ensures that hostd records
with the DHCP server leases are kept in sync with
VyOS hostd records right after the DHCP server is
started.

Note that `Restart` directive needs to be updated to
`on-failure` so that the service is restarted in case
of failure/timeout in interaction with hostd service.
@indrajitr
dhcp: T6948: Smoketest for DHCP server leases sync with hostd records
64d758b 
The test `test_dhcp_hostsd_lease_sync` validates
DHCP server leases in sync with hostd records.

Since the DHCP server running in smoketest environment
might not have actual clients to lease IP addresses,
we simulate the lease creation and deletion using kea
helper functions (`kea_add_lease` and `kea_delete_lease`).
@indrajitr
ruff: T6583: Reformat to comply with code style
6c809e5
@indrajitr
dhcp: T7052: Clean up unnecessary params in formatter methods
cc45faf 
The formatter methods are mostly `family` agnostic now.
@github-actions GitHub Actions
Copy link

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@sever-sever sever-sever merged commit 8c14daa into vyos:current Feb 27, 2025
14 checks passed
@vyosbot vyosbot added the mirror-initiated This PR initiated for mirror sync workflow label Feb 27, 2025
@indrajitr indrajitr deleted the hostd-update branch February 28, 2025 06:07
@kumvijaya kumvijaya removed the mirror-initiated This PR initiated for mirror sync workflow label Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarthurdev sarthurdev sarthurdev approved these changes

@sever-sever sever-sever sever-sever approved these changes

@c-po c-po c-po approved these changes

@dmbaturin dmbaturin Awaiting requested review from dmbaturin

@jestabro jestabro Awaiting requested review from jestabro

@fett0 fett0 Awaiting requested review from fett0

@nicolas-fort nicolas-fort Awaiting requested review from nicolas-fort

@zdc zdc Awaiting requested review from zdc

Assignees

@indrajitr indrajitr

Labels
current rebase
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@indrajitr @sarthurdev @sever-sever @c-po @kumvijaya @vyosbot