kubevirt: Support static gateway and DNS but dynamic address #1251

qinqon · 2025-11-26T10:22:24Z

Add support for configuring static gateway routes and DNS servers on DHCP-enabled interfaces in KubeVirt. This allows mixed configurations where IP addresses are obtained via DHCP but routing and DNS are statically configured.

gemini-code-assist

Code Review

This pull request adds support for configuring static gateways and DNS on DHCP-enabled interfaces in KubeVirt, which is a valuable feature for mixed network configurations. The implementation correctly handles this for both ConfigDrive and NoCloud data sources. The changes are well-tested with new test cases for various formats. My review focuses on improving code maintainability by addressing significant code duplication in both the network configuration logic and the newly added tests. By refactoring these areas, the code will be cleaner and easier to manage in the future.

gemini-code-assist · 2025-11-26T10:23:34Z

src/providers/kubevirt/nocloud.rs

+                // Add static gateway for DHCP if provided
+                if let Some(gateway) = &subnet.gateway {
+                    let gateway = IpAddr::from_str(gateway)?;
+                    let destination = IpNetwork::from_str("0.0.0.0/0")?;
+                    iface.routes.push(NetworkRoute {
+                        destination,
+                        gateway,
+                    });
+                }
            }
            if subnet.subnet_type == "dhcp6" {
                iface.dhcp = match iface.dhcp {
                    Some(DhcpSetting::V4) => Some(DhcpSetting::Both),
                    _ => Some(DhcpSetting::V6),
                };
+                // Add static gateway for DHCP6 if provided
+                if let Some(gateway) = &subnet.gateway {
+                    let gateway = IpAddr::from_str(gateway)?;
+                    let destination = IpNetwork::from_str("::/0")?;
+                    iface.routes.push(NetworkRoute {
+                        destination,
+                        gateway,
+                    });
+                }
            }


The logic for adding a static gateway for DHCP is duplicated for IPv4 (lines 312-319) and IPv6 (lines 327-334). This code can be refactored to reduce redundancy and improve maintainability.

For example, you could extract the common logic into a closure or a private helper method. The only difference is the destination network string ("0.0.0.0/0" vs "::/0"), which can be passed as an argument or determined based on a flag.

This will make the code cleaner and easier to modify in the future.

gemini-code-assist · 2025-11-26T10:23:34Z

src/providers/kubevirt/tests.rs

