tsarevvv/nmstate
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

brdige/bond: Add support of slaves back as deprecated

Browse Source 
With enough user complains, we would like to adding old property `slaves` back
to bond, linux bridge and ovs bridge as alias of `port`/`ports`.

A warning message will be emitted when this deprecated property been
used in YAML/JSON desire state.

Considering we never expose this property via Rust API, hence no need to
mark this internal property as deprecated in Rust API.

Unit and integration test case included.

Manually confirmed warning message will show when used, it is waste of
time to write test case to assert warning message in this case.

Signed-off-by: Gris Ge <fge@redhat.com>
This commit is contained in:
Gris Ge 2023-11-13 21:34:08 +08:00
parent ab07783b17
commit 631ff65558
12 changed files with 215 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
rust/src/lib/iface.rs
View File

@ -724,6 +724,15 @@ impl Interface {
iface.change_port_name(org_port_name, new_port_name);
}
}
pub(crate) fn post_deserialize_cleanup(&mut self) {
match self {
Interface::OvsBridge(iface) => iface.post_deserialize_cleanup(),
Interface::Bond(iface) => iface.post_deserialize_cleanup(),
Interface::LinuxBridge(iface) => iface.post_deserialize_cleanup(),
_ => (),
}
}
}
// The default on enum is experimental, but clippy is suggestion we use

25
rust/src/lib/ifaces/bond.rs
View File

@ -375,6 +375,12 @@ impl BondInterface {
}
ret
}
pub(crate) fn post_deserialize_cleanup(&mut self) {
if let Some(i) = self.bond.as_mut() {
i.post_deserialize_cleanup()
}
}
}
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy)]
@ -473,9 +479,16 @@ pub struct BondConfig {
pub options: Option<BondOptions>,
#[serde(skip_serializing_if = "Option::is_none", alias = "ports")]
/// Deserialize and serialize from/to `port`.
/// You can also use `ports` for deserializing.
/// You can also use `ports` or `slaves`(deprecated) for deserializing.
/// When applying, if defined, it will override current port list.
pub port: Option<Vec<String>>,
// Deprecated, please use `ports`, this is only for backwards compatibility
#[serde(
skip_serializing_if = "Option::is_none",
rename = "port",
alias = "slaves"
)]
pub(crate) slaves: Option<Vec<String>>,
#[serde(skip_serializing_if = "Option::is_none")]
/// Deserialize and serialize from/to `ports-config`.
/// When applying, if defined, it will override current ports
@ -490,6 +503,16 @@ impl BondConfig {
pub fn new() -> Self {
Self::default()
}
pub(crate) fn post_deserialize_cleanup(&mut self) {
if self.slaves.as_ref().is_some() {
log::warn!(
"The `slaves` is deprecated, please replace with `ports`."
);
self.port = self.slaves.clone();
self.slaves = None;
}
}
}
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy)]

8
rust/src/lib/ifaces/inter_ifaces.rs
View File

