Signed-off-by: Rishikpulhani <[email protected]>

feat(dhcp): Send DHCPRELEASE on container teardown

When a container using DHCP is torn down, its lease is left active on the
DHCP server until it expires. This can be problematic in environments
with small IP pools or long lease times. In setups using Dynamic DNS (DDNS),
it can also lead to stale DNS records.

This change introduces the capability for netavark to send a DHCPRELEASE
message to the DHCP server when a container's network is torn down.

This is implemented by:
- Adding a `release_lease` method to the `DhcpV4Service` in `dhcp_service.rs`,
  which wraps the `release` function from the underlying mozim client.
- Modifying the `teardown` gRPC handler in `dhcp_proxy.rs` to create a
  temporary `DhcpV4Service` instance, retrieve the lease from the cache,
  and call the new `release_lease` method in a "best-effort" manner.

Author: Rishikpulhani

src/commands/dhcp_proxy.rs

@@ -136,8 +136,8 @@ impl<W: Write + Clear + Send + 'static> NetavarkProxy for NetavarkProxyService<W
         };
     }
 
-    /// When a container is shut down this method should be called. It will clear the lease information
-    /// from the caching system.
+    /// When a container is shut down this method should be called. It will release the
+    /// DHCP lease and clear the lease information from the caching system.
     async fn teardown(
         &self,
         request: Request<NetworkConfig>,
@@ -164,6 +164,36 @@ impl<W: Write + Clear + Send + 'static> NetavarkProxy for NetavarkProxyService<W
             .remove_lease(&nc.container_mac_addr)
             .map_err(|e| Status::internal(e.to_string()))?;
 
+        // We have the lease, now try to release it. This is a "best effort"
+        // operation. If it fails, we log the error but do not fail the
+        // teardown process.
+        match mozim::DhcpV4Lease::try_from(lease.clone()) {
+            Ok(mozim_lease) => {
+                // We use a clone of 'nc' for the new service, and the original 'nc' for logging.
+                match DhcpV4Service::new(nc.clone(), self.dora_timeout) {
+                    Ok(mut service) => {
+                        if let Err(e) = service.release_lease(&mozim_lease) {
+                            // Log the error but continue. Use the correct variable for the MAC address.
+                            warn!(
+                                "Failed to send DHCPRELEASE for {}: {}",
+                                &nc.container_mac_addr, e
+                            );
+                        } else {
+                            debug!(
+                                "Successfully sent DHCPRELEASE for {}",
+                                &nc.container_mac_addr
+                            );
+                        }
+                    }
+                    Err(e) => {
+                        warn!("Failed to create DHCP service for release: {e}");
+                    }
+                }
+            }
+            Err(e) => {
+                warn!("Failed to convert lease for release: {e}");
+            }
+        }
         Ok(Response::new(lease))
     }
 

src/dhcp_proxy/dhcp_service.rs

@@ -129,6 +129,19 @@ impl DhcpV4Service {
             "Could not find a lease within the timeout limit".to_string(),
         ))
     }
+
+    /// Sends a DHCPRELEASE message for the given lease.
+    /// This is a "best effort" operation and should not block teardown.
+    pub fn release_lease(&mut self, lease: &MozimV4Lease) -> Result<(), DhcpServiceError> {
+        debug!(
+            "Attempting to release lease for MAC: {}",
+            &self.network_config.container_mac_addr
+        );
+        // Directly call the release function on the underlying mozim client.
+        self.client
+            .release(lease)
+            .map_err(|e| DhcpServiceError::new(Bug, e.to_string()))
+    }
 }
 
 impl std::fmt::Display for DhcpServiceError {

test-dhcp/003-teardown.bats

@@ -5,6 +5,13 @@
 
 load helpers
 
+# Redefine start_proxy locally to enable debug logging for this test only.
+# This version will be used instead of the one from helpers.bash.
+function start_proxy() {
+  RUST_LOG=debug ip netns exec "$NS_NAME" $NETAVARK dhcp-proxy --dir "$TMP_TESTDIR" --uds "$TMP_TESTDIR" &>"$TMP_TESTDIR/proxy.log" &
+  PROXY_PID=$!
+}
+
 @test "basic teardown" {
       read -r -d '\0' input_config <<EOF
 {
@@ -30,6 +37,12 @@ EOF
        assert "$output" == "true"
        # Run teardown
        run_teardown "$input_config"
+       # Check the NETAVARK PROXY log to confirm it successfully SENT the release.
+       # In this test environment, the virtual bridge may not reliably deliver the
+       # broadcast DHCPRELEASE packet to the dnsmasq listener. We check the
+       # proxy's own log for the success message, as this directly confirms
+       # the code path was executed successfully.
+       run_helper grep "Successfully sent DHCPRELEASE for ${CONTAINER_MAC}" "$TMP_TESTDIR/proxy.log"
        run_helper cat "$TMP_TESTDIR/nv-proxy.lease"
        # Check that the length of the lease file is now zero
        run_helper jq ". | length" <<<"$output"