From: Ondrej Lichtner olichtne@redhat.com
Hi,
the following patchset is a collection of various minor fixes and a semi large refactoring of the netlink related methods of the Device class hierarchy.
These were implemented as a preparation for the following patchset that refactors and redesigns the ENRT recipes hierarchy to both enable a more flexible recipe design and to enable an easy generation of test_wide configuration and sub configuration descriptions.
-Ondrej
Ondrej Lichtner (9): lnst.Tests.Ping: don't fail due to stderr lnst.Devices.Device: refactor nl_sync methods lnst.Controller.Job: add host info to __str__ lnst.Controller.RecipeResults: reduce the level of DeviceConfigResults lnst.Devices.Device: minor adaptive coalescing improvement lnst.Common.IpAddress: add flags attribute lnst.Common.IpAddress: add is_tentative attribute lnst.Devices.Device: add flags to parsed ip addresses lnst.Controller.RecipeControl: add timeout parameter to wait_for_condition
lnst/Common/IpAddress.py | 28 +++++--- lnst/Controller/Job.py | 2 + lnst/Controller/RecipeControl.py | 4 +- lnst/Controller/RecipeResults.py | 2 +- lnst/Devices/BondDevice.py | 52 +++++++-------- lnst/Devices/BridgeDevice.py | 6 +- lnst/Devices/Device.py | 109 +++++++++++++------------------ lnst/Devices/MacvlanDevice.py | 4 +- lnst/Devices/SoftDevice.py | 2 +- lnst/Devices/VethDevice.py | 2 +- lnst/Devices/VlanDevice.py | 4 +- lnst/Devices/VtiDevice.py | 10 +-- lnst/Devices/VxlanDevice.py | 10 +-- lnst/Tests/Ping.py | 1 - 14 files changed, 114 insertions(+), 122 deletions(-)
From: Ondrej Lichtner olichtne@redhat.com
Removing the immediate False return if there's any stderr output. Instead only fail based on the returncode. This is the same as what we've had to fix for the iperf module.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Tests/Ping.py | 1 - 1 file changed, 1 deletion(-)
diff --git a/lnst/Tests/Ping.py b/lnst/Tests/Ping.py index 390a8e4..17de8ab 100644 --- a/lnst/Tests/Ping.py +++ b/lnst/Tests/Ping.py @@ -59,7 +59,6 @@ def run(self): self._res_data["msg"] = "errors reported by ping" logging.error(self._res_data["msg"]) logging.error(self._res_data["stderr"]) - return False
if ping_process.returncode > 0: self._res_data["msg"] = "returncode = {}".format(ping_process.returncode)
From: Ondrej Lichtner olichtne@redhat.com
Created a new _ipr_wrapper method that wraps any kind of IPRoute() object use that we need in the Device classes.
Renamed the _nl_sync method to _nl_link_sync to indicate that this method handles IPRoute().link methods specifically. This differentiates it a little better from the related IPRoute().addr methods used for manipulating IPs of a device.
Centralized error handling into the new _ipr_wrapper method to improve consistency of the logs.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Devices/BondDevice.py | 52 +++++++++---------- lnst/Devices/BridgeDevice.py | 6 +-- lnst/Devices/Device.py | 95 ++++++++++++++--------------------- lnst/Devices/MacvlanDevice.py | 4 +- lnst/Devices/SoftDevice.py | 2 +- lnst/Devices/VethDevice.py | 2 +- lnst/Devices/VlanDevice.py | 4 +- lnst/Devices/VtiDevice.py | 10 ++-- lnst/Devices/VxlanDevice.py | 10 ++-- 9 files changed, 83 insertions(+), 102 deletions(-)
diff --git a/lnst/Devices/BondDevice.py b/lnst/Devices/BondDevice.py index 07fd124..ce01427 100644 --- a/lnst/Devices/BondDevice.py +++ b/lnst/Devices/BondDevice.py @@ -34,7 +34,7 @@ def active_slave(self, val): raise DeviceConfigError("Invalid value, must be Device.")
self._set_linkinfo_data_attr("IFLA_BOND_ACTIVE_SLAVE", val.ifindex) - self._nl_sync("set") + self._nl_link_sync("set")
@property def ad_actor_sys_prio(self): @@ -50,7 +50,7 @@ def ad_actor_sys_prio(self, val): raise DeviceConfigError("Invalid value, must be 1-65535.")
self._set_linkinfo_data_attr("IFLA_BOND_AD_ACTOR_SYS_PRIO", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def ad_actor_system(self): @@ -64,7 +64,7 @@ def ad_actor_system(self): def ad_actor_system(self, val): val = hwaddress(val) self._set_linkinfo_data_attr("IFLA_BOND_AD_ACTOR_SYSTEM", val) - self._nl_sync("set") + self._nl_link_sync("set")
@property def ad_select(self): @@ -84,7 +84,7 @@ def ad_select(self, val): raise DeviceConfigError("Invalid value, must be in {} or {}.". format(list(m.keys()), list(m.values())))
- self._nl_sync("set") + self._nl_link_sync("set")
@property def ad_user_port_key(self): @@ -96,7 +96,7 @@ def ad_user_port_key(self): @ad_user_port_key.setter def ad_user_port_key(self, val): self._set_linkinfo_data_attr("IFLA_BOND_AD_PORT_KEY", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def all_slaves_active(self): @@ -105,7 +105,7 @@ def all_slaves_active(self): @all_slaves_active.setter def all_slaves_active(self, val): self._set_linkinfo_data_attr("IFLA_BOND_ALL_SLAVES_ACTIVE", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def arp_interval(self): @@ -114,7 +114,7 @@ def arp_interval(self): @arp_interval.setter def arp_interval(self, val): self._set_linkinfo_data_attr("IFLA_BOND_ARP_INTERVAL", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def arp_ip_target(self): @@ -142,7 +142,7 @@ def arp_ip_target(self, val):
ip_str = ",".join(new) self._set_linkinfo_data_attr("IFLA_BOND_ARP_IP_TARGET", ip_str) - self._nl_sync("set") + self._nl_link_sync("set")
@property def arp_validate(self): @@ -161,7 +161,7 @@ def arp_validate(self, val): raise DeviceConfigError("Invalid value, must be in {} or {}.". format(list(m.keys()), list(m.values())))
- self._nl_sync("set") + self._nl_link_sync("set")
@property def arp_all_targets(self): @@ -179,7 +179,7 @@ def arp_all_targets(self, val): raise DeviceConfigError("Invalid value, must be in {} or {}.". format(list(m.keys()), list(m.values())))
- self._nl_sync("set") + self._nl_link_sync("set")
@property def downdelay(self): @@ -188,7 +188,7 @@ def downdelay(self): @downdelay.setter def downdelay(self, val): self._set_linkinfo_data_attr("IFLA_BOND_DOWNDELAY", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def fail_over_mac(self): @@ -206,7 +206,7 @@ def fail_over_mac(self, val): raise DeviceConfigError("Invalid value, must be in {} or {}.". format(list(m.keys()), list(m.values())))
- self._nl_sync("set") + self._nl_link_sync("set")
@property def lacp_rate(self): @@ -224,7 +224,7 @@ def lacp_rate(self, val): raise DeviceConfigError("Invalid value, must be in {} or {}.". format(list(m.keys()), list(m.values())))
- self._nl_sync("set") + self._nl_link_sync("set")
@property def miimon(self): @@ -233,7 +233,7 @@ def miimon(self): @miimon.setter def miimon(self, val): self._set_linkinfo_data_attr("IFLA_BOND_LACP_RATE", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def min_links(self): @@ -242,7 +242,7 @@ def min_links(self): @min_links.setter def min_links(self, val): self._set_linkinfo_data_attr("IFLA_BOND_MIN_LINKS", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def mode(self): @@ -261,7 +261,7 @@ def mode(self, val): raise DeviceConfigError("Invalid value, must be in {} or {}.". format(list(m.keys()), list(m.values())))
- self._nl_sync("set") + self._nl_link_sync("set")
@property def num_peer_notif(self): @@ -273,7 +273,7 @@ def num_peer_notif(self, val): raise DeviceConfigError("Invalid value, must be 0-255.")
self._set_linkinfo_data_attr("IFLA_BOND_NUM_PEER_NOTIF", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def packets_per_slave(self): @@ -285,7 +285,7 @@ def packets_per_slave(self, val): raise DeviceConfigError("Invalid value, must be 0-65535.")
self._set_linkinfo_data_attr("IFLA_BOND_PACKETS_PER_SLAVE", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def primary(self): @@ -301,7 +301,7 @@ def primary(self, val): raise DeviceConfigError("Invalid value, must be Device.")
self._set_linkinfo_data_attr("IFLA_BOND_PRIMARY", val.ifindex) - self._nl_sync("set") + self._nl_link_sync("set")
@property def primary_reselect(self): @@ -319,7 +319,7 @@ def primary_reselect(self, val): raise DeviceConfigError("Invalid value, must be in {} or {}.". format(list(m.keys()), list(m.values())))
- self._nl_sync("set") + self._nl_link_sync("set")
@property def tlb_dynamic_lb(self): @@ -328,7 +328,7 @@ def tlb_dynamic_lb(self): @tlb_dynamic_lb.setter def tlb_dynamic_lb(self, val): self._set_linkinfo_data_attr("IFLA_BOND_TLB_DYNAMIC_LB", bool(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def updelay(self): @@ -337,7 +337,7 @@ def updelay(self): @updelay.setter def updelay(self, val): self._set_linkinfo_data_attr("IFLA_BOND_UPDELAY", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def use_carrier(self): @@ -346,7 +346,7 @@ def use_carrier(self): @use_carrier.setter def use_carrier(self, val): self._set_linkinfo_data_attr("IFLA_BOND_USE_CARRIER", bool(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def xmit_hash_policy(self): @@ -360,7 +360,7 @@ def xmit_hash_policy(self, val): self._set_linkinfo_data_attr("IFLA_BOND_XMIT_HASH_POLICY", val) else: raise DeviceConfigError("Invalid value, must be in {}}.".format(m)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def resend_igmp(self): @@ -372,7 +372,7 @@ def resend_igmp(self, val): raise DeviceConfigError("Invalid value, must be 0-255.")
self._set_linkinfo_data_attr("IFLA_BOND_RESEND_IGMP", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def lp_interval(self): @@ -384,4 +384,4 @@ def lp_interval(self, val): raise DeviceConfigError("Invalid value, must be 1-0x7fffffff.")
self._set_linkinfo_data_attr("IFLA_BOND_LP_INTERVAL", int(val)) - self._nl_sync("set") + self._nl_link_sync("set") diff --git a/lnst/Devices/BridgeDevice.py b/lnst/Devices/BridgeDevice.py index 817235b..364ffac 100644 --- a/lnst/Devices/BridgeDevice.py +++ b/lnst/Devices/BridgeDevice.py @@ -24,7 +24,7 @@ def ageing_time(self): @ageing_time.setter def ageing_time(self, val): self._set_linkinfo_data_attr("IFLA_BR_AGEING_TIME", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def stp_state(self): @@ -33,7 +33,7 @@ def stp_state(self): @stp_state.setter def stp_state(self, val): self._set_linkinfo_data_attr("IFLA_BR_STP_STATE", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def vlan_filtering(self): @@ -42,4 +42,4 @@ def vlan_filtering(self): @vlan_filtering.setter def vlan_filtering(self, val): self._set_linkinfo_data_attr("IFLA_BR_VLAN_FILTERING", bool(val)) - self._nl_sync("set") + self._nl_link_sync("set") diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py index 39d1dd2..e10aa95 100644 --- a/lnst/Devices/Device.py +++ b/lnst/Devices/Device.py @@ -60,7 +60,7 @@ def __init__(self, if_manager):
self._ip_addrs = []
- self._nl_update = {} + self._nl_link_update = {} self._bulk_enabled = False
self._cleanup_data = None @@ -83,7 +83,7 @@ def _set_nested_nl_attr(self, msg, value, *args): raise DeviceError("Error constructing nested nl msg.")
def _update_attr(self, value, *args): - self._set_nested_nl_attr(self._nl_update, value, *args) + self._set_nested_nl_attr(self._nl_link_update, value, *args)
def _process_nested_nl_attrs(self, msg): ret = {} @@ -95,33 +95,42 @@ def _process_nested_nl_attrs(self, msg): ret[k] = v return ret
- def _nl_sync(self, op, ipr_attrs=None, bulk=False): + def _nl_link_sync(self, op, ipr_attrs=None, bulk=False): if self._bulk_enabled and not bulk: return
if ipr_attrs is None: - attrs = self._process_nested_nl_attrs(self._nl_update) + attrs = self._process_nested_nl_attrs(self._nl_link_update) else: attrs = self._process_nested_nl_attrs(ipr_attrs)
+ if op == "add": + self._ipr_wrapper("link", op, **attrs) + else: + self._ipr_wrapper("link", op, index=self.ifindex, **attrs) + + if ipr_attrs is None: + self._nl_link_update = {} + + def _ipr_wrapper(self, obj_name, op_name, *args, **kwargs): + pretty_attrs = pprint.pformat({"args": args, "kwargs": kwargs}) + logging.debug("Performing pyroute.IPRoute().{}({}, *args, **kwargs)".format(obj_name, op_name)) + logging.debug("{}".format(pretty_attrs)) + + ret_val = None with pyroute2.IPRoute() as ipr: try: - pretty_attrs = pprint.pformat(attrs) - logging.debug("Performing Netlink operation {}, data:".format(op)) - logging.debug("{}".format(pretty_attrs)) - - if op == "add": - ipr.link(op, **attrs) + obj = getattr(ipr, obj_name) + if op_name is not None: + ret_val = obj(op_name, *args, **kwargs) else: - ipr.link(op, index=self.ifindex, **attrs) + ret_val = obj(*args, **kwargs) self._if_manager.rescan_devices() except Exception as e: log_exc_traceback() - raise DeviceConfigError("Operation {} on link {} failed: {}" - .format(op, self.name, str(e))) - - if ipr_attrs is None: - self._nl_update = {} + raise DeviceConfigError("Object {} operation {} on link {} failed: {}" + .format(obj_name, op_name, self.name, str(e))) + return ret_val
def _enable(self): """Enables the Device object""" @@ -351,7 +360,7 @@ def name(self, new_name): new_name -- the new name of the interface. """ self._update_attr(new_name, "IFLA_IFNAME") - self._nl_sync("set") + self._nl_link_sync("set")
@property def hwaddr(self): @@ -371,7 +380,7 @@ def hwaddr(self, addr): """ addr = hwaddress(addr) self._update_attr(str(addr), "IFLA_ADDRESS") - self._nl_sync("set") + self._nl_link_sync("set")
@property def adaptive_rx_coalescing(self): @@ -431,7 +440,7 @@ def mtu(self, value): Args: value -- the new MTU.""" self._update_attr(int(value), "IFLA_MTU") - self._nl_sync("set") + self._nl_link_sync("set")
@property def master(self): @@ -461,7 +470,7 @@ def master(self, dev): raise DeviceError("Invalid dev argument.")
self._update_attr(master_idx, "IFLA_MASTER") - self._nl_sync("set") + self._nl_link_sync("set")
@property def driver(self): @@ -515,14 +524,8 @@ def _clear_ips(self): def _ip_add_one(self, addr): ip = ipaddress(addr) if ip not in self.ips: - with pyroute2.IPRoute() as ipr: - try: - ipr.addr("add", index=self.ifindex, address=str(ip), - mask=ip.prefixlen) - self._if_manager.handle_netlink_msgs() - except pyroute2.netlink.NetlinkError: - log_exc_traceback() - raise DeviceConfigValueError("Invalid IP address") + self._ipr_wrapper("addr", "add", index=self.ifindex, + address=str(ip), mask=ip.prefixlen)
def ip_add(self, addr): """add an ip address or a list of ip addresses @@ -542,14 +545,8 @@ def ip_add(self, addr): def _ip_del_one(self, addr): ip = ipaddress(addr) if ip in self.ips: - with pyroute2.IPRoute() as ipr: - try: - ipr.addr("del", index=self.ifindex, address=str(ip), - mask=ip.prefixlen) - self._if_manager.handle_netlink_msgs() - except pyroute2.netlink.NetlinkError: - log_exc_traceback() - raise DeviceConfigValueError("Invalid IP address") + self._ipr_wrapper("addr", "del", index=self.ifindex, + address=str(ip), mask=ip.prefixlen)
def ip_del(self, addr): """remove an ip address or a list of ip addresses @@ -567,13 +564,7 @@ def ip_del(self, addr):
def ip_flush(self, scope=0): """flush all ip addresses of the device""" - with pyroute2.IPRoute() as ipr: - try: - ipr.flush_addr(index=self.ifindex, scope=scope) - self._if_manager.handle_netlink_msgs() - except pyroute2.netlink.NetlinkError: - log_exc_traceback() - raise DeviceConfigError("IP address flush failed") + self._ipr_wrapper("flush_addr", None, index=self.ifindex, scope=scope)
def ips_filter(self, **selectors): result = [] @@ -592,23 +583,13 @@ def ips_filter(self, **selectors):
def up(self): """set device up""" - with pyroute2.IPRoute() as ipr: - try: - ipr.link("set", index=self.ifindex, state="up") - self._if_manager.handle_netlink_msgs() - except pyroute2.netlink.NetlinkError: - log_exc_traceback() - raise DeviceConfigError("Setting link up failed.") + self._nl_link_update["state"] = "up" + self._nl_link_sync("set")
def down(self): """set device down""" - with pyroute2.IPRoute() as ipr: - try: - ipr.link("set", index=self.ifindex, state="down") - self._if_manager.handle_netlink_msgs() - except pyroute2.netlink.NetlinkError: - log_exc_traceback() - raise DeviceConfigError("Setting link down failed.") + self._nl_link_update["state"] = "down" + self._nl_link_sync("set")
#TODO looks like python ethtool module doesn't support these so we'll keep #exec_cmd for now... diff --git a/lnst/Devices/MacvlanDevice.py b/lnst/Devices/MacvlanDevice.py index 8408efa..be163bb 100644 --- a/lnst/Devices/MacvlanDevice.py +++ b/lnst/Devices/MacvlanDevice.py @@ -33,7 +33,7 @@ def realdev(self, val): raise DeviceConfigError("Value must be a Device object.")
self._update_attr(val.ifindex, "IFLA_LINK") - self._nl_sync("set") + self._nl_link_sync("set")
@property def mode(self): @@ -42,4 +42,4 @@ def mode(self): @mode.setter def mode(self, val): self._set_linkinfo_data_attr("IFLA_MACVLAN_MODE", str(val)) - self._nl_sync("set") + self._nl_link_sync("set") diff --git a/lnst/Devices/SoftDevice.py b/lnst/Devices/SoftDevice.py index c5a1219..7a92c24 100644 --- a/lnst/Devices/SoftDevice.py +++ b/lnst/Devices/SoftDevice.py @@ -62,7 +62,7 @@ def _get_linkinfo_data_attr(self, attr_name): def _create(self): self._update_attr(self._link_type, "IFLA_LINKINFO", "IFLA_INFO_KIND") try: - self._nl_sync("add", bulk=True) + self._nl_link_sync("add", bulk=True) except Exception as e: log_exc_traceback() raise DeviceConfigError("Creating link {} failed: {}".format( diff --git a/lnst/Devices/VethDevice.py b/lnst/Devices/VethDevice.py index 7538202..af16f64 100644 --- a/lnst/Devices/VethDevice.py +++ b/lnst/Devices/VethDevice.py @@ -52,7 +52,7 @@ def peer_name(self, val): else: self._update_attr(str(val), "IFLA_LINKINFO", "IFLA_INFO_DATA", "VETH_INFO_PEER", "IFLA_IFNAME") - self._nl_sync("set") + self._nl_link_sync("set")
class PairedVethDevice(VethDevice): def __init__(self, ifmanager, *args, **kwargs): diff --git a/lnst/Devices/VlanDevice.py b/lnst/Devices/VlanDevice.py index b1224c9..a5652fb 100644 --- a/lnst/Devices/VlanDevice.py +++ b/lnst/Devices/VlanDevice.py @@ -36,7 +36,7 @@ def realdev(self, val): raise DeviceConfigError("realdev value must be a Device object.")
self._update_attr(val.ifindex, "IFLA_LINK") - self._nl_sync("set") + self._nl_link_sync("set")
@property def vlan_id(self): @@ -51,4 +51,4 @@ def vlan_id(self, val): raise DeviceConfigError("Invalid value, must be 1-4095.")
self._set_linkinfo_data_attr("IFLA_VLAN_ID", int(val)) - self._nl_sync("set") + self._nl_link_sync("set") diff --git a/lnst/Devices/VtiDevice.py b/lnst/Devices/VtiDevice.py index 2c0fb91..cf568e8 100644 --- a/lnst/Devices/VtiDevice.py +++ b/lnst/Devices/VtiDevice.py @@ -46,7 +46,7 @@ def ikey(self): @ikey.setter def ikey(self, val): self._set_linkinfo_data_attr("IFLA_VTI_IKEY", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def okey(self): @@ -58,7 +58,7 @@ def okey(self): @okey.setter def okey(self, val): self._set_linkinfo_data_attr("IFLA_VTI_OKEY", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def local(self): @@ -70,7 +70,7 @@ def local(self): @local.setter def local(self, val): self._set_linkinfo_data_attr("IFLA_VTI_LOCAL", str(ipaddress(val))) - self._nl_sync("set") + self._nl_link_sync("set")
@property def remote(self): @@ -82,7 +82,7 @@ def remote(self): @remote.setter def remote(self, val): self._set_linkinfo_data_attr("IFLA_VTI_REMOTE", str(ipaddress(val))) - self._nl_sync("set") + self._nl_link_sync("set")
@property def realdev(self): @@ -98,7 +98,7 @@ def realdev(self, val): raise DeviceConfigError("realdev value must be a Device object.")
self._set_linkinfo_data_attr("IFLA_VTI_LINK", val.ifindex) - self._nl_sync("set") + self._nl_link_sync("set")
@property def vti_type(self): diff --git a/lnst/Devices/VxlanDevice.py b/lnst/Devices/VxlanDevice.py index 780d1b2..c317913 100644 --- a/lnst/Devices/VxlanDevice.py +++ b/lnst/Devices/VxlanDevice.py @@ -53,7 +53,7 @@ def realdev(self, val): raise DeviceConfigError("realdev value must be a Device object.")
self._set_linkinfo_data_attr("IFLA_VXLAN_LINK", val.ifindex) - self._nl_sync("set") + self._nl_link_sync("set")
@property def vxlan_id(self): @@ -68,7 +68,7 @@ def vxlan_id(self, val): raise DeviceConfigError("Invalid value, must be 0-16777215.")
self._set_linkinfo_data_attr("IFLA_VXLAN_ID", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
@property def group(self): @@ -80,7 +80,7 @@ def group(self): @group.setter def group(self, val): self._set_linkinfo_data_attr("IFLA_VXLAN_GROUP", str(ipaddress(val))) - self._nl_sync("set") + self._nl_link_sync("set")
@property def remote(self): @@ -92,7 +92,7 @@ def remote(self): @remote.setter def remote(self, val): self._set_linkinfo_data_attr("IFLA_VXLAN_GROUP", str(ipaddress(val))) - self._nl_sync("set") + self._nl_link_sync("set")
@property def dst_port(self): @@ -101,4 +101,4 @@ def dst_port(self): @dst_port.setter def dst_port(self, val): self._set_linkinfo_data_attr("IFLA_VXLAN_PORT", int(val)) - self._nl_sync("set") + self._nl_link_sync("set")
From: Ondrej Lichtner olichtne@redhat.com
Since the method is often used when adding Recipe results (started or finished) that get automatically logged to the terminal, it's useful to also see what host the Job is associated with.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Controller/Job.py | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lnst/Controller/Job.py b/lnst/Controller/Job.py index 9c50210..3e5b3e6 100644 --- a/lnst/Controller/Job.py +++ b/lnst/Controller/Job.py @@ -218,6 +218,8 @@ def _to_dict(self): def __str__(self): attrs = ["type(%s)" % self.type]
+ attrs.append("host(%s)" % self.host.hostid) + if self._netns is not None: attrs.append("netns(%s)" % self._netns.name)
From: Ondrej Lichtner olichtne@redhat.com
IMPORTANT is the default result level that gets printed by the ResultSummaryFormatter. Seeing all the individual config actions can make it quite verbose and hard to see the actual tests. Reducing the level to NORMAL will clear it out from the summary by default, while retaining some importance and the config actions will still be visible in the full logs of the recipe execution.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Controller/RecipeResults.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lnst/Controller/RecipeResults.py b/lnst/Controller/RecipeResults.py index 45cadb1..dac609a 100644 --- a/lnst/Controller/RecipeResults.py +++ b/lnst/Controller/RecipeResults.py @@ -112,7 +112,7 @@ def __init__(self, success, device):
@property def level(self): - return ResultLevel.IMPORTANT + return ResultLevel.NORMAL
@property def device(self):
From: Ondrej Lichtner olichtne@redhat.com
* instead of handling exception raised by exec_cmd we can simply use the die_on_err=False parameter * don't attempt to cleanup configure coalescing if the cleanup_data for is (None, None)
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Devices/Device.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py index e10aa95..1ec7433 100644 --- a/lnst/Devices/Device.py +++ b/lnst/Devices/Device.py @@ -617,15 +617,11 @@ def autoneg_off(self): exec_cmd("ethtool -s %s autoneg off" % self.name)
def _read_adaptive_coalescing(self): - try: - res = exec_cmd("ethtool -c %s" % self.name) - except: - logging.debug("Could not read coalescence values of %s." % self.name) - return (None, None) + res, _ = exec_cmd("ethtool -c %s" % self.name, die_on_err=False)
regex = "Adaptive RX: (on|off) TX: (on|off)" try: - res = re.search(regex, res[0]).groups() + res = re.search(regex, res).groups() except AttributeError: raise DeviceError("No values for coalescence of %s." % self.name) @@ -644,7 +640,8 @@ def _write_adaptive_coalescing(self, rx_val, tx_val): def restore_coalescing(self): rx_val = self._cleanup_data["adaptive_rx_coalescing"] tx_val = self._cleanup_data["adaptive_tx_coalescing"] - self._write_adaptive_coalescing(rx_val, tx_val) + if (rx_val, tx_val) != (None, None): + self._write_adaptive_coalescing(rx_val, tx_val)
#TODO implement proper Route objects #consider the same as with tc?
From: Ondrej Lichtner olichtne@redhat.com
This optional attribute can store flags as provided by the kernel, this can be used for example for checking if an address is tentative as is the case for IPv6 addresses that haven't been DAD checked yet.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Common/IpAddress.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/lnst/Common/IpAddress.py b/lnst/Common/IpAddress.py index 54c3735..d8aabcc 100644 --- a/lnst/Common/IpAddress.py +++ b/lnst/Common/IpAddress.py @@ -19,9 +19,10 @@ #network
class BaseIpAddress(object): - def __init__(self, addr): + def __init__(self, addr, flags=None): self.addr, self.prefixlen = self._parse_addr(addr)
+ self.flags = flags self.family = None
def __str__(self): @@ -52,8 +53,8 @@ def __repr__(self): return "{}({}/{})".format(self.__class__.__name__, str(self), self.prefixlen)
class Ip4Address(BaseIpAddress): - def __init__(self, addr): - super(Ip4Address, self).__init__(addr) + def __init__(self, addr, flags=None): + super(Ip4Address, self).__init__(addr, flags)
self.family = AF_INET
@@ -82,8 +83,8 @@ def is_multicast(self): return aton("224.0.0.0") <= self.addr <= aton("239.255.255.255")
class Ip6Address(BaseIpAddress): - def __init__(self, addr): - super(Ip6Address, self).__init__(addr) + def __init__(self, addr, flags=None): + super(Ip6Address, self).__init__(addr, flags)
self.family = AF_INET6
@@ -114,15 +115,15 @@ def is_link_local(self): def is_multicast(self): return self.addr[:1] == b'\xff'
-def ipaddress(addr): +def ipaddress(addr, flags=None): """Factory method to create a BaseIpAddress object""" if isinstance(addr, BaseIpAddress): return addr elif isinstance(addr, str): try: - return Ip4Address(addr) + return Ip4Address(addr, flags) except: - return Ip6Address(addr) + return Ip6Address(addr, flags) else: raise LnstError("Value must be a BaseIpAddress or string object." " Not {}".format(type(addr)))
From: Ondrej Lichtner olichtne@redhat.com
By default all ip addresses are non-tentative, as this check only makes sense for ipv6 addresses at the moment.
The IPv6 address class checks the flag for by binary and with 0x40 which is the flag used by the Linux kernel.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Common/IpAddress.py | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/lnst/Common/IpAddress.py b/lnst/Common/IpAddress.py index d8aabcc..2640007 100644 --- a/lnst/Common/IpAddress.py +++ b/lnst/Common/IpAddress.py @@ -52,6 +52,11 @@ def _parse_addr(addr): def __repr__(self): return "{}({}/{})".format(self.__class__.__name__, str(self), self.prefixlen)
+ @property + def is_tentative(self): + #can only be True for IPv6 addresses + return False + class Ip4Address(BaseIpAddress): def __init__(self, addr, flags=None): super(Ip4Address, self).__init__(addr, flags) @@ -115,6 +120,12 @@ def is_link_local(self): def is_multicast(self): return self.addr[:1] == b'\xff'
+ @property + def is_tentative(self): + #constant from linux/if_addr.h + IFA_F_TENTATIVE = 0x40 + return IFA_F_TENTATIVE & self.flags + def ipaddress(addr, flags=None): """Factory method to create a BaseIpAddress object""" if isinstance(addr, BaseIpAddress):
From: Ondrej Lichtner olichtne@redhat.com
Set the flags attribute for ip address objects when created from an RTM_NEWADDR netlink message.
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Devices/Device.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py index 1ec7433..3984776 100644 --- a/lnst/Devices/Device.py +++ b/lnst/Devices/Device.py @@ -203,7 +203,8 @@ def _update_netlink(self, nl_msg): if nl_msg['header']['type'] == RTM_NEWLINK: self._nl_msg = nl_msg elif nl_msg['header']['type'] == RTM_NEWADDR: - addr = ipaddress(nl_msg.get_attr('IFA_ADDRESS')) + addr = ipaddress(nl_msg.get_attr('IFA_ADDRESS'), + flags=nl_msg.get_attr("IFA_FLAGS")) addr.prefixlen = nl_msg["prefixlen"]
if addr not in self._ip_addrs:
From: Ondrej Lichtner olichtne@redhat.com
Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Controller/RecipeControl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lnst/Controller/RecipeControl.py b/lnst/Controller/RecipeControl.py index b92b978..d43f7ce 100644 --- a/lnst/Controller/RecipeControl.py +++ b/lnst/Controller/RecipeControl.py @@ -27,12 +27,12 @@ def condition(): msg_dispatcher = self._controller._msg_dispatcher msg_dispatcher.wait_for_condition(condition)
- def wait_for_condition(self, condition): + def wait_for_condition(self, condition, timeout=0): #TODO add descriptions to conditions? logging.info("Suspending recipe execution until condition is true")
msg_dispatcher = self._controller._msg_dispatcher - msg_dispatcher.wait_for_condition(condition) + msg_dispatcher.wait_for_condition(condition, timeout)
def connect_host(self, hostname, timeout=60, port=None, machine_id=None, security=None):
Tue, Jun 11, 2019 at 11:22:12AM CEST, olichtne@redhat.com wrote:
From: Ondrej Lichtner olichtne@redhat.com
Hi,
the following patchset is a collection of various minor fixes and a semi large refactoring of the netlink related methods of the Device class hierarchy.
These were implemented as a preparation for the following patchset that refactors and redesigns the ENRT recipes hierarchy to both enable a more flexible recipe design and to enable an easy generation of test_wide configuration and sub configuration descriptions.
-Ondrej
Ondrej Lichtner (9): lnst.Tests.Ping: don't fail due to stderr lnst.Devices.Device: refactor nl_sync methods lnst.Controller.Job: add host info to __str__ lnst.Controller.RecipeResults: reduce the level of DeviceConfigResults lnst.Devices.Device: minor adaptive coalescing improvement lnst.Common.IpAddress: add flags attribute lnst.Common.IpAddress: add is_tentative attribute lnst.Devices.Device: add flags to parsed ip addresses lnst.Controller.RecipeControl: add timeout parameter to wait_for_condition
lnst/Common/IpAddress.py | 28 +++++--- lnst/Controller/Job.py | 2 + lnst/Controller/RecipeControl.py | 4 +- lnst/Controller/RecipeResults.py | 2 +- lnst/Devices/BondDevice.py | 52 +++++++-------- lnst/Devices/BridgeDevice.py | 6 +- lnst/Devices/Device.py | 109 +++++++++++++------------------ lnst/Devices/MacvlanDevice.py | 4 +- lnst/Devices/SoftDevice.py | 2 +- lnst/Devices/VethDevice.py | 2 +- lnst/Devices/VlanDevice.py | 4 +- lnst/Devices/VtiDevice.py | 10 +-- lnst/Devices/VxlanDevice.py | 10 +-- lnst/Tests/Ping.py | 1 - 14 files changed, 114 insertions(+), 122 deletions(-)
-- 2.22.0 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedorahos...
This one looks good.
Acked-by: Jan Tluka jtluka@redhat.com
On Tue, Jun 11, 2019 at 12:41:23PM +0200, Jan Tluka wrote:
Tue, Jun 11, 2019 at 11:22:12AM CEST, olichtne@redhat.com wrote:
From: Ondrej Lichtner olichtne@redhat.com
Hi,
the following patchset is a collection of various minor fixes and a semi large refactoring of the netlink related methods of the Device class hierarchy.
These were implemented as a preparation for the following patchset that refactors and redesigns the ENRT recipes hierarchy to both enable a more flexible recipe design and to enable an easy generation of test_wide configuration and sub configuration descriptions.
-Ondrej
Ondrej Lichtner (9): lnst.Tests.Ping: don't fail due to stderr lnst.Devices.Device: refactor nl_sync methods lnst.Controller.Job: add host info to __str__ lnst.Controller.RecipeResults: reduce the level of DeviceConfigResults lnst.Devices.Device: minor adaptive coalescing improvement lnst.Common.IpAddress: add flags attribute lnst.Common.IpAddress: add is_tentative attribute lnst.Devices.Device: add flags to parsed ip addresses lnst.Controller.RecipeControl: add timeout parameter to wait_for_condition
lnst/Common/IpAddress.py | 28 +++++--- lnst/Controller/Job.py | 2 + lnst/Controller/RecipeControl.py | 4 +- lnst/Controller/RecipeResults.py | 2 +- lnst/Devices/BondDevice.py | 52 +++++++-------- lnst/Devices/BridgeDevice.py | 6 +- lnst/Devices/Device.py | 109 +++++++++++++------------------ lnst/Devices/MacvlanDevice.py | 4 +- lnst/Devices/SoftDevice.py | 2 +- lnst/Devices/VethDevice.py | 2 +- lnst/Devices/VlanDevice.py | 4 +- lnst/Devices/VtiDevice.py | 10 +-- lnst/Devices/VxlanDevice.py | 10 +-- lnst/Tests/Ping.py | 1 - 14 files changed, 114 insertions(+), 122 deletions(-)
-- 2.22.0 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedorahos...
This one looks good.
Acked-by: Jan Tluka jtluka@redhat.com
thanks for the review, pushed.
-Ondrej
lnst-developers@lists.fedorahosted.org
-
Jan Tluka -
olichtne＠redhat.com -
Ondrej Lichtner