@ -623,6 +623,14 @@ impl MergedInterfaces {
MergedInterface,
> = HashMap::new();
for iface in desired
.kernel_ifaces
.values_mut()
.chain(desired.user_ifaces.values_mut())
{
iface.post_deserialize_cleanup();
}
if gen_conf_mode {
desired.set_unknown_iface_to_eth()?;
desired.set_missing_port_to_eth();

24
rust/src/lib/ifaces/linux_bridge.rs
View File

@ -340,6 +340,12 @@ impl LinuxBridgeInterface {
}
}
}
pub(crate) fn post_deserialize_cleanup(&mut self) {
if let Some(i) = self.bridge.as_mut() {
i.post_deserialize_cleanup()
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
@ -354,13 +360,31 @@ pub struct LinuxBridgeConfig {
#[serde(skip_serializing_if = "Option::is_none", alias = "ports")]
/// Linux bridge ports. When applying, desired port list will __override__
/// current port list.
/// Serialize to 'port'. Deserialize from `port` or `ports`.
pub port: Option<Vec<LinuxBridgePortConfig>>,
// Deprecated, please use `ports`, this is only for backwards compatibility
#[serde(
skip_serializing_if = "Option::is_none",
rename = "port",
alias = "slaves"
)]
pub(crate) slaves: Option<Vec<LinuxBridgePortConfig>>,
}
impl LinuxBridgeConfig {
pub fn new() -> Self {
Self::default()
}
pub(crate) fn post_deserialize_cleanup(&mut self) {
if self.slaves.as_ref().is_some() {
log::warn!(
"The `slaves` is deprecated, please replace with `ports`."
);
self.port = self.slaves.clone();
self.slaves = None;
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]

53
rust/src/lib/ifaces/ovs.rs
View File

@ -62,6 +62,12 @@ impl Default for OvsBridgeInterface {
}
impl OvsBridgeInterface {
pub(crate) fn post_deserialize_cleanup(&mut self) {
if let Some(i) = self.bridge.as_mut() {
i.post_deserialize_cleanup()
}
}
// Return None when desire state does not mention ports
pub(crate) fn ports(&self) -> Option<Vec<&str>> {
if let Some(br_conf) = &self.bridge {
@ -130,17 +136,10 @@ impl OvsBridgeInterface {
self.base.ipv6 = None;
self.sort_ports();
if let Some(port_confs) = self
.bridge
.as_ref()
.and_then(|br_conf| br_conf.ports.as_ref())
{
for port_conf in port_confs {
if let Some(vlan_conf) = port_conf.vlan.as_ref() {
vlan_conf.sanitize(is_desired)?;
}
}
if let Some(br_conf) = self.bridge.as_mut() {
br_conf.sanitize(is_desired)?;
}
Ok(())
}
@ -262,14 +261,46 @@ pub struct OvsBridgeConfig {
rename = "port",
alias = "ports"
)]
/// Serialize to 'port'. Deserialize from `port` or `ports`.
/// Serialize to 'port'. Deserialize from `port` or `ports` or
/// `slaves`(deprecated).
pub ports: Option<Vec<OvsBridgePortConfig>>,
// Deprecated, please use `ports`, this is only for backwards compatibility
#[serde(
skip_serializing_if = "Option::is_none",
rename = "port",
alias = "slaves"
)]
pub(crate) slaves: Option<Vec<OvsBridgePortConfig>>,
}
impl OvsBridgeConfig {
pub fn new() -> Self {
Self::default()
}
pub(crate) fn post_deserialize_cleanup(&mut self) {
if self.slaves.as_ref().is_some() {
log::warn!(
"The `slaves` is deprecated, please replace with `ports`."
);
self.ports = self.slaves.clone();
self.slaves = None;
}
}
pub(crate) fn sanitize(
&mut self,
is_desired: bool,
) -> Result<(), NmstateError> {
if let Some(port_confs) = self.ports.as_ref() {
for port_conf in port_confs {
if let Some(vlan_conf) = port_conf.vlan.as_ref() {
vlan_conf.sanitize(is_desired)?;
}
}
}
Ok(())
}
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]

20
rust/src/lib/unit_tests/bond.rs
View File

@ -598,3 +598,23 @@ fn test_disable_balance_slb_valid_override_current() {
MergedInterface::new(Some(des_iface), Some(cur_iface)).unwrap();
merged_iface.post_inter_ifaces_process_bond().unwrap();
}
#[test]
fn test_bond_deprecated_prop() {
let mut iface: BondInterface = serde_yaml::from_str(
r"---
name: bond99
type: bond
state: up
link-aggregation:
mode: balance-xor
slaves:
- eth1
- eth2",
)
.unwrap();
iface.post_deserialize_cleanup();
assert_eq!(iface.ports(), Some(vec!["eth1", "eth2"]));
}

20
rust/src/lib/unit_tests/bridge.rs
View File

@ -719,3 +719,23 @@ fn test_bridge_sanitize_group_forward_mask_and_group_fwd_mask() {
assert_eq!(desired_old, expected);
assert_eq!(desired_new, expected);
}
#[test]
fn test_linux_bridge_deprecated_prop() {
let mut iface: LinuxBridgeInterface = serde_yaml::from_str(
r"
name: br0
type: linux-bridge
state: up
bridge:
slaves:
- name: eth1
- name: eth2
",
)
.unwrap();
iface.post_deserialize_cleanup();
assert_eq!(iface.ports(), Some(vec!["eth1", "eth2"]));
}

20
rust/src/lib/unit_tests/ovs.rs
View File

@ -901,3 +901,23 @@ fn test_ovs_stp_option_as_bool() {
Some(true)
);
}
#[test]
fn test_ovs_bridge_deprecated_prop() {
let mut iface: OvsBridgeInterface = serde_yaml::from_str(
r"---
name: br0
type: ovs-bridge
state: up
bridge:
slaves:
- name: eth1
options:
stp: true",
)
.unwrap();
iface.post_deserialize_cleanup();
assert_eq!(iface.ports(), Some(vec!["eth1"]));
}

14
tests/integration/bond_test.py
View File

@ -1,5 +1,6 @@
# SPDX-License-Identifier: LGPL-2.1-or-later
import copy
import os
import time
@ -1348,3 +1349,16 @@ def test_change_bond_option_arp_missed_max(bond99_with_2_port):
bond_options["arp_missed_max"] = 200
libnmstate.apply(desired_state)
assertlib.assert_state_match(desired_state)
def test_bond_deprecated_prop(eth1_up, eth2_up):
with bond_interface(
name=BOND99, port=["eth1", "eth2"], create=False
) as state:
original_state = copy.deepcopy(state)
state[Interface.KEY][0][Bond.CONFIG_SUBTREE]["slaves"] = state[
Interface.KEY
][0][Bond.CONFIG_SUBTREE].pop(Bond.PORT)
libnmstate.apply(state)
assertlib.assert_state_match(original_state)

14
tests/integration/linux_bridge_test.py
View File

@ -1295,3 +1295,17 @@ def test_controller_detach_from_linux_bridge(bridge0_with_port0):
)
== 0
)
def test_linux_bridge_deprecated_prop(eth1_up, eth2_up):
bridge_state = _create_bridge_subtree_config(("eth1", "eth2"))
with linux_bridge(TEST_BRIDGE0, bridge_state, create=False) as state:
original_state = deepcopy(state)
state[Interface.KEY][0][LinuxBridge.CONFIG_SUBTREE]["slaves"] = state[
Interface.KEY
][0][LinuxBridge.CONFIG_SUBTREE].pop(LinuxBridge.PORT_SUBTREE)
libnmstate.apply(state)
assertlib.assert_state_match(original_state)

