[DISCUSSION] python version requirement
by Ondrej Lichtner
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 years, 5 months
[PATCH] IperfFlowMeasurement: fix parse_job_cpu duration get, again
by olichtne＠redhat.com
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
2 years, 8 months
[PATCH] BaseFlowMeasurement: fix copy paste error in time_slice method
by olichtne＠redhat.com
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
2 years, 8 months
[PATCH] IperfFlowMeasurement: fix dict key access
by olichtne＠redhat.com
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 years, 8 months
[PATCH v2 1/2] *Measurement: add proper timestamp init where missing
by olichtne＠redhat.com
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
2 years, 8 months
[PATCH 1/3] Ping.Evaluators: fix evaluate_results method parameters
by olichtne＠redhat.com
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
2 years, 8 months
[PATCH] TRexFlowMeasurement: add missing timestamp initialization to PerfInterval
by olichtne＠redhat.com
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
2 years, 8 months
[PATCH 1/2] Recipes.ENRT: move {idlestates,turboboost}_host_list properties
by olichtne＠redhat.com
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
2 years, 8 months
[PATCH] add sleep between start of iperf server and client
by Jan Tluka
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 years, 8 months
[PATCH] Tests.Iperf: use port in client command
by Jan Tluka
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 years, 8 months