nm dns: Fix error when moving dns from port to controller

When moving IP and DNS config from port to controller, nmstate will fail
with verification error as nmstate failed to store DNS config to
controller.

Fixed by:
 * `apply_ctrller_change()` should purge IP stack if interface is not
   valid to IP any more in merge state.

 * `store_dns_config()` should reconfig the DNS if changed interface is
   DNS interface and not valid for current DNS anymore even DNS config
   not changed.

 * `purge_dns_config()` should do nothing for absent interface.

Integration test case crated for two use cases:
 * Move identical IP and DNS config from port to controller.
 * Move identical IP from port to controller but with DNS changed.

Signed-off-by: Gris Ge <fge@redhat.com>

rust/src/lib/iface.rs

@@ -973,8 +973,14 @@ impl MergedInterface {
                 verify_iface.base_iface_mut().state = InterfaceState::Absent;
             }
         } else {
-            apply_iface.base_iface_mut().controller = Some(ctrl_name);
-            apply_iface.base_iface_mut().controller_type = ctrl_type;
+            apply_iface.base_iface_mut().controller = Some(ctrl_name.clone());
+            apply_iface.base_iface_mut().controller_type = ctrl_type.clone();
+            self.merged.base_iface_mut().controller = Some(ctrl_name);
+            self.merged.base_iface_mut().controller_type = ctrl_type;
+            if !self.merged.base_iface().can_have_ip() {
+                self.merged.base_iface_mut().ipv4 = None;
+                self.merged.base_iface_mut().ipv6 = None;
+            }
         }
         Ok(())
     }

rust/src/lib/nm/dns.rs