16
tests/integration/ovs_test.py
View File

@ -1,6 +1,7 @@
# SPDX-License-Identifier: LGPL-2.1-or-later
from contextlib import contextmanager
from copy import deepcopy
import os
import pytest
import yaml
@ -2220,3 +2221,18 @@ def test_raise_error_on_unknown_ovsdb_global_section():
def remove_ovn_state_present(state):
for ovn_map in state.get(Ovn.BRIDGE_MAPPINGS, []):
ovn_map.pop(Ovn.BridgeMappings.STATE, None)
def test_ovs_bridge_deprecated_prop(eth1_up):
bridge = Bridge(BRIDGE1)
bridge.add_system_port("eth1")
bridge.add_internal_port(PORT1, ipv4_state={InterfaceIPv4.ENABLED: False})
original_state = deepcopy(bridge.state)
bridge.bridge_iface[OVSBridge.CONFIG_SUBTREE][
"slaves"
] = bridge.bridge_iface[OVSBridge.CONFIG_SUBTREE].pop(
OVSBridge.PORT_SUBTREE
)
with bridge.create():
assertlib.assert_state_match(original_state)

4
tests/integration/testlib/ovslib.py
View File

@ -96,6 +96,10 @@ class Bridge:
def ports_names(self):
return [port[OVSBridge.Port.NAME] for port in self._get_ports()]
@property
def bridge_iface(self):
return self._bridge_iface
def _add_port(self, name):
self._bridge_iface[OVSBridge.CONFIG_SUBTREE].setdefault(
OVSBridge.PORT_SUBTREE, []