From a8590744fbdd4bce9ab340ac49a7add31727b990 Mon Sep 17 00:00:00 2001 From: Gris Ge Date: Wed, 29 Jul 2020 17:51:27 +0800 Subject: [PATCH 1/3] nm applier: Fix external managed interface been marked as changed The net_state passing to `_mark_nm_external_subordinate_changed()` does not contain unknown type interface, so the `net_state.ifaces[subordinate]` could trigger KeyError as subordinate not found. Changed it to use `get()` and only perform this changes to desired/changed interface. Signed-off-by: Gris Ge --- libnmstate/nm/applier.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libnmstate/nm/applier.py b/libnmstate/nm/applier.py index 68d11dc..d0fb5f3 100644 --- a/libnmstate/nm/applier.py +++ b/libnmstate/nm/applier.py @@ -637,7 +637,7 @@ def _mark_nm_external_subordinate_changed(context, net_state): that subordinate should be marked as changed for NM to take over. """ for iface in net_state.ifaces.values(): - if iface.type in MASTER_IFACE_TYPES: + if iface.is_desired or iface.is_changed and iface.is_master: for subordinate in iface.slaves: nmdev = context.get_nm_dev(subordinate) if nmdev: @@ -647,5 +647,6 @@ def _mark_nm_external_subordinate_changed(context, net_state): and NM.ActivationStateFlags.EXTERNAL & nm_ac.get_state_flags() ): - subordinate_iface = net_state.ifaces[subordinate] - subordinate_iface.mark_as_changed() + subordinate_iface = net_state.ifaces.get(subordinate) + if subordinate_iface: + subordinate_iface.mark_as_changed() -- 2.28.0 From 77a05cfe726efc4a4207d57958a71e1730b6d62a Mon Sep 17 00:00:00 2001 From: Gris Ge Date: Wed, 29 Jul 2020 18:02:52 +0800 Subject: [PATCH 2/3] nm: Ignore externally managed interface for down/absent main interface When main interface been marked as down or absent, its subordinate should be also marked as so. NetworkManager plugin should ignore externally managed subordinate in this case. Signed-off-by: Gris Ge --- libnmstate/nm/applier.py | 26 +++++++++++--------------- libnmstate/nm/device.py | 8 ++++++++ 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/libnmstate/nm/applier.py b/libnmstate/nm/applier.py index d0fb5f3..9cd8f9a 100644 --- a/libnmstate/nm/applier.py +++ b/libnmstate/nm/applier.py @@ -50,6 +50,7 @@ from . import vxlan from . import wired from .common import NM from .dns import get_dns_config_iface_names +from .device import is_externally_managed MAXIMUM_INTERFACE_LENGTH = 15 @@ -265,13 +266,14 @@ def _set_ifaces_admin_state(context, ifaces_desired_state, con_profiles): == InterfaceState.ABSENT ) for affected_nmdev in nmdevs: - devs_to_deactivate[ - affected_nmdev.get_iface() - ] = affected_nmdev - if is_absent: - devs_to_delete_profile[ + if not is_externally_managed(affected_nmdev): + devs_to_deactivate[ affected_nmdev.get_iface() ] = affected_nmdev + if is_absent: + devs_to_delete_profile[ + affected_nmdev.get_iface() + ] = affected_nmdev if ( is_absent and nmdev.is_software() @@ -640,13 +642,7 @@ def _mark_nm_external_subordinate_changed(context, net_state): if iface.is_desired or iface.is_changed and iface.is_master: for subordinate in iface.slaves: nmdev = context.get_nm_dev(subordinate) - if nmdev: - nm_ac = nmdev.get_active_connection() - if ( - nm_ac - and NM.ActivationStateFlags.EXTERNAL - & nm_ac.get_state_flags() - ): - subordinate_iface = net_state.ifaces.get(subordinate) - if subordinate_iface: - subordinate_iface.mark_as_changed() + if nmdev and is_externally_managed(nmdev): + subordinate_iface = net_state.ifaces.get(subordinate) + if subordinate_iface: + subordinate_iface.mark_as_changed() diff --git a/libnmstate/nm/device.py b/libnmstate/nm/device.py index 528f57d..a175b71 100644 --- a/libnmstate/nm/device.py +++ b/libnmstate/nm/device.py @@ -23,6 +23,7 @@ from libnmstate.error import NmstateLibnmError from . import active_connection as ac from . import connection +from .common import NM def activate(context, dev=None, connection_id=None): @@ -161,3 +162,10 @@ def get_device_common_info(dev): "type_name": dev.get_type_description(), "state": dev.get_state(), } + + +def is_externally_managed(nmdev): + nm_ac = nmdev.get_active_connection() + return ( + nm_ac and NM.ActivationStateFlags.EXTERNAL & nm_ac.get_state_flags() + ) -- 2.28.0 From afb51e8421b8749962dd9ee2e31b61548de09a78 Mon Sep 17 00:00:00 2001 From: Gris Ge Date: Wed, 29 Jul 2020 18:22:32 +0800 Subject: [PATCH 3/3] state: Remove unmanaged interface before verifying Since we remove unknown type interface before sending to apply, we should also remove unknown type interface before verifying. Signed-off-by: Gris Ge --- libnmstate/ifaces/ifaces.py | 9 +++++---- libnmstate/nm/device.py | 4 +--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/libnmstate/ifaces/ifaces.py b/libnmstate/ifaces/ifaces.py index 1ff4198..a400712 100644 --- a/libnmstate/ifaces/ifaces.py +++ b/libnmstate/ifaces/ifaces.py @@ -286,16 +286,16 @@ class Ifaces: def cur_ifaces(self): return self._cur_ifaces - def _remove_unmanaged_slaves(self): + def _remove_unknown_interface_type_slaves(self): """ - When master containing unmanaged slaves, they should be removed from - master slave list. + When master containing slaves with unknown interface type, they should + be removed from master slave list before verifying. """ for iface in self._ifaces.values(): if iface.is_up and iface.is_master and iface.slaves: for slave_name in iface.slaves: slave_iface = self._ifaces[slave_name] - if not slave_iface.is_up: + if slave_iface.type == InterfaceType.UNKNOWN: iface.remove_slave(slave_name) def verify(self, cur_iface_infos): @@ -304,6 +304,7 @@ class Ifaces: cur_iface_infos=cur_iface_infos, save_to_disk=self._save_to_disk, ) + cur_ifaces._remove_unknown_interface_type_slaves() for iface in self._ifaces.values(): if iface.is_desired: if iface.is_virtual and iface.original_dict.get( diff --git a/libnmstate/nm/device.py b/libnmstate/nm/device.py index a175b71..fdf05bc 100644 --- a/libnmstate/nm/device.py +++ b/libnmstate/nm/device.py @@ -166,6 +166,4 @@ def get_device_common_info(dev): def is_externally_managed(nmdev): nm_ac = nmdev.get_active_connection() - return ( - nm_ac and NM.ActivationStateFlags.EXTERNAL & nm_ac.get_state_flags() - ) + return nm_ac and NM.ActivationStateFlags.EXTERNAL & nm_ac.get_state_flags() -- 2.28.0