+#[test]
+fn test_dhcp_with_static_gateway_and_dns() {
+    let fixture_path = "./tests/fixtures/kubevirt/dhcp_static_gw_dns";
+    let config = KubeVirtCloudConfig::try_new(
+        Path::new(fixture_path),
+        NetworkConfigurationFormat::ConfigDrive,
+    )
+    .expect("cannot parse config");
+
+    let interfaces = config.networks().expect("cannot get interfaces");
+    assert_eq!(
+        interfaces.len(),
+        1,
+        "Expected 1 interface, got {} interfaces: {:?}",
+        interfaces.len(),
+        interfaces
+    );
+
+    let eth0 = &interfaces[0];
+    assert_eq!(
+        eth0.name,
+        Some("eth0".to_string()),
+        "Expected eth0.name to be {:?}, got {:?}",
+        Some("eth0".to_string()),
+        eth0.name
+    );
+    assert_eq!(
+        eth0.dhcp,
+        Some(DhcpSetting::Both),
+        "Expected eth0.dhcp to be {:?}, got {:?}",
+        Some(DhcpSetting::Both),
+        eth0.dhcp
+    );
+    assert_eq!(
+        eth0.ip_addresses.len(),
+        0,
+        "Expected eth0 to have 0 static IP addresses (using DHCP), got {} addresses: {:?}",
+        eth0.ip_addresses.len(),
+        eth0.ip_addresses
+    );
+    assert_eq!(
+        eth0.routes.len(),
+        2,
+        "Expected eth0 to have 2 static routes (IPv4 and IPv6 default gateways), got {} routes: {:?}",
+        eth0.routes.len(),
+        eth0.routes
+    );
+    assert!(
+        eth0.routes
+            .iter()
+            .any(|r| r.gateway == IpAddr::from_str("192.168.1.1").unwrap()),
+        "Expected eth0.routes to contain gateway 192.168.1.1, but got routes: {:?}",
+        eth0.routes
+    );
+    assert!(
+        eth0.routes
+            .iter()
+            .any(|r| r.gateway == IpAddr::from_str("2001:db8::1").unwrap()),
+        "Expected eth0.routes to contain gateway 2001:db8::1, but got routes: {:?}",
+        eth0.routes
+    );
+    assert_eq!(
+        eth0.nameservers.len(),
+        2,
+        "Expected eth0 to have 2 nameservers, got {} nameservers: {:?}",
+        eth0.nameservers.len(),
+        eth0.nameservers
+    );
+    assert!(
+        eth0.nameservers
+            .contains(&IpAddr::from_str("8.8.8.8").unwrap()),
+        "Expected eth0.nameservers to contain 8.8.8.8, but got: {:?}",
+        eth0.nameservers
+    );
+    assert!(
+        eth0.nameservers
+            .contains(&IpAddr::from_str("8.8.4.4").unwrap()),
+        "Expected eth0.nameservers to contain 8.8.4.4, but got: {:?}",
+        eth0.nameservers
+    );
+
+    // Test the kernel arguments generation
+    let kargs = config.rd_network_kargs().unwrap().unwrap();
+    let kargs_parts: Vec<&str> = kargs.split_whitespace().collect();
+
+    // Expected parts:
+    // 1. ip=eth0:dhcp,dhcp6
+    // 2. rd.route=0.0.0.0/0:192.168.1.1
+    // 3. rd.route=::/0:2001:db8::1
+    // 4. nameserver=8.8.8.8
+    // 5. nameserver=8.8.4.4
+    assert_eq!(
+        kargs_parts.len(),
+        5,
+        "Expected kargs to have 5 parts (1 dhcp + 2 routes + 2 nameservers), got {} parts: {:?}",
+        kargs_parts.len(),
+        kargs_parts
+    );
+
+    assert!(
+        kargs.contains("ip=eth0:dhcp,dhcp6"),
+        "Expected kargs to contain 'ip=eth0:dhcp,dhcp6', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("rd.route=0.0.0.0/0:192.168.1.1"),
+        "Expected kargs to contain 'rd.route=0.0.0.0/0:192.168.1.1', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("rd.route=::/0:2001:db8::1"),
+        "Expected kargs to contain 'rd.route=::/0:2001:db8::1', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("nameserver=8.8.8.8"),
+        "Expected kargs to contain 'nameserver=8.8.8.8', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("nameserver=8.8.4.4"),
+        "Expected kargs to contain 'nameserver=8.8.4.4', but got: {:?}",
+        kargs
+    );
+}
+
+#[test]
+fn test_dhcp_with_static_gateway_and_dns_nocloud_v1() {
+    let fixture_path = "./tests/fixtures/kubevirt/dhcp_static_gw_dns_nocloud_v1";
+    let config = KubeVirtCloudConfig::try_new(
+        Path::new(fixture_path),
+        NetworkConfigurationFormat::NoCloud,
+    )
+    .expect("cannot parse config");
+
+    let interfaces = config.networks().expect("cannot get interfaces");
+    assert_eq!(
+        interfaces.len(),
+        1,
+        "Expected 1 interface, got {} interfaces: {:?}",
+        interfaces.len(),
+        interfaces
+    );
+
+    let eth0 = &interfaces[0];
+    assert_eq!(
+        eth0.name,
+        Some("eth0".to_string()),
+        "Expected eth0.name to be {:?}, got {:?}",
+        Some("eth0".to_string()),
+        eth0.name
+    );
+    assert_eq!(
+        eth0.dhcp,
+        Some(DhcpSetting::Both),
+        "Expected eth0.dhcp to be {:?}, got {:?}",
+        Some(DhcpSetting::Both),
+        eth0.dhcp
+    );
+    assert_eq!(
+        eth0.ip_addresses.len(),
+        0,
+        "Expected eth0 to have 0 static IP addresses (using DHCP), got {} addresses: {:?}",
+        eth0.ip_addresses.len(),
+        eth0.ip_addresses
+    );
+    assert_eq!(
+        eth0.routes.len(),
+        2,
+        "Expected eth0 to have 2 static routes (IPv4 and IPv6 default gateways), got {} routes: {:?}",
+        eth0.routes.len(),
+        eth0.routes
+    );
+    assert!(
+        eth0.routes
+            .iter()
+            .any(|r| r.gateway == IpAddr::from_str("192.168.1.1").unwrap()),
+        "Expected eth0.routes to contain gateway 192.168.1.1, but got routes: {:?}",
+        eth0.routes
+    );
+    assert!(
+        eth0.routes
+            .iter()
+            .any(|r| r.gateway == IpAddr::from_str("2001:db8::1").unwrap()),
+        "Expected eth0.routes to contain gateway 2001:db8::1, but got routes: {:?}",
+        eth0.routes
+    );
+    assert_eq!(
+        eth0.nameservers.len(),
+        2,
+        "Expected eth0 to have 2 nameservers, got {} nameservers: {:?}",
+        eth0.nameservers.len(),
+        eth0.nameservers
+    );
+    assert!(
+        eth0.nameservers
+            .contains(&IpAddr::from_str("8.8.8.8").unwrap()),
+        "Expected eth0.nameservers to contain 8.8.8.8, but got: {:?}",
+        eth0.nameservers
+    );
+    assert!(
+        eth0.nameservers
+            .contains(&IpAddr::from_str("8.8.4.4").unwrap()),
+        "Expected eth0.nameservers to contain 8.8.4.4, but got: {:?}",
+        eth0.nameservers
+    );
+
+    // Test the kernel arguments generation
+    let kargs = config.rd_network_kargs().unwrap().unwrap();
+    let kargs_parts: Vec<&str> = kargs.split_whitespace().collect();
+
+    // Expected parts:
+    // 1. ip=eth0:dhcp,dhcp6
+    // 2. rd.route=0.0.0.0/0:192.168.1.1
+    // 3. rd.route=::/0:2001:db8::1
+    // 4. nameserver=8.8.8.8
+    // 5. nameserver=8.8.4.4
+    assert_eq!(
+        kargs_parts.len(),
+        5,
+        "Expected kargs to have 5 parts (1 dhcp + 2 routes + 2 nameservers), got {} parts: {:?}",
+        kargs_parts.len(),
+        kargs_parts
+    );
+
+    assert!(
+        kargs.contains("ip=eth0:dhcp,dhcp6"),
+        "Expected kargs to contain 'ip=eth0:dhcp,dhcp6', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("rd.route=0.0.0.0/0:192.168.1.1"),
+        "Expected kargs to contain 'rd.route=0.0.0.0/0:192.168.1.1', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("rd.route=::/0:2001:db8::1"),
+        "Expected kargs to contain 'rd.route=::/0:2001:db8::1', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("nameserver=8.8.8.8"),
+        "Expected kargs to contain 'nameserver=8.8.8.8', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("nameserver=8.8.4.4"),
+        "Expected kargs to contain 'nameserver=8.8.4.4', but got: {:?}",
+        kargs
+    );
+}
+
+#[test]
+fn test_dhcp_with_static_gateway_and_dns_nocloud_v2() {
+    let fixture_path = "./tests/fixtures/kubevirt/dhcp_static_gw_dns_nocloud_v2";
+    let config = KubeVirtCloudConfig::try_new(
+        Path::new(fixture_path),
+        NetworkConfigurationFormat::NoCloud,
+    )
+    .expect("cannot parse config");
+
+    let interfaces = config.networks().expect("cannot get interfaces");
+    assert_eq!(
+        interfaces.len(),
+        1,
+        "Expected 1 interface, got {} interfaces: {:?}",
+        interfaces.len(),
+        interfaces
+    );
+
+    let eth0 = &interfaces[0];
+    assert_eq!(
+        eth0.name,
+        Some("eth0".to_string()),
+        "Expected eth0.name to be {:?}, got {:?}",
+        Some("eth0".to_string()),
+        eth0.name
+    );
+    assert_eq!(
+        eth0.dhcp,
+        Some(DhcpSetting::Both),
+        "Expected eth0.dhcp to be {:?}, got {:?}",
+        Some(DhcpSetting::Both),
+        eth0.dhcp
+    );
+    assert_eq!(
+        eth0.ip_addresses.len(),
+        0,
+        "Expected eth0 to have 0 static IP addresses (using DHCP), got {} addresses: {:?}",
+        eth0.ip_addresses.len(),
+        eth0.ip_addresses
+    );
+    assert_eq!(
+        eth0.routes.len(),
+        2,
+        "Expected eth0 to have 2 static routes (IPv4 and IPv6 default gateways), got {} routes: {:?}",
+        eth0.routes.len(),
+        eth0.routes
+    );
+    assert!(
+        eth0.routes
+            .iter()
+            .any(|r| r.gateway == IpAddr::from_str("192.168.1.1").unwrap()),
+        "Expected eth0.routes to contain gateway 192.168.1.1, but got routes: {:?}",
+        eth0.routes
+    );
+    assert!(
+        eth0.routes
+            .iter()
+            .any(|r| r.gateway == IpAddr::from_str("2001:db8::1").unwrap()),
+        "Expected eth0.routes to contain gateway 2001:db8::1, but got routes: {:?}",
+        eth0.routes
+    );
+    assert_eq!(
+        eth0.nameservers.len(),
+        2,
+        "Expected eth0 to have 2 nameservers, got {} nameservers: {:?}",
+        eth0.nameservers.len(),
+        eth0.nameservers
+    );
+    assert!(
+        eth0.nameservers
+            .contains(&IpAddr::from_str("8.8.8.8").unwrap()),
+        "Expected eth0.nameservers to contain 8.8.8.8, but got: {:?}",
+        eth0.nameservers
+    );
+    assert!(
+        eth0.nameservers
+            .contains(&IpAddr::from_str("8.8.4.4").unwrap()),
+        "Expected eth0.nameservers to contain 8.8.4.4, but got: {:?}",
+        eth0.nameservers
+    );
+
+    // Test the kernel arguments generation
+    let kargs = config.rd_network_kargs().unwrap().unwrap();
+    let kargs_parts: Vec<&str> = kargs.split_whitespace().collect();
+
+    // Expected parts:
+    // 1. ip=eth0:dhcp,dhcp6
+    // 2. rd.route=0.0.0.0/0:192.168.1.1
+    // 3. rd.route=::/0:2001:db8::1
+    // 4. nameserver=8.8.8.8
+    // 5. nameserver=8.8.4.4
+    assert_eq!(
+        kargs_parts.len(),
+        5,
+        "Expected kargs to have 5 parts (1 dhcp + 2 routes + 2 nameservers), got {} parts: {:?}",
+        kargs_parts.len(),
+        kargs_parts
+    );
+
+    assert!(
+        kargs.contains("ip=eth0:dhcp,dhcp6"),
+        "Expected kargs to contain 'ip=eth0:dhcp,dhcp6', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("rd.route=0.0.0.0/0:192.168.1.1"),
+        "Expected kargs to contain 'rd.route=0.0.0.0/0:192.168.1.1', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("rd.route=::/0:2001:db8::1"),
+        "Expected kargs to contain 'rd.route=::/0:2001:db8::1', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("nameserver=8.8.8.8"),
+        "Expected kargs to contain 'nameserver=8.8.8.8', but got: {:?}",
+        kargs
+    );
+    assert!(
+        kargs.contains("nameserver=8.8.4.4"),
+        "Expected kargs to contain 'nameserver=8.8.4.4', but got: {:?}",
+        kargs
+    );
+}