@@ -11,7 +11,9 @@ const DEFAULT_DNS_PRIORITY: i32 = 40;
 pub(crate) fn store_dns_config(
     merged_state: &mut MergedNetworkState,
 ) -> Result<(), NmstateError> {
-    if merged_state.dns.is_changed() {
+    if merged_state.dns.is_changed()
+        || !cur_dns_ifaces_still_valid_for_dns(&merged_state.interfaces)
+    {
         let (cur_v4_ifaces, cur_v6_ifaces) =
             get_cur_dns_ifaces(&merged_state.interfaces);
         let (v4_iface_name, v6_iface_name) = reselect_dns_ifaces(
@@ -113,23 +115,30 @@ pub(crate) fn purge_dns_config(
         if let Some(iface) =
             merged_state.interfaces.kernel_ifaces.get_mut(iface_name)
         {
+            if iface.merged.is_absent() {
+                return;
+            }
             if !iface.is_changed() {
                 iface.mark_as_changed();
                 if let Some(apply_iface) = iface.for_apply.as_mut() {
-                    apply_iface.base_iface_mut().ipv4 =
-                        iface.merged.base_iface_mut().ipv4.clone();
-                    apply_iface.base_iface_mut().ipv6 =
-                        iface.merged.base_iface_mut().ipv6.clone();
+                    if apply_iface.base_iface().can_have_ip() {
+                        apply_iface.base_iface_mut().ipv4 =
+                            iface.merged.base_iface_mut().ipv4.clone();
+                        apply_iface.base_iface_mut().ipv6 =
+                            iface.merged.base_iface_mut().ipv6.clone();
+                    }
                 }
             }
             if let Some(apply_iface) = iface.for_apply.as_mut() {
-                set_iface_dns_conf(
-                    is_ipv6,
-                    apply_iface,
-                    Vec::new(),
-                    Vec::new(),
-                    None,
-                );
+                if apply_iface.base_iface().can_have_ip() {
+                    set_iface_dns_conf(
+                        is_ipv6,
+                        apply_iface,
+                        Vec::new(),
+                        Vec::new(),
+                        None,
+                    );
+                }
             }
         }
     }
@@ -263,13 +272,13 @@ fn set_iface_dns_conf(
     if is_ipv6 {
         if let Some(ip_conf) = iface.base_iface_mut().ipv6.as_mut() {
             ip_conf.dns = Some(dns_conf);
-        } else {
+        } else if !dns_conf.is_purge() {
             // Should never happen
             log::error!("BUG: The dns interface is hold None IP {:?}", iface);
         }
     } else if let Some(ip_conf) = iface.base_iface_mut().ipv4.as_mut() {
         ip_conf.dns = Some(dns_conf);
-    } else {
+    } else if !dns_conf.is_purge() {
         // Should never happen
         log::error!("BUG: The dns interface is hold None IP {:?}", iface);
     }
@@ -316,3 +325,24 @@ fn get_cur_dns_ifaces(
     }
     (v4_ifaces, v6_ifaces)
 }
+
+fn cur_dns_ifaces_still_valid_for_dns(
+    merged_ifaces: &MergedInterfaces,
+) -> bool {
+    let (cur_v4_ifaces, cur_v6_ifaces) = get_cur_dns_ifaces(merged_ifaces);
+    for iface_name in &cur_v4_ifaces {
+        if let Some(iface) = merged_ifaces.kernel_ifaces.get(iface_name) {
+            if iface.is_changed() && !iface.is_iface_valid_for_dns(false) {
+                return false;
+            }
+        }
+    }
+    for iface_name in &cur_v6_ifaces {
+        if let Some(iface) = merged_ifaces.kernel_ifaces.get(iface_name) {
+            if iface.is_changed() && !iface.is_iface_valid_for_dns(true) {
+                return false;
+            }
+        }
+    }
+    true
+}

tests/integration/dns_test.py

@@ -1,21 +1,4 @@
-#
-# Copyright (c) 2019-2021 Red Hat, Inc.
-#
-# This file is part of nmstate
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU Lesser General Public License as published by
-# the Free Software Foundation, either version 2.1 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU Lesser General Public License for more details.
-#
-# You should have received a copy of the GNU Lesser General Public License
-# along with this program. If not, see <https://www.gnu.org/licenses/>.
-#
+# SPDX-License-Identifier: LGPL-2.1-or-later
 
 import json
 
@@ -31,7 +14,9 @@ from libnmstate.schema import InterfaceState
 from libnmstate.schema import InterfaceType
 from libnmstate.schema import Route
 
+from .testlib import assertlib
 from .testlib import cmdlib
+from .testlib.bondlib import bond_interface
 from .testlib.genconf import gen_conf_apply
 
 IPV4_DNS_NAMESERVERS = ["8.8.8.8", "1.1.1.1"]
@@ -41,6 +26,7 @@ IPV6_DNS_LONG_NAMESERVER = ["2000:0000:0000:0000:0000:0000:0000:0100"]
 EXTRA_IPV6_DNS_NAMESERVER = "2620:fe::9"
 EXAMPLE_SEARCHES = ["example.org", "example.com"]
 EXAMPLE_SEARCHES2 = ["example.info", "example.org"]
+TEST_BOND0 = "test-bond0"
 
 parametrize_ip_ver = pytest.mark.parametrize(
     "dns_config",
@@ -397,7 +383,7 @@ def _gen_default_gateway_route():
 
 
 @pytest.fixture
-def static_dns():
+def static_dns(eth1_up):
     desired_state = {
         Interface.KEY: _get_test_iface_states(),
         Route.KEY: {Route.CONFIG: _gen_default_gateway_route()},
@@ -410,6 +396,7 @@ def static_dns():
     }
     libnmstate.apply(desired_state)
     yield desired_state
+    libnmstate.apply({DNS.KEY: {DNS.CONFIG: {}}})
 
 
 @pytest.mark.tier1
@@ -461,3 +448,53 @@ def test_dns_edit_nameserver_with_static_gateway_genconf(dns_config):
     with gen_conf_apply(desired_state):
         current_state = libnmstate.show()
         assert dns_config == current_state[DNS.KEY][DNS.CONFIG]
+
+
+def test_move_dns_from_port_to_controller(static_dns, eth2_up):
+    with bond_interface(
+        name=TEST_BOND0, port=["eth1", "eth2"], create=False
+    ) as state:
+        state[Route.KEY] = static_dns[Route.KEY]
+        for route in state[Route.KEY][Route.CONFIG]:
+            route[Route.NEXT_HOP_INTERFACE] = TEST_BOND0
+
+        state[DNS.KEY] = static_dns[DNS.KEY]
+        state[Interface.KEY][0][Interface.IPV4] = static_dns[Interface.KEY][0][
+            Interface.IPV4
+        ]
+        state[Interface.KEY][0][Interface.IPV6] = static_dns[Interface.KEY][0][
+            Interface.IPV6
+        ]
+        libnmstate.apply(state)
+        current_state = libnmstate.show()
+
+        assertlib.assert_state_match(state)
+        assert state[DNS.KEY][DNS.CONFIG] == current_state[DNS.KEY][DNS.CONFIG]
+        # Remove DNS before deleting bond
+        libnmstate.apply({DNS.KEY: {DNS.CONFIG: {}}})
+
+
+def test_changed_dns_from_port_to_controller(static_dns, eth2_up):
+    with bond_interface(
+        name=TEST_BOND0, port=["eth1", "eth2"], create=False
+    ) as state:
+        state[Route.KEY] = static_dns[Route.KEY]
+        for route in state[Route.KEY][Route.CONFIG]:
+            route[Route.NEXT_HOP_INTERFACE] = TEST_BOND0
+
+        state[DNS.KEY] = static_dns[DNS.KEY]
+        state[DNS.KEY][DNS.CONFIG][DNS.SEARCH].reverse()
+        state[DNS.KEY][DNS.CONFIG][DNS.SERVER].reverse()
+        state[Interface.KEY][0][Interface.IPV4] = static_dns[Interface.KEY][0][
+            Interface.IPV4
+        ]
+        state[Interface.KEY][0][Interface.IPV6] = static_dns[Interface.KEY][0][
+            Interface.IPV6
+        ]
+        libnmstate.apply(state)
+        current_state = libnmstate.show()
+
+        assertlib.assert_state_match(state)
+        assert state[DNS.KEY][DNS.CONFIG] == current_state[DNS.KEY][DNS.CONFIG]
+        # Remove DNS before deleting bond
+        libnmstate.apply({DNS.KEY: {DNS.CONFIG: {}}})

tests/integration/testlib/genconf.py

@@ -4,6 +4,7 @@ import os
 from contextlib import contextmanager
 
 import libnmstate
+from libnmstate.schema import DNS
 from libnmstate.schema import Interface
 from libnmstate.schema import InterfaceState
 
@@ -28,7 +29,7 @@ def gen_conf_apply(desire_state):
         activate_all_nm_connections()
         yield
     finally:
-        absent_state = {Interface.KEY: []}
+        absent_state = {DNS.KEY: {DNS.CONFIG: {}}, Interface.KEY: []}
         for iface_name in iface_names:
             absent_state[Interface.KEY].append(
                 {