Skip to content

Commit

Permalink
nm: Preserve current IP setting for multiconnect profile
Browse files Browse the repository at this point in the history
Given a interface is activated by a NM profile of multiconnect,
When applying empty state for this interface,
Then this interface will get IPv4 and IPv6 disabled.

This is because current nmstate code treat multiconnect or NM connection
without interface name as not matched, hence `exist_nm_conn` is set to
None which lead to nmstate discarding current IP settings.

Cloning multiconnect profiles could have risk when NM added more
properties impacting activation but nmstate not aware.

Hence we create NM connection from scratch in this case by only
preserving IP settings from multiconnect. Other non-ip settings applied
by multiconnect profile will be discard if not mentioned in desired
state.

Integration test case included.

Resolves: https://issues.redhat.com/browse/RHEL-61890

Signed-off-by: Gris Ge <[email protected]>
  • Loading branch information
cathay4t committed Oct 9, 2024
1 parent 62c5729 commit 23247af
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 24 deletions.
71 changes: 47 additions & 24 deletions rust/src/lib/nm/settings/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,31 +43,33 @@ pub(crate) fn iface_to_nm_connections(
) -> Result<Vec<NmConnection>, NmstateError> {
let mut ret: Vec<NmConnection> = Vec::new();

let iface = if let Some(i) = merged_iface.for_apply.as_ref() {
i
let mut iface = if let Some(i) = merged_iface.for_apply.as_ref() {
i.clone()
} else {
return Ok(ret);
};

let base_iface = iface.base_iface();
let exist_nm_conn =
if base_iface.identifier == Some(InterfaceIdentifier::MacAddress) {
get_exist_profile_by_profile_name(
exist_nm_conns,
base_iface
.profile_name
.as_deref()
.unwrap_or(base_iface.name.as_str()),
&base_iface.iface_type,
)
} else {
get_exist_profile(
exist_nm_conns,
&base_iface.name,
&base_iface.iface_type,
nm_ac_uuids,
)
};
let exist_nm_conn = if iface.base_iface().identifier
== Some(InterfaceIdentifier::MacAddress)
{
get_exist_profile_by_profile_name(
exist_nm_conns,
iface
.base_iface()
.profile_name
.as_deref()
.unwrap_or(iface.base_iface().name.as_str()),
&iface.base_iface().iface_type,
)
} else {
get_exist_profile(
exist_nm_conns,
&iface.base_iface().name,
&iface.base_iface().iface_type,
nm_ac_uuids,
)
};

if iface.is_up_exist_config() {
if let Some(nm_conn) = exist_nm_conn {
if !iface.is_userspace()
Expand Down Expand Up @@ -102,6 +104,16 @@ pub(crate) fn iface_to_nm_connections(
}
}
}

// If exist_nm_conn is None and desired state did not mention IP settings,
// we are supported to preserve current IP state instead of setting ipv4 and
// ipv6 disabled.
if exist_nm_conn.is_none() {
preserve_current_ip(&mut iface, merged_iface.current.as_ref());
}

let iface = &iface;

let mut nm_conn = exist_nm_conn.cloned().unwrap_or_default();
nm_conn.flags = Vec::new();

Expand Down Expand Up @@ -243,8 +255,8 @@ pub(crate) fn iface_to_nm_connections(
}

if let (Some(ctrl), Some(ctrl_type)) = (
base_iface.controller.as_ref(),
base_iface.controller_type.as_ref(),
iface.base_iface().controller.as_ref(),
iface.base_iface().controller_type.as_ref(),
) {
if let Some(ctrl_iface) =
merged_state.interfaces.get_iface(ctrl, ctrl_type.clone())
Expand Down Expand Up @@ -307,7 +319,7 @@ pub(crate) fn iface_to_nm_connections(

// When detaching a OVS system interface from OVS bridge, we should remove
// its NmSettingOvsIface setting
if base_iface.controller.as_deref() == Some("") {
if iface.base_iface().controller.as_deref() == Some("") {
nm_conn.ovs_iface = None;
}

Expand Down Expand Up @@ -545,3 +557,14 @@ fn persisten_iface_cur_conf(
gen_conf_mode,
)
}

fn preserve_current_ip(iface: &mut Interface, cur_iface: Option<&Interface>) {
if iface.base_iface().ipv4.is_none() {
iface.base_iface_mut().ipv4 =
cur_iface.as_ref().and_then(|i| i.base_iface().ipv4.clone());
}
if iface.base_iface().ipv6.is_none() {
iface.base_iface_mut().ipv6 =
cur_iface.as_ref().and_then(|i| i.base_iface().ipv6.clone());
}
}
36 changes: 36 additions & 0 deletions tests/integration/nm/profile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,3 +676,39 @@ def test_change_profile_name(eth1_up):
].strip()
== "eth1-new"
)


@pytest.fixture
def multiconnect_profile_with_ip_enabled(eth1_up):
cmdlib.exec_cmd("nmcli c del eth1".split(), check=True)
cmdlib.exec_cmd(
"nmcli c add type ethernet connection.id nmstate-test-default "
"connection.multi-connect multiple "
"ipv4.method auto ipv6.method auto".split(),
check=True,
)
yield
cmdlib.exec_cmd("nmcli c del nmstate-test-default".split(), check=False)


def test_preserve_ip_settings_from_multiconnect(
multiconnect_profile_with_ip_enabled,
):
desired_state = {
Interface.KEY: [
{
Interface.NAME: "eth1",
Interface.TYPE: InterfaceType.ETHERNET,
Interface.STATE: InterfaceState.UP,
}
]
}
libnmstate.apply(desired_state)
assert (
cmdlib.exec_cmd("nmcli -g ipv4.method c show eth1".split())[1].strip()
== "auto"
)
assert (
cmdlib.exec_cmd("nmcli -g ipv6.method c show eth1".split())[1].strip()
== "auto"
)

0 comments on commit 23247af

Please sign in to comment.