dhcp-proxy: apply new ip address/gateway

If we get a new lease with different ips or a new gateway we have to
update the contianer netns with the new addresses. The added tests takes
over 2m because the minimal lease time that dnsmasq supports is 2m so we
have to love with that for now.

One outstanding issue is that podman has no idea that things changed, it
will continue to show incorrect network info in podman inspect. This
never worked with CNI either so it should be ok for now.

However I think it would be a great improvement for long running
containers if we could somehow update the satus in podman. I think we
need some hidden podman command callback where we can feed podman the
new info.

Signed-off-by: Paul Holzinger <[email protected]>

src/dhcp_proxy/dhcp_service.rs

@@ -1,8 +1,14 @@
+use std::net::Ipv4Addr;
+
 use crate::dhcp_proxy::dhcp_service::DhcpServiceErrorKind::{
     Bug, InvalidArgument, NoLease, Timeout,
 };
 
 use crate::dhcp_proxy::lib::g_rpc::{Lease as NetavarkLease, NetworkConfig};
+use crate::error::{ErrorWrap, NetavarkError, NetavarkResult};
+use crate::network::core_utils;
+use crate::network::netlink::Route;
+use crate::wrap;
 use log::debug;
 use mozim::{DhcpV4ClientAsync, DhcpV4Config, DhcpV4Lease as MozimV4Lease};
 use tokio_stream::StreamExt;