These three new test functions (test_dhcp_with_static_gateway_and_dns, test_dhcp_with_static_gateway_and_dns_nocloud_v1, and test_dhcp_with_static_gateway_and_dns_nocloud_v2) are almost identical and contain a lot of duplicated assertion logic.

To improve maintainability and reduce code duplication, consider creating a single helper function that takes a &KubeVirtCloudConfig and performs all the common assertions. Each test would then just be responsible for setting up its specific configuration and calling this shared helper.

Example of a shared assertion function signature:

fn assert_dhcp_with_static_gw_and_dns(config: &KubeVirtCloudConfig) { // All the common assertion logic goes here }

This would make the test suite much cleaner and easier to manage.

Add support for configuring static gateway routes and DNS servers on DHCP-enabled interfaces in KubeVirt. This allows mixed configurations where IP addresses are obtained via DHCP but routing and DNS are statically configured. Signed-off-by: Enrique Llorente <[email protected]>

prestist

One small question, otherwise it LGTM from my perspective.

@travier would you mind also taking a look as well :)

prestist · 2025-12-16T20:47:32Z

src/providers/kubevirt/configdrive.rs

+                        should_configure_static_dns = true;
+                    }
                }
                "ipv6_dhcp" => {


Hmm so really accept_dhcp_option is IPv4 limited right? Is this a technical difference or intentional absence of functionality for ipv6

gemini-code-assist bot reviewed Nov 26, 2025

View reviewed changes

qinqon force-pushed the kubevirt-network-dynamic-address-static-gw-dns branch 3 times, most recently from bcbf302 to cde6a31 Compare November 26, 2025 14:05

qinqon changed the title ~~kubevirt: Support static gateway and DNS with DHCP~~ kubevirt: Support static gateway and DNS but dynamic address Nov 26, 2025

qinqon force-pushed the kubevirt-network-dynamic-address-static-gw-dns branch from cde6a31 to bd09cb1 Compare November 26, 2025 16:18

qinqon mentioned this pull request Nov 27, 2025

spike: Kubevirt networkdata openshift/hypershift#7299

Draft

qinqon force-pushed the kubevirt-network-dynamic-address-static-gw-dns branch from bd09cb1 to 6ac7a1b Compare December 16, 2025 09:49

qinqon force-pushed the kubevirt-network-dynamic-address-static-gw-dns branch from 6ac7a1b to dc360a0 Compare December 16, 2025 09:58

prestist reviewed Dec 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

kubevirt: Support static gateway and DNS but dynamic address #1251

kubevirt: Support static gateway and DNS but dynamic address #1251

qinqon commented Nov 26, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Nov 26, 2025

Uh oh!

gemini-code-assist bot Nov 26, 2025

Uh oh!

prestist left a comment

Uh oh!

prestist Dec 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kubevirt: Support static gateway and DNS but dynamic address #1251

Are you sure you want to change the base?

kubevirt: Support static gateway and DNS but dynamic address #1251

Conversation

qinqon commented Nov 26, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

prestist left a comment

Choose a reason for hiding this comment

Uh oh!

prestist Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants