From 44efa8fcf66a37f438f02f96b3428d0e43e3e508 Mon Sep 17 00:00:00 2001 From: Packit Service Date: Feb 24 2021 15:30:14 +0000 Subject: Prepare for a new update Reverting patches so we can apply the latest update and changes can be seen in the spec file and sources. --- diff --git a/libnmstate/ifaces/ifaces.py b/libnmstate/ifaces/ifaces.py index 703e672..1ff4198 100644 --- a/libnmstate/ifaces/ifaces.py +++ b/libnmstate/ifaces/ifaces.py @@ -217,8 +217,7 @@ class Ifaces: self._ifaces[iface_name].mark_as_changed() if cur_iface: for slave_name in iface.config_changed_slaves(cur_iface): - if slave_name in self._ifaces: - self._ifaces[slave_name].mark_as_changed() + self._ifaces[slave_name].mark_as_changed() def _match_child_iface_state_with_parent(self): """ @@ -242,10 +241,8 @@ class Ifaces: def _mark_orphen_as_absent(self): for iface in self._ifaces.values(): - if ( - iface.need_parent - and (not iface.parent or not self._ifaces.get(iface.parent)) - and (iface.is_desired or iface.is_changed) + if iface.need_parent and ( + not iface.parent or not self._ifaces.get(iface.parent) ): iface.mark_as_changed() iface.state = InterfaceState.ABSENT @@ -289,16 +286,16 @@ class Ifaces: def cur_ifaces(self): return self._cur_ifaces - def _remove_unknown_interface_type_slaves(self): + def _remove_unmanaged_slaves(self): """ - When master containing slaves with unknown interface type, they should - be removed from master slave list before verifying. + When master containing unmanaged slaves, they should be removed from + master slave list. """ 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 slave_iface.type == InterfaceType.UNKNOWN: + if not slave_iface.is_up: iface.remove_slave(slave_name) def verify(self, cur_iface_infos): @@ -307,7 +304,6 @@ 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/applier.py b/libnmstate/nm/applier.py index 26a057f..4e20af5 100644 --- a/libnmstate/nm/applier.py +++ b/libnmstate/nm/applier.py @@ -17,11 +17,9 @@ # along with this program. If not, see . # -from distutils.version import StrictVersion import logging import itertools -from libnmstate.error import NmstateNotSupportedError from libnmstate.error import NmstateValueError from libnmstate.schema import Interface from libnmstate.schema import InterfaceState @@ -50,7 +48,6 @@ 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 @@ -68,19 +65,7 @@ MASTER_IFACE_TYPES = ( def apply_changes(context, net_state, save_to_disk): con_profiles = [] - if ( - not save_to_disk - and _has_ovs_interface_desired_or_changed(net_state) - and StrictVersion(context.client.get_version()) - < StrictVersion("1.28.0") - ): - raise NmstateNotSupportedError( - f"NetworkManager version {context.client.get_version()} does not " - f"support 'save_to_disk=False' against OpenvSwitch interface" - ) - _preapply_dns_fix(context, net_state) - _mark_nm_external_subordinate_changed(context, net_state) ifaces_desired_state = net_state.ifaces.state_to_edit ifaces_desired_state.extend( @@ -105,15 +90,6 @@ def apply_changes(context, net_state, save_to_disk): cur_con_profile = connection.ConnectionProfile( context, profile=con_profile ) - - if save_to_disk: - # TODO: Need handle save_to_disk=False - connection.delete_iface_profiles_except( - context, - ifname, - cur_con_profile.profile if cur_con_profile else None, - ) - original_desired_iface_state = {} if net_state.ifaces.get(ifname): iface = net_state.ifaces[ifname] @@ -125,7 +101,6 @@ def apply_changes(context, net_state, save_to_disk): and cur_con_profile and cur_con_profile.profile and not net_state.ifaces[ifname].is_changed - and cur_con_profile.is_memory_only != save_to_disk ): # Don't create new profile if original desire does not ask # anything besides state:up and not been marked as changed. @@ -147,6 +122,7 @@ def apply_changes(context, net_state, save_to_disk): con_profiles.append(new_con_profile) else: # Missing connection, attempting to create a new one. + connection.delete_iface_inactive_connections(context, ifname) new_con_profile.add(save_to_disk) con_profiles.append(new_con_profile) context.wait_all_finish() @@ -275,14 +251,13 @@ def _set_ifaces_admin_state(context, ifaces_desired_state, con_profiles): == InterfaceState.ABSENT ) for affected_nmdev in nmdevs: - if not is_externally_managed(affected_nmdev): - devs_to_deactivate[ + 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: - devs_to_delete_profile[ - affected_nmdev.get_iface() - ] = affected_nmdev if ( is_absent and nmdev.is_software() @@ -627,31 +602,3 @@ def _preapply_dns_fix(context, net_state): for iface in net_state.ifaces.values(): if iface.is_changed or iface.is_desired: iface.remove_dns_metadata() - - -def _has_ovs_interface_desired_or_changed(net_state): - for iface in net_state.ifaces.values(): - if iface.type in ( - InterfaceType.OVS_BRIDGE, - InterfaceType.OVS_INTERFACE, - InterfaceType.OVS_PORT, - ) and (iface.is_desired or iface.is_changed): - return True - - -def _mark_nm_external_subordinate_changed(context, net_state): - """ - When certain main interface contains subordinates is marked as - connected(externally), it means its profile is memory only and will lost - on next deactivation. - For this case, we should mark the subordinate as changed. - that subordinate should be marked as changed for NM to take over. - """ - for iface in net_state.ifaces.values(): - 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 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/bond.py b/libnmstate/nm/bond.py index d196965..9ea3648 100644 --- a/libnmstate/nm/bond.py +++ b/libnmstate/nm/bond.py @@ -38,8 +38,6 @@ NM_SUPPORTED_BOND_OPTIONS = NM.SettingBond.get_valid_options( SYSFS_BOND_OPTION_FOLDER_FMT = "/sys/class/net/{ifname}/bonding" -BOND_AD_ACTOR_SYSTEM_USE_BOND_MAC = "00:00:00:00:00:00" - def create_setting(options, wired_setting): bond_setting = NM.SettingBond.new() @@ -50,13 +48,6 @@ def create_setting(options, wired_setting): ): # When in MAC restricted mode, MAC address should be unset. wired_setting.props.cloned_mac_address = None - if ( - option_name == "ad_actor_system" - and option_value == BOND_AD_ACTOR_SYSTEM_USE_BOND_MAC - ): - # The all zero ad_actor_system is the kernel default value - # And it is invalid to set as all zero - continue if option_value != SYSFS_EMPTY_VALUE: success = bond_setting.add_option(option_name, str(option_value)) if not success: diff --git a/libnmstate/nm/bridge.py b/libnmstate/nm/bridge.py index 0ca6c2d..b885f7a 100644 --- a/libnmstate/nm/bridge.py +++ b/libnmstate/nm/bridge.py @@ -260,9 +260,9 @@ def _get_slave_profiles_by_name(master_device): for dev in master_device.get_slaves(): active_con = connection.get_device_active_connection(dev) if active_con: - profile = active_con.props.connection - if profile: - slaves_profiles_by_name[dev.get_iface()] = profile + slaves_profiles_by_name[ + dev.get_iface() + ] = active_con.props.connection return slaves_profiles_by_name diff --git a/libnmstate/nm/connection.py b/libnmstate/nm/connection.py index 1f6c734..02890bc 100644 --- a/libnmstate/nm/connection.py +++ b/libnmstate/nm/connection.py @@ -163,16 +163,6 @@ class ConnectionProfile: self._con_profile = con_profile @property - def is_memory_only(self): - if self._con_profile: - profile_flags = self._con_profile.get_flags() - return ( - NM.SettingsConnectionFlags.UNSAVED & profile_flags - or NM.SettingsConnectionFlags.VOLATILE & profile_flags - ) - return False - - @property def devname(self): if self._con_profile: return self._con_profile.get_interface_name() @@ -506,14 +496,9 @@ def get_device_active_connection(nm_device): return active_conn -def delete_iface_profiles_except(context, ifname, excluded_profile): +def delete_iface_inactive_connections(context, ifname): for con in list_connections_by_ifname(context, ifname): - if ( - not excluded_profile - or not con.profile - or con.profile.get_uuid() != excluded_profile.get_uuid() - ): - con.delete() + con.delete() def list_connections_by_ifname(context, ifname): diff --git a/libnmstate/nm/device.py b/libnmstate/nm/device.py index fdf05bc..528f57d 100644 --- a/libnmstate/nm/device.py +++ b/libnmstate/nm/device.py @@ -23,7 +23,6 @@ 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): @@ -162,8 +161,3 @@ 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() diff --git a/libnmstate/nm/ipv6.py b/libnmstate/nm/ipv6.py index 9777c89..f252578 100644 --- a/libnmstate/nm/ipv6.py +++ b/libnmstate/nm/ipv6.py @@ -106,7 +106,6 @@ def create_setting(config, base_con_profile): setting_ip.props.never_default = False setting_ip.props.ignore_auto_dns = False setting_ip.clear_routes() - setting_ip.clear_routing_rules() setting_ip.props.gateway = None setting_ip.props.route_table = Route.USE_DEFAULT_ROUTE_TABLE setting_ip.props.route_metric = Route.USE_DEFAULT_METRIC diff --git a/libnmstate/nm/ovs.py b/libnmstate/nm/ovs.py index d1f26ba..2518773 100644 --- a/libnmstate/nm/ovs.py +++ b/libnmstate/nm/ovs.py @@ -140,12 +140,7 @@ def get_port_by_slave(nmdev): def get_ovs_info(context, bridge_device, devices_info): - ovs_ports_info = ( - info - for info in devices_info - if is_ovs_port_type_id(info[1]["type_id"]) - ) - port_profiles = _get_slave_profiles(bridge_device, ovs_ports_info) + port_profiles = _get_slave_profiles(bridge_device, devices_info) ports = _get_bridge_ports_info(context, port_profiles, devices_info) options = _get_bridge_options(context, bridge_device) @@ -208,21 +203,8 @@ def _get_bridge_port_info(context, port_profile, devices_info): vlan_mode = port_setting.props.vlan_mode port_name = port_profile.get_interface_name() - port_device = next( - dev - for dev, devinfo in devices_info - if devinfo["name"] == port_name - and is_ovs_port_type_id(devinfo["type_id"]) - ) - devices_info_excluding_bridges_and_ports = ( - info - for info in devices_info - if not is_ovs_bridge_type_id(info[1]["type_id"]) - and not is_ovs_port_type_id(info[1]["type_id"]) - ) - port_slave_profiles = _get_slave_profiles( - port_device, devices_info_excluding_bridges_and_ports - ) + port_device = context.get_nm_dev(port_name) + port_slave_profiles = _get_slave_profiles(port_device, devices_info) port_slave_names = [c.get_interface_name() for c in port_slave_profiles] if port_slave_names: @@ -297,7 +279,5 @@ def _get_slave_profiles(master_device, devices_info): if active_con: master = active_con.props.master if master and (master.get_iface() == master_device.get_iface()): - profile = active_con.props.connection - if profile: - slave_profiles.append(profile) + slave_profiles.append(active_con.props.connection) return slave_profiles diff --git a/libnmstate/nm/plugin.py b/libnmstate/nm/plugin.py index 06a5acd..4032359 100644 --- a/libnmstate/nm/plugin.py +++ b/libnmstate/nm/plugin.py @@ -97,6 +97,7 @@ class NetworkManagerPlugin(NmstatePlugin): def get_interfaces(self): info = [] + capabilities = self.capabilities devices_info = [ (dev, nm_device.get_device_common_info(dev)) @@ -122,15 +123,18 @@ class NetworkManagerPlugin(NmstatePlugin): if nm_bond.is_bond_type_id(type_id): bondinfo = nm_bond.get_bond_info(dev) iface_info.update(_ifaceinfo_bond(bondinfo)) - elif nm_ovs.is_ovs_bridge_type_id(type_id): - iface_info["bridge"] = nm_ovs.get_ovs_info( - self.context, dev, devices_info - ) - iface_info = _remove_ovs_bridge_unsupported_entries(iface_info) - elif nm_ovs.is_ovs_interface_type_id(type_id): - iface_info.update(nm_ovs.get_interface_info(act_con)) - elif nm_ovs.is_ovs_port_type_id(type_id): - continue + elif NmstatePlugin.OVS_CAPABILITY in capabilities: + if nm_ovs.is_ovs_bridge_type_id(type_id): + iface_info["bridge"] = nm_ovs.get_ovs_info( + self.context, dev, devices_info + ) + iface_info = _remove_ovs_bridge_unsupported_entries( + iface_info + ) + elif nm_ovs.is_ovs_interface_type_id(type_id): + iface_info.update(nm_ovs.get_interface_info(act_con)) + elif nm_ovs.is_ovs_port_type_id(type_id): + continue info.append(iface_info) diff --git a/libnmstate/nm/wired.py b/libnmstate/nm/wired.py index 64662ac..27d4318 100644 --- a/libnmstate/nm/wired.py +++ b/libnmstate/nm/wired.py @@ -124,9 +124,7 @@ def get_info(device): iface = device.get_iface() try: - mtu = int(device.get_mtu()) - if mtu: - info[Interface.MTU] = mtu + info[Interface.MTU] = int(device.get_mtu()) except AttributeError: pass diff --git a/libnmstate/plugins/nmstate_plugin_ovsdb.py b/libnmstate/plugins/nmstate_plugin_ovsdb.py index f667e8f..83965e1 100644 --- a/libnmstate/plugins/nmstate_plugin_ovsdb.py +++ b/libnmstate/plugins/nmstate_plugin_ovsdb.py @@ -161,14 +161,7 @@ class NmstateOvsdbPlugin(NmstatePlugin): return ifaces def apply_changes(self, net_state, save_to_disk): - # State might changed after other plugin invoked apply_changes() self.refresh_content() - cur_iface_to_ext_ids = {} - for iface_info in self.get_interfaces(): - cur_iface_to_ext_ids[iface_info[Interface.NAME]] = iface_info[ - OvsDB.OVS_DB_SUBTREE - ][OvsDB.EXTERNAL_IDS] - pending_changes = [] for iface in net_state.ifaces.values(): if not iface.is_changed and not iface.is_desired: @@ -181,34 +174,7 @@ class NmstateOvsdbPlugin(NmstatePlugin): table_name = "Interface" else: continue - ids_after_nm_applied = cur_iface_to_ext_ids.get(iface.name, {}) - ids_before_nm_applied = ( - iface.to_dict() - .get(OvsDB.OVS_DB_SUBTREE, {}) - .get(OvsDB.EXTERNAL_IDS, {}) - ) - original_desire_ids = iface.original_dict.get( - OvsDB.OVS_DB_SUBTREE, {} - ).get(OvsDB.EXTERNAL_IDS) - - desire_ids = [] - - if original_desire_ids is None: - desire_ids = ids_before_nm_applied - else: - desire_ids = original_desire_ids - - # should include external_id created by NetworkManager. - if NM_EXTERNAL_ID in ids_after_nm_applied: - desire_ids[NM_EXTERNAL_ID] = ids_after_nm_applied[ - NM_EXTERNAL_ID - ] - if desire_ids != ids_after_nm_applied: - pending_changes.append( - _generate_db_change_external_ids( - table_name, iface.name, desire_ids - ) - ) + pending_changes.extend(_generate_db_change(table_name, iface)) if pending_changes: if not save_to_disk: raise NmstateNotImplementedError( @@ -222,11 +188,19 @@ class NmstateOvsdbPlugin(NmstatePlugin): def _db_write(self, changes): changes_index = {change.row_name: change for change in changes} changed_tables = set(change.table_name for change in changes) + updated_names = [] for changed_table in changed_tables: for row in self._idl.tables[changed_table].rows.values(): if row.name in changes_index: change = changes_index[row.name] setattr(row, change.column_name, change.column_value) + updated_names.append(change.row_name) + new_rows = set(changes_index.keys()) - set(updated_names) + if new_rows: + raise NmstatePluginError( + f"BUG: row {new_rows} does not exists in OVS DB " + "and currently we don't create new row" + ) def _start_transaction(self): self._transaction = Transaction(self._idl) @@ -268,15 +242,38 @@ class NmstateOvsdbPlugin(NmstatePlugin): ) -def _generate_db_change_external_ids(table_name, iface_name, desire_ids): +def _generate_db_change(table_name, iface_state): + return _generate_db_change_external_ids(table_name, iface_state) + + +def _generate_db_change_external_ids(table_name, iface_state): + pending_changes = [] + desire_ids = iface_state.original_dict.get(OvsDB.OVS_DB_SUBTREE, {}).get( + OvsDB.EXTERNAL_IDS + ) if desire_ids and not isinstance(desire_ids, dict): raise NmstateValueError("Invalid external_ids, should be dictionary") - # Convert all value to string - for key, value in desire_ids.items(): - desire_ids[key] = str(value) + if desire_ids or desire_ids == {}: + # should include external_id required by NetworkManager. + merged_ids = ( + iface_state.to_dict() + .get(OvsDB.OVS_DB_SUBTREE, {}) + .get(OvsDB.EXTERNAL_IDS, {}) + ) + if NM_EXTERNAL_ID in merged_ids: + desire_ids[NM_EXTERNAL_ID] = merged_ids[NM_EXTERNAL_ID] - return _Changes(table_name, OvsDB.EXTERNAL_IDS, iface_name, desire_ids) + # Convert all value to string + for key, value in desire_ids.items(): + desire_ids[key] = str(value) + + pending_changes.append( + _Changes( + table_name, OvsDB.EXTERNAL_IDS, iface_state.name, desire_ids + ) + ) + return pending_changes NMSTATE_PLUGIN = NmstateOvsdbPlugin