@@ -36,7 +42,7 @@ impl DhcpServiceError {
 pub struct DhcpV4Service {
     client: DhcpV4ClientAsync,
     network_config: NetworkConfig,
-    previous_lease: Option<NetavarkLease>,
+    previous_lease: Option<MozimV4Lease>,
 }
 
 impl DhcpV4Service {
@@ -63,15 +69,15 @@ impl DhcpV4Service {
     ///
     pub async fn get_lease(&mut self) -> Result<NetavarkLease, DhcpServiceError> {
         if let Some(Ok(lease)) = self.client.next().await {
-            let mut netavark_lease = <NetavarkLease as From<MozimV4Lease>>::from(lease);
+            let mut netavark_lease = <NetavarkLease as From<MozimV4Lease>>::from(lease.clone());
             netavark_lease.add_domain_name(&self.network_config.domain_name);
             netavark_lease.add_mac_address(&self.network_config.container_mac_addr);
 
             debug!(
                 "found a lease for {:?}, {:?}",
                 &self.network_config.container_mac_addr, &netavark_lease
             );
-            self.previous_lease = Some(netavark_lease.clone());
+            self.previous_lease = Some(lease);
 
             return Ok(netavark_lease);
         }
@@ -104,20 +110,34 @@ pub async fn process_client_stream(mut client: DhcpV4Service) {
     while let Some(lease) = client.client.next().await {
         match lease {
             Ok(lease) => {
-                log::debug!(
+                log::info!(
                     "got new lease for mac {}: {:?}",
                     &client.network_config.container_mac_addr,
                     &lease
                 );
-                let lease = NetavarkLease::from(lease);
                 // get previous lease and check if ip addr changed, if not we do not have to do anything
                 if let Some(old_lease) = &client.previous_lease {
                     if old_lease.yiaddr != lease.yiaddr
-                        || old_lease.gateways != lease.gateways
                         || old_lease.subnet_mask != lease.subnet_mask
+                        || old_lease.gateways != lease.gateways
                     {
                         // ips do not match, remove old ones and assign new ones.
-                        log::error!("ips do not match, reassign not implemented")
+                        log::info!(
+                            "ip or gateway for mac {} changed, update address",
+                            &client.network_config.container_mac_addr
+                        );
+                        match update_lease_ip(
+                            &client.network_config.ns_path,
+                            &client.network_config.container_iface,
+                            old_lease,
+                            &lease,
+                        ) {
+                            Ok(_) => {}
+                            Err(err) => {
+                                log::error!("{err}");
+                                continue;
+                            }
+                        }
                     }
                 }
                 client.previous_lease = Some(lease)
@@ -129,3 +149,65 @@ pub async fn process_client_stream(mut client: DhcpV4Service) {
         }
     }
 }
+
+fn update_lease_ip(
+    netns: &str,
+    interface: &str,
+    old_lease: &MozimV4Lease,
+    new_lease: &MozimV4Lease,
+) -> NetavarkResult<()> {
+    let (_, netns) =
+        core_utils::open_netlink_sockets(netns).wrap("failed to open netlink socket in netns")?;
+    let mut sock = netns.netlink;
+    let old_net = wrap!(
+        ipnet::Ipv4Net::with_netmask(old_lease.yiaddr, old_lease.subnet_mask),
+        "create ipnet from old lease"
+    )?;
+    let new_net = wrap!(
+        ipnet::Ipv4Net::with_netmask(new_lease.yiaddr, new_lease.subnet_mask),
+        "create ipnet from new lease"
+    )?;
+
+    if new_net != old_net {
+        let link = sock
+            .get_link(crate::network::netlink::LinkID::Name(interface.to_string()))
+            .wrap("get interface in netns")?;
+        sock.add_addr(link.header.index, &ipnet::IpNet::V4(new_net))
+            .wrap("add new addr")?;
+        sock.del_addr(link.header.index, &ipnet::IpNet::V4(old_net))
+            .wrap("remove old addrs")?;
+    }
+    if new_lease.gateways != old_lease.gateways {
+        if let Some(gws) = &old_lease.gateways {
+            let old_gw = gws.first();
+            if let Some(gw) = old_gw {
+                let route = Route::Ipv4 {
+                    dest: ipnet::Ipv4Net::new(Ipv4Addr::new(0, 0, 0, 0), 0)?,
+                    gw: *gw,
+                    metric: None,
+                };
+                match sock.del_route(&route) {
+                    Ok(_) => {}
+                    Err(err) => match err.unwrap() {
+                        // special case do not error if route does not exists
+                        NetavarkError::Netlink(e) if -e.code == libc::ESRCH => {}
+                        _ => return Err(err).wrap("delete old default route"),
+                    },
+                };
+            }
+        }
+        if let Some(gws) = &new_lease.gateways {
+            let new_gw = gws.first();
+            if let Some(gw) = new_gw {
+                let route = Route::Ipv4 {
+                    dest: ipnet::Ipv4Net::new(Ipv4Addr::new(0, 0, 0, 0), 0)?,
+                    gw: *gw,
+                    metric: None,
+                };
+                sock.add_route(&route)?;
+            }
+        }
+    }
+
+    Ok(())
+}

test-dhcp/005-renew.bats

@@ -0,0 +1,59 @@
+#!/usr/bin/env bats   -*- bats -*-
+#
+# Test that release is working after lease timeout
+#
+
+load helpers
+
+
+@test "release after timeout" {
+      read -r -d '\0' input_config <<EOF
+{
+  "host_iface": "veth1",
+  "container_iface": "veth0",
+  "container_mac_addr": "$CONTAINER_MAC",
+  "domain_name": "example.com",
+  "host_name": "foobar",
+  "version": 0,
+  "ns_path": "$NS_PATH"
+}
+  \0
+EOF
+
+
+    run_setup "$input_config"
+    ip_before=$(jq -r '.yiaddr' <<<"$output")
+    gw_before=$(jq -r '.gateways[0]' <<<"$output")
+    has_ip "$ip_before" veth0
+    run_in_container_netns ip -j route show default
+    assert "$output" =~ "$gw_before"
+
+
+    # stop dhcp and restart with new subnet to get a new ip on the next lease
+    stop_dhcp
+    run_in_container_netns ip add del $(gateway_from_subnet) dev br0
+    run_in_container_netns ip addr
+    run_in_container_netns ip route
+
+    # get new subnet
+    SUBNET_CIDR=$(random_subnet)
+    run_in_container_netns ip addr add $(gateway_from_subnet) dev br0
+    stripped_subnet=$(strip_last_octet_from_subnet)
+    run_dhcp
+
+    run_in_container_netns ip addr
+    run_in_container_netns ip route
+
+    # Sigh, minimum lease time in dnsmasq is 2m, give some extra time for the
+    # lease roundtrip and ip changes to be applied
+    sleep 125
+    # after two minutes we should have a new lease and assigned the new ip
+    has_ip "$stripped_subnet" veth0
+
+    # make sure we got the new gateway set as well
+    run_in_container_netns ip -j route show default
+    assert "$output" =~ "$(gateway_from_subnet)"
+
+    # extra check to make sure we got our expected log
+    run_helper grep "ip or gateway for mac $CONTAINER_MAC changed" "$TMP_TESTDIR/proxy.log"
+}

test-dhcp/helpers.bash

@@ -263,6 +263,7 @@ function basic_teardown(){
   remove_bridge "br0"
   stop_dhcp "$DNSMASQ_PID"
   run_in_container_netns ip link set lo down
+  rm -rf "$TMP_TESTDIR"
 }
 
 
@@ -276,7 +277,6 @@ function basic_setup() {
   add_veth "veth1" "br0"
   run_in_container_netns ip link set lo up
   run_dhcp "$TESTSDIR/dnsmasqfiles"
-  DNSMASQ_PID="$output"
   start_proxy
 }
 
@@ -346,7 +346,7 @@ port=0
 
 
 # To enable dnsmasq's DHCP server functionality.
-dhcp-range=${stripped_subnet}50,${stripped_subnet}59,255.255.255.0,12h
+dhcp-range=${stripped_subnet}50,${stripped_subnet}59,255.255.255.0,2m
 
 # Set gateway as Router. Following two lines are identical.
 dhcp-option=3,$gw
@@ -362,27 +362,30 @@ log-dhcp    # log dhcp related messages.
 \0
 EOF
   dnsmasq_testdir="${TMP_TESTDIR}/dnsmasq"
-  DNSMASQ_PIDFILE="${TMP_TESTDIR}/dns.PID"
-  mkdir $dnsmasq_testdir
+  mkdir -p $dnsmasq_testdir
   echo "$dnsmasq_config" > "$dnsmasq_testdir/test.conf"
 
-   run_in_container_netns dnsmasq --log-debug --log-queries --conf-dir "${dnsmasq_testdir}" -x "${DNSMASQ_PIDFILE}" &
+  ip netns exec "${NS_NAME}" dnsmasq --log-debug --log-dhcp --no-daemon --conf-dir "${dnsmasq_testdir}" &>>"$TMP_TESTDIR/dnsmasq.log" &
+  DNSMASQ_PID=$!
 }
 
 #
 #  stop_dhcp 27231
 #
 function stop_dhcp() {
-  run_helper cat "$DNSMASQ_PIDFILE"
-  kill -9 "$output"
+  echo "dnsmasq log:"
+  cat "${TMP_TESTDIR}/dnsmasq.log"
+  kill -9 "$DNSMASQ_PID"
 }
 
 function start_proxy() {
-  ip netns exec "$NS_NAME" $NETAVARK dhcp-proxy --dir "$TMP_TESTDIR" --uds "$TMP_TESTDIR"  &
+  RUST_LOG=info ip netns exec "$NS_NAME" $NETAVARK dhcp-proxy --dir "$TMP_TESTDIR" --uds "$TMP_TESTDIR" &>"$TMP_TESTDIR/proxy.log" &
   PROXY_PID=$!
 }
 
 function stop_proxy(){
+  echo "proxy log:"
+  cat "$TMP_TESTDIR/proxy.log"
   kill -9 $PROXY_PID
 }