Skip to content

Effectively release the dhcp lease - #440

Merged
w13915984028 merged 1 commit into
harvester:masterfrom
w13915984028:release-dhcp-lease
Oct 19, 2026
Merged

w13915984028 merged 1 commit into
harvester:masterfrom
w13915984028:release-dhcp-lease

Conversation

@w13915984028

@w13915984028 w13915984028 commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Problem:

While test #424, we notice:

When a HNC is deleted, the dhcp lease is never trully released. As the controller removes the vlan first.

Client tcpdump

sudo tcpdump -i mgmt-br.100 -e -nn port 67 or port 68
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on mgmt-br.100, link-type EN10MB (Ethernet), snapshot length 262144 bytes




10:14:17.273838 1a:b2:ff:ba:b7:56 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 1a:b2:ff:ba:b7:56, length 300
10:14:17.274299 52:54:00:2b:06:ba > 1a:b2:ff:ba:b7:56, ethertype IPv4 (0x0800), length 342: 192.168.100.1.67 > 192.168.100.50.68: BOOTP/DHCP, Reply, length 300
10:14:17.274448 1a:b2:ff:ba:b7:56 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 1a:b2:ff:ba:b7:56, length 300
10:14:17.277588 52:54:00:2b:06:ba > 1a:b2:ff:ba:b7:56, ethertype IPv4 (0x0800), length 342: 192.168.100.1.67 > 192.168.100.50.68: BOOTP/DHCP, Reply, length 300
tcpdump: pcap_loop: The interface disappeared
4 packets captured
5 packets received by filter
0 packets dropped by kernel

Server log:

12:14:17.351533 1a:b2:ff:ba:b7:56 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 1a:b2:ff:ba:b7:56, length 300
12:14:17.351776 52:54:00:2b:06:ba > 1a:b2:ff:ba:b7:56, ethertype IPv4 (0x0800), length 342: 192.168.100.1.67 > 192.168.100.50.68: BOOTP/DHCP, Reply, length 300
12:14:17.352130 1a:b2:ff:ba:b7:56 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 1a:b2:ff:ba:b7:56, length 300
12:14:17.355099 52:54:00:2b:06:ba > 1a:b2:ff:ba:b7:56, ethertype IPv4 (0x0800), length 342: 192.168.100.1.67 > 192.168.100.50.68: BOOTP/DHCP, Reply, length 300

Solution:

Do dhcp lease release first before tearing down vlan sub interface.

Related Issue:

harvester/harvester#11757

Test plan:

  1. create HCP in dhcp mode

  2. Delete it.

On DHCP server side, the tcp dump like sudo tcpdump -i virbr0.100 -e -nn port 67 or port 68 log shows one additional dhcp lease log.

13:46:00.794366 1a:b2:ff:ba:b7:56 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 1a:b2:ff:ba:b7:56, length 300
13:46:00.794691 52:54:00:2b:06:ba > 1a:b2:ff:ba:b7:56, ethertype IPv4 (0x0800), length 342: 192.168.100.1.67 > 192.168.100.50.68: BOOTP/DHCP, Reply, length 300
13:46:00.795054 1a:b2:ff:ba:b7:56 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: 0.0.0.0.68 > 255.255.255.255.67: BOOTP/DHCP, Request from 1a:b2:ff:ba:b7:56, length 300
13:46:00.801118 52:54:00:2b:06:ba > 1a:b2:ff:ba:b7:56, ethertype IPv4 (0x0800), length 342: 192.168.100.1.67 > 192.168.100.50.68: BOOTP/DHCP, Reply, length 300

13:46:07.512128 1a:b2:ff:ba:b7:56 > ff:ff:ff:ff:ff:ff, ethertype IPv4 (0x0800), length 342: 0.0.0.0.68 > 192.168.100.1.67: BOOTP/DHCP, Request from 1a:b2:ff:ba:b7:56, length 300

Network controller has a newly added log:

time="2026-10-16T11:46:07Z" level=info msg="Released dhcp lease &{DHCPv4(xid=0x58eaf411 hwaddr=1a:b2:ff:ba:b7:56 msg_type=OFFER, your_ip=192.168.100.50, server_ip=192.168.100.1) DHCPv4(xid=0x58eaf411 hwaddr=1a:b2:ff:ba:b7:56 msg_type=ACK, your_ip=192.168.100.50, server_ip=192.168.100.1) 2026-10-16 11:46:00.786718352 +0000 UTC m=+41.925616405}"

Note: per comment from vendor/github.com/insomniacslk/dhcp/dhcpv4/nclient4/lease.go, not every dhcp server accept above release, my test dnsmasq does not log above relese log.

But, at least, the client side releases the lease effectively.

// Release send DHCPv4 release messsage to server, based on specified lease.
// release is sent as unicast per RFC2131, section 4.4.4.
// Note: some DHCP server requries of using assigned IP address as source IP,
// use nclient4.WithUnicast to create client for such case.
func (c *Client) Release(lease *Lease, modifiers ...dhcpv4.Modifier) error {

NOTE: this PR cannot fully solve the potential race case:
(1) background renew timer triggers Renew/Request action (pending)
(2) controller calls cancel & release

It will be fixed on another PR.

@w13915984028
w13915984028 force-pushed the release-dhcp-lease branch 2 times, most recently from 3fdb509 to 4f0e34a Compare October 16, 2026 11:43
Signed-off-by: Jian Wang <jian.wang@suse.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused changes address release ordering with no identified blocking regressions; the documented renewal race remains outside scope.

0 open findings

What changed in this PR

Ensures DHCP lease release occurs before VLAN teardown, preserving connectivity needed to send the release.

Changes:

  • Stops the lease manager before removing bridge VLAN configuration.
  • Adds release-result logging and closes the DHCP client after a release attempt.
File Description
pkg/​controller/​agent/​hostnetworkconfig/​leasemanager.go Handles missing leases, logs release results, and closes the client.
pkg/​controller/​agent/​hostnetworkconfig/​controller.go Moves lease cleanup ahead of VLAN teardown.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@rrajendran17

Copy link
Copy Markdown
Contributor

LGTM, Thanks for the changes as its right to release the lease before vlan is removed from the bridge and not after that.

@mergify

mergify Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@w13915984028
w13915984028 merged commit 3f0c981 into harvester:master Oct 19, 2026
8 checks passed
@w13915984028
w13915984028 deleted the release-dhcp-lease branch October 19, 2026 07:34
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