LNST-developers January 2021

lnst-developers@lists.fedorahosted.org

4 participants
21 discussions

[DISCUSSION] python version requirement
by Ondrej Lichtner 06 Apr '21

06 Apr '21

Hi all, since we've moved to python3 that is actively developed and versions move between various long term/short term support cycles, we should also adapt LNST to this cycle of updating which minimal version of python LNST requires. TL;DR: The main questions I'm asking are: * how do we _implement_ a python version requirement? * how do we _upgrade_/_migrate_ in the future? * how do we _document_ a python version requirement? * which version do we want to use _now_? More context: I think at the moment we have a "soft" requirement for python3.6. Soft because we: * probably haven't tested on anything older * it's not explicitly configured/documented anywhere At the same time, there are now at least two reasons to start thinking about moving to python3.8: * I remember Perry asking about the f-string feature introduced in 3.8 * while working with Adrian on the TRex refactoring I started thinking about a feature for the lnst.Tests package that I've had in mind for a while, which requires a 3.7 feature * python3.8 is the current version on Fedora32, is available in RHEL8 (via dnf install python38), and python3.7 was skipped The lnst.Tests feature I'm thinking of is "lazy" and "dynamic" loading of BaseTestModule derived modules - for example at the moment, if a Recipe imports any module from lnst.Tests (e.g. lnst.Tests.Ping), the entire package is parsed and "loaded", which means that the python environment will also parse and load lnst.Tests.TRex. This means that a basic hello world recipe that simply calls Ping, will in some way require load time dependencies of TRex. The "lazy" and "dynamic" loading of test modules would ensure that when a recipe calls: from lnst.Tests import Ping Only the Ping module will be parsed, loaded and imported, and nothing else. And the dynamicity here could mean that we could be able to extend test modules exported by the lnst.Tests package via the lnst-ctl config file, for example for user/tester implemented test modules that are not tracked in the main lnst repository. I wrote a rough patch to experiment with this: --- diff --git a/lnst/Tests/__init__.py b/lnst/Tests/__init__.py index f7c6c90..a39b6f4 100644 --- a/lnst/Tests/__init__.py +++ b/lnst/Tests/__init__.py @@ -12,8 +12,26 @@ olichtne(a)redhat.com (Ondrej Lichtner) """ -from lnst.Tests.Ping import Ping -from lnst.Tests.PacketAssert import PacketAssert -from lnst.Tests.Iperf import IperfClient, IperfServer +# from lnst.Tests.Ping import Ping +# from lnst.Tests.PacketAssert import PacketAssert +# from lnst.Tests.Iperf import IperfClient, IperfServer +import importlib + +lazy_load_modules = { + "Ping": "lnst.Tests.Ping", + "PacketAssert": "lnst.Tests.PacketAssert", + "IperfClient": "lnst.Tests.Iperf", + "IperfServer": "lnst.Tests.Iperf", +} + + +def __getattr__(name): + if name not in lazy_load_modules: + raise ImportError("Cannot import {}".format(name)) + mod = importlib.import_module(lazy_load_modules[name]) + globals()[name] = getattr(mod, name) + return globals()[name] + + +# #TODO add support for test classes from lnst-ctl.conf -#TODO add support for test classes from lnst-ctl.conf --- However this requires the ability to define __getattr__ for a module, which is introduced as a python3.7 feature via PEP562 [0]. -Ondrej [0] https://www.python.org/dev/peps/pep-0562/

2 3

[PATCH] IperfFlowMeasurement: fix parse_job_cpu duration get, again
by olichtne＠redhat.com 01 Feb '21

01 Feb '21

From: Ondrej Lichtner <olichtne(a)redhat.com> One more fix for accessing the test duration when parsing test results for cpu utilization of the iperf process. The json dictionaries are inconsistent between tcp/udp stream on how they report the end of test data - udp reports only a "sum" dictionary, tcp only reports a "sum_sent" and "sum_received" dictionaries. So instead we look at the test start and the *requested* test duration. Not as precise as the actual duration of the test but probably good enough considering how the duration is used in this case... Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py index 87ef70f..722b49e 100644 --- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py @@ -187,5 +187,5 @@ def _parse_job_cpu(self, job): else: cpu_percent = job.result["data"]["end"]["cpu_utilization_percent"]["host_total"] job_start = job.result["data"]["start"]["timestamp"]["timesecs"] - duration = job.result["data"]["end"]["sum"]["seconds"] + duration = job.result["data"]["start"]["test_start"]["duration"] return PerfInterval(cpu_percent*duration, duration, "cpu_percent", job_start) -- 2.30.0

3 4

[PATCH] BaseFlowMeasurement: fix copy paste error in time_slice method
by olichtne＠redhat.com 29 Jan '21

29 Jan '21

From: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py index a8e9328..a798ca5 100644 --- a/lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/BaseFlowMeasurement.py @@ -192,7 +192,7 @@ def time_slice(self, start, end): result_copy.receiver_cpu_stats = self.receiver_cpu_stats.time_slice(start, end) result_copy.generator_results = self.generator_results.time_slice(start, end) - result_copy.receiver_results = self.generator_results.time_slice(start, end) + result_copy.receiver_results = self.receiver_results.time_slice(start, end) return result_copy -- 2.30.0

3 2

[PATCH] IperfFlowMeasurement: fix dict key access
by olichtne＠redhat.com 28 Jan '21

28 Jan '21

From: Ondrej Lichtner <olichtne(a)redhat.com> Aparently when testing udp_stream, the "sum_received" key isn't defined and data is reported slightly differently. Instead the total duration can also be accessed from the "sum" subdictionary. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py index e52f262..87ef70f 100644 --- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py @@ -187,5 +187,5 @@ def _parse_job_cpu(self, job): else: cpu_percent = job.result["data"]["end"]["cpu_utilization_percent"]["host_total"] job_start = job.result["data"]["start"]["timestamp"]["timesecs"] - duration = job.result["data"]["end"]["sum_received"]["seconds"] + duration = job.result["data"]["end"]["sum"]["seconds"] return PerfInterval(cpu_percent*duration, duration, "cpu_percent", job_start) -- 2.30.0

2 1

[PATCH v2 1/2] *Measurement: add proper timestamp init where missing
by olichtne＠redhat.com 28 Jan '21

28 Jan '21

From: Ondrej Lichtner <olichtne(a)redhat.com> We shouldn't use "None" for initialization of timestamps as it breaks any time alignment/time slice related code that works with these results. Use current timestamp of the controller (code that creates the PerfResult object) as "good enough" in cases where no timestamp is returned from the Test tool used. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- .../Perf/Measurements/IperfFlowMeasurement.py | 8 +++++--- .../Perf/Measurements/TRexFlowMeasurement.py | 9 +++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py index 10088f4..e52f262 100644 --- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py @@ -167,7 +167,7 @@ def _prepare_client(self, flow): def _parse_job_streams(self, job): result = ParallelPerfResult() if not job.passed: - result.append(PerfInterval(0, 0, "bits", None)) + result.append(PerfInterval(0, 0, "bits", time.time())) else: for i in job.result["data"]["end"]["streams"]: result.append(SequentialPerfResult()) @@ -183,7 +183,9 @@ def _parse_job_streams(self, job): def _parse_job_cpu(self, job): if not job.passed: - return PerfInterval(0, 0, "cpu_percent", None) + return PerfInterval(0, 0, "cpu_percent", time.time()) else: cpu_percent = job.result["data"]["end"]["cpu_utilization_percent"]["host_total"] - return PerfInterval(cpu_percent, 1, "cpu_percent", None) + job_start = job.result["data"]["start"]["timestamp"]["timesecs"] + duration = job.result["data"]["end"]["sum_received"]["seconds"] + return PerfInterval(cpu_percent*duration, duration, "cpu_percent", job_start) diff --git a/lnst/RecipeCommon/Perf/Measurements/TRexFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/TRexFlowMeasurement.py index b780842..dca58b1 100644 --- a/lnst/RecipeCommon/Perf/Measurements/TRexFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/TRexFlowMeasurement.py @@ -147,10 +147,11 @@ def _parse_results_by_port(self, job, port, flow): results.receiver_cpu_stats = SequentialPerfResult() if not job.passed: - results.generator_results.append(PerfInterval(0, 0, "packets", None)) - results.generator_cpu_stats.append(PerfInterval(0, 0, "cpu_percent", None)) - results.receiver_results.append(PerfInterval(0, 0, "packets", None)) - results.receiver_cpu_stats.append(PerfInterval(0, 0, "cpu_percent", None)) + timestamp = time.time() + results.generator_results.append(PerfInterval(0, 0, "packets", timestamp)) + results.generator_cpu_stats.append(PerfInterval(0, 0, "cpu_percent", timestamp)) + results.receiver_results.append(PerfInterval(0, 0, "packets", timestamp)) + results.receiver_cpu_stats.append(PerfInterval(0, 0, "cpu_percent", timestamp)) else: prev_time = job.result["start_time"] prev_tx_val = 0 -- 2.30.0

3 3

[PATCH 1/3] Ping.Evaluators: fix evaluate_results method parameters
by olichtne＠redhat.com 27 Jan '21

27 Jan '21

From: Ondrej Lichtner <olichtne(a)redhat.com> The evaluate_results method signature changed in cd5328db8b0196a6102ebea54bdea1ce632cfe46 and the ping evaluators weren't updated... Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py | 10 +++++++++- .../Ping/Evaluators/ZeroPassPingEvaluator.py | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py b/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py index 6760d32..f2a3dce 100644 --- a/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py +++ b/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py @@ -1,3 +1,6 @@ +from typing import List, Any + +from lnst.Controller.Recipe import BaseRecipe from lnst.RecipeCommon.BaseResultEvaluator import BaseResultEvaluator class RatePingEvaluator(BaseResultEvaluator): @@ -12,7 +15,12 @@ def __init__(self, min_rate=None, max_rate=None, rate=None): self.__class__.__name__) ) - def evaluate_results(self, recipe, result): + def evaluate_results( + self, + recipe: BaseRecipe, + recipe_conf: Any, + result: List[Any], + ): result_status = True ping_rate = int(result['rate']) diff --git a/lnst/RecipeCommon/Ping/Evaluators/ZeroPassPingEvaluator.py b/lnst/RecipeCommon/Ping/Evaluators/ZeroPassPingEvaluator.py index 350e157..afb3d02 100644 --- a/lnst/RecipeCommon/Ping/Evaluators/ZeroPassPingEvaluator.py +++ b/lnst/RecipeCommon/Ping/Evaluators/ZeroPassPingEvaluator.py @@ -1,7 +1,15 @@ +from typing import List, Any + +from lnst.Controller.Recipe import BaseRecipe from lnst.RecipeCommon.BaseResultEvaluator import BaseResultEvaluator class ZeroPassPingEvaluator(BaseResultEvaluator): - def evaluate_results(self, recipe, result): + def evaluate_results( + self, + recipe: BaseRecipe, + recipe_conf: Any, + result: List[Any], + ): result_status = True trans_packets = int(result['trans_pkts']) recv_packets = int(result['recv_pkts']) -- 2.30.0

3 10

[PATCH] TRexFlowMeasurement: add missing timestamp initialization to PerfInterval
by olichtne＠redhat.com 22 Jan '21

22 Jan '21

From: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- .../Perf/Measurements/TRexFlowMeasurement.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lnst/RecipeCommon/Perf/Measurements/TRexFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/TRexFlowMeasurement.py index d96ecc9..b780842 100644 --- a/lnst/RecipeCommon/Perf/Measurements/TRexFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/TRexFlowMeasurement.py @@ -147,10 +147,10 @@ def _parse_results_by_port(self, job, port, flow): results.receiver_cpu_stats = SequentialPerfResult() if not job.passed: - results.generator_results.append(PerfInterval(0, 0, "packets")) - results.generator_cpu_stats.append(PerfInterval(0, 0, "cpu_percent")) - results.receiver_results.append(PerfInterval(0, 0, "packets")) - results.receiver_cpu_stats.append(PerfInterval(0, 0, "cpu_percent")) + results.generator_results.append(PerfInterval(0, 0, "packets", None)) + results.generator_cpu_stats.append(PerfInterval(0, 0, "cpu_percent", None)) + results.receiver_results.append(PerfInterval(0, 0, "packets", None)) + results.receiver_cpu_stats.append(PerfInterval(0, 0, "cpu_percent", None)) else: prev_time = job.result["start_time"] prev_tx_val = 0 @@ -162,11 +162,11 @@ def _parse_results_by_port(self, job, port, flow): results.generator_results.append(PerfInterval( tx_delta, time_delta, - "pkts")) + "pkts", i["timestamp"])) results.receiver_results.append(PerfInterval( rx_delta, time_delta, - "pkts")) + "pkts", i["timestamp"])) prev_time = i["timestamp"] prev_tx_val = i["measurement"][port]["opackets"] @@ -176,9 +176,9 @@ def _parse_results_by_port(self, job, port, flow): results.generator_cpu_stats.append(PerfInterval( cpu_delta, time_delta, - "cpu_percent")) + "cpu_percent", i["timestamp"])) results.receiver_cpu_stats.append(PerfInterval( cpu_delta, time_delta, - "cpu_percent")) + "cpu_percent", i["timestamp"])) return results -- 2.30.0

3 2

[PATCH 1/2] Recipes.ENRT: move {idlestates,turboboost}_host_list properties
by olichtne＠redhat.com 22 Jan '21

22 Jan '21

From: Ondrej Lichtner <olichtne(a)redhat.com> Because the mixin classes for idlestates and turboboost disabling were moved to be inherited into the Baremetal and Virtual enrtrecipes base classes, the host list definition in BaseEnrtRecipe was being overriden by the higher placed base definition in the original mixin classes like this: SimpleNetworkRecipe -> Mixin -> BaseEnrtRecipe The base definition of the "host_list" just returns [], and doesn't call super so the definition from BaseEnrtRecipe was never called. This effectively disabled the mixins in all the enrt recipes that previously used them. Moving these to the new base classes of BaremetalEnrtRecipe and VirtualEnrtRecipe fixes the issue. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- lnst/Recipes/ENRT/BaremetalEnrtRecipe.py | 24 +++++++++++++++++- lnst/Recipes/ENRT/BaseEnrtRecipe.py | 32 ------------------------ lnst/Recipes/ENRT/VirtualEnrtRecipe.py | 24 ++++++++++++++++++ 3 files changed, 47 insertions(+), 33 deletions(-) diff --git a/lnst/Recipes/ENRT/BaremetalEnrtRecipe.py b/lnst/Recipes/ENRT/BaremetalEnrtRecipe.py index da19095..59e222d 100644 --- a/lnst/Recipes/ENRT/BaremetalEnrtRecipe.py +++ b/lnst/Recipes/ENRT/BaremetalEnrtRecipe.py @@ -23,4 +23,26 @@ class BaremetalEnrtRecipe( IperfMeasurementGenerator, BaseEnrtRecipe, ): - pass + @property + def disable_idlestates_host_list(self): + """ + The `disable_idlestates_host_list` property value is the list of all + matched baremetal hosts for the recipe. + + For detailed explanation of this property see + :any:`DisableIdleStatesMixin` and + :any:`DisableIdleStatesMixin.disable_idlestates_host_list`. + """ + return self.matched + + @property + def disable_turboboost_host_list(self): + """ + The `disable_turboboost_host_list` property value is the list of all + matched baremetal hosts for the recipe. + + For detailed explanation of this property see + :any:`DisableTurboboostMixin` and + :any:`DisableTurboboostMixin.disable_turboboost_host_list`. + """ + return self.matched diff --git a/lnst/Recipes/ENRT/BaseEnrtRecipe.py b/lnst/Recipes/ENRT/BaseEnrtRecipe.py index 9813ba6..c263cfd 100644 --- a/lnst/Recipes/ENRT/BaseEnrtRecipe.py +++ b/lnst/Recipes/ENRT/BaseEnrtRecipe.py @@ -449,8 +449,6 @@ def condition(): self.ctl.wait_for_condition(condition, timeout=5) - - def _create_reverse_ping(self, pconf): return PingConf( client = pconf.destination, @@ -461,33 +459,3 @@ def _create_reverse_ping(self, pconf): interval = pconf.ping_interval, size = pconf.ping_psize, ) - - @property - def disable_turboboost_host_list(self): - """ - The `disable_turboboost_host_list` property value is the list of all - matched hosts for the recipe. If a specific recipe needs to change - this it should override the :any:`baremetal_hosts` property. - - For detailed explanation of this property see - :any:`DisableTurboboostMixin` and - :any:`DisableTurboboostMixin.disable_turboboost_host_list`. - """ - return self.baremetal_hosts - - @property - def disable_idlestates_host_list(self): - """ - The `disable_idlestates_host_list` property value is the list of all - matched hosts for the recipe. If a specific recipe needs to change - this it should override the :any:`baremetal_hosts` property. - - For detailed explanation of this property see - :any:`DisableIdleStatesMixin` and - :any:`DisableIdleStatesMixin.disable_idlestates_host_list`. - """ - return self.baremetal_hosts - - @property - def baremetal_hosts(self): - return self.matched diff --git a/lnst/Recipes/ENRT/VirtualEnrtRecipe.py b/lnst/Recipes/ENRT/VirtualEnrtRecipe.py index 82d6e3a..d706815 100644 --- a/lnst/Recipes/ENRT/VirtualEnrtRecipe.py +++ b/lnst/Recipes/ENRT/VirtualEnrtRecipe.py @@ -26,3 +26,27 @@ class VirtualEnrtRecipe( @property def hypervisor_hosts(self): return set([self.matched.host1, self.matched.host2]) + + @property + def disable_idlestates_host_list(self): + """ + The `disable_idlestates_host_list` property value is the list of all + matched baremetal hosts for the recipe. + + For detailed explanation of this property see + :any:`DisableIdleStatesMixin` and + :any:`DisableIdleStatesMixin.disable_idlestates_host_list`. + """ + return self.hypervisor_hosts + + @property + def disable_turboboost_host_list(self): + """ + The `disable_turboboost_host_list` property value is the list of all + matched baremetal hosts for the recipe. + + For detailed explanation of this property see + :any:`DisableTurboboostMixin` and + :any:`DisableTurboboostMixin.disable_turboboost_host_list`. + """ + return self.hypervisor_hosts -- 2.30.0

3 3

[PATCH] add sleep between start of iperf server and client
by Jan Tluka 22 Jan '21

22 Jan '21

Add a short delay before starting iperf client connection to server. This is to reduce probability of the iperf server not being able to start quick enough to handle the client connection. Fixes https://github.com/LNST-project/lnst/issues/200 Signed-off-by: Jan Tluka <jtluka(a)redhat.com> Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py index ed1f5371..ccfe17e3 100644 --- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py @@ -62,6 +62,7 @@ class IperfFlowMeasurement(BaseFlowMeasurement): for flow in test_flows: flow.server_job.start(bg=True) + time.sleep(2) for flow in test_flows: flow.client_job.start(bg=True) -- 2.26.2

2 1

[PATCH] Tests.Iperf: use port in client command
by Jan Tluka 22 Jan '21

22 Jan '21

This updates the iperf client command to use the port specified similar to the server command. Signed-off-by: Jan Tluka <jtluka(a)redhat.com> --- lnst/Tests/Iperf.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lnst/Tests/Iperf.py b/lnst/Tests/Iperf.py index bedeb518..483a9116 100644 --- a/lnst/Tests/Iperf.py +++ b/lnst/Tests/Iperf.py @@ -141,11 +141,12 @@ class IperfClient(IperfBase): test = "" cmd = ("iperf3 -c {server} -b 0/1000 -J -t {duration}" - " {cpu} {test} {mss} {blksize} {parallel}" + " {cpu} {test} {mss} {blksize} {parallel} {port}" " {opts}".format( server=self.params.server, duration=self.params.duration, cpu=cpu, test=test, mss=mss, blksize=blksize, parallel=parallel, + port=port, opts=self.params.opts if "opts" in self.params else "")) return cmd -- 2.26.2

2 1

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

LNST-developers January 2021