[OE-Core][PATCH 0/4] testimage: add failed test post actions and fetch more data

[OE-Core][PATCH 0/4] testimage: add failed test post actions and fetch more data

[Alexis Lothoré]

Hello,
this small series is related to some disk space issue observed by the SWAT
team (see [1]) which eventually leads to tests failure. In order to help
diagnose those issues, I propose to enrich the retrieved artifacts with
some additional data, like disk usage on target and on host (in case the
target is a virtualized guest)

To do so, this series first reworks a bit actions executed on failed tests
to allow declaring a sequence of actions. It then introduces two small
actions which basically run "df" on target and host. I chose to store
those stats in dedicated files, thinking it is better than "contaminating"
tests steps logs with those very specific data.

[1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=15220

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>

Alexis Lothoré (4):
  testimage: create a list of failed test post actions
  testimage: isolate artifacts directory creation in dedicated post
    action
  testimage: add target disk usage stat as post action
  testimage: add host disk usage stat as post action

 .../failed-tests-post-actions.bbclass         | 94 +++++++++++++++++++
 meta/classes-recipe/testimage.bbclass         | 42 +--------
 2 files changed, 96 insertions(+), 40 deletions(-)
 create mode 100644 meta/classes-recipe/failed-tests-post-actions.bbclass

-- 
2.43.1

[OE-Core][PATCH 1/4] testimage: create a list of failed test post actions

[Alexis Lothoré]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

testimage is able to detect whenever a test run leads to some tests
failing, and execute some actions in this case. The only action currently
defined in such case is to retrieve artifacts from the target under test,
as listed in TESTIMAGE_FAILED_QA_ARTIFACTS

In order to be able to add multiple actions, define a central function to
gather all "post actions" to run whenever a test has failed
(run_failed_tests_post_actions). This function contains a table listing all
functions to be called whenever a test fails. Any function in this table
will be provided with bitbake internal data dictionary ("d") and the
current runtime testing context ("tc"). Isolate all this feature in a
dedicated bbclass file inherited by testimage.
This patch does not bring any functional change.

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 .../failed-tests-post-actions.bbclass         | 64 +++++++++++++++++++
 meta/classes-recipe/testimage.bbclass         | 42 +-----------
 2 files changed, 66 insertions(+), 40 deletions(-)
 create mode 100644 meta/classes-recipe/failed-tests-post-actions.bbclass

diff --git a/meta/classes-recipe/failed-tests-post-actions.bbclass b/meta/classes-recipe/failed-tests-post-actions.bbclass
new file mode 100644
index 000000000000..7c7d3391298f
--- /dev/null
+++ b/meta/classes-recipe/failed-tests-post-actions.bbclass
@@ -0,0 +1,64 @@
+#
+# Copyright OpenEmbedded Contributors
+#
+# SPDX-License-Identifier: MIT
+#
+
+
+##################################################################
+# Artifacts retrieval
+##################################################################
+
+def get_artifacts_list(target, raw_list):
+    result = []
+    # Passed list may contains patterns in paths, expand them directly on target
+    for raw_path in raw_list.split():
+        cmd = f"for p in {raw_path}; do if [ -e $p ]; then echo $p; fi; done"
+        try:
+            status, output = target.run(cmd)
+            if status != 0 or not output:
+                raise Exception()
+            result += output.split()
+        except:
+            bb.note(f"No file/directory matching path {raw_path}")
+
+    return result
+
+def retrieve_test_artifacts(target, artifacts_list, target_dir):
+    import shutil
+
+    local_artifacts_dir = os.path.join(target_dir, "artifacts")
+    if os.path.isdir(local_artifacts_dir):
+        shutil.rmtree(local_artifacts_dir)
+
+    os.makedirs(local_artifacts_dir)
+    for artifact_path in artifacts_list:
+        if not os.path.isabs(artifact_path):
+            bb.warn(f"{artifact_path} is not an absolute path")
+            continue
+        try:
+            dest_dir = os.path.join(local_artifacts_dir, os.path.dirname(artifact_path[1:]))
+            os.makedirs(dest_dir, exist_ok=True)
+            target.copyFrom(artifact_path, dest_dir)
+        except Exception as e:
+            bb.warn(f"Can not retrieve {artifact_path} from test target: {e}")
+
+def list_and_fetch_failed_tests_artifacts(d, tc):
+    artifacts_list = get_artifacts_list(tc.target, d.getVar("TESTIMAGE_FAILED_QA_ARTIFACTS"))
+    if not artifacts_list:
+        bb.warn("Could not load artifacts list, skip artifacts retrieval")
+    else:
+        retrieve_test_artifacts(tc.target, artifacts_list, get_testimage_json_result_dir(d))
+
+
+##################################################################
+# General post actions runner
+##################################################################
+
+def run_failed_tests_post_actions(d, tc):
+    post_actions=[
+        list_and_fetch_failed_tests_artifacts
+    ]
+
+    for action in post_actions:
+        action(d, tc)
diff --git a/meta/classes-recipe/testimage.bbclass b/meta/classes-recipe/testimage.bbclass
index bee19674ef4f..d2b525d40f41 100644
--- a/meta/classes-recipe/testimage.bbclass
+++ b/meta/classes-recipe/testimage.bbclass
@@ -4,6 +4,7 @@
 
 inherit metadata_scm
 inherit image-artifact-names
+inherit failed-tests-post-actions
 
 # testimage.bbclass enables testing of qemu images using python unittests.
 # Most of the tests are commands run on target image over ssh.
@@ -177,40 +178,6 @@ def get_testimage_boot_patterns(d):
                 boot_patterns[flag] = flagval.encode().decode('unicode-escape')
     return boot_patterns
 
-def get_artifacts_list(target, raw_list):
-    result = []
-    # Passed list may contains patterns in paths, expand them directly on target
-    for raw_path in raw_list.split():
-        cmd = f"for p in {raw_path}; do if [ -e $p ]; then echo $p; fi; done"
-        try:
-            status, output = target.run(cmd)
-            if status != 0 or not output:
-                raise Exception()
-            result += output.split()
-        except:
-            bb.note(f"No file/directory matching path {raw_path}")
-
-    return result
-
-def retrieve_test_artifacts(target, artifacts_list, target_dir):
-    import shutil
-
-    local_artifacts_dir = os.path.join(target_dir, "artifacts")
-    if os.path.isdir(local_artifacts_dir):
-        shutil.rmtree(local_artifacts_dir)
-
-    os.makedirs(local_artifacts_dir)
-    for artifact_path in artifacts_list:
-        if not os.path.isabs(artifact_path):
-            bb.warn(f"{artifact_path} is not an absolute path")
-            continue
-        try:
-            dest_dir = os.path.join(local_artifacts_dir, os.path.dirname(artifact_path[1:]))
-            os.makedirs(dest_dir, exist_ok=True)
-            target.copyFrom(artifact_path, dest_dir)
-        except Exception as e:
-            bb.warn(f"Can not retrieve {artifact_path} from test target: {e}")
-
 def testimage_main(d):
     import os
     import json
@@ -406,12 +373,7 @@ def testimage_main(d):
         results = tc.runTests()
         complete = True
         if results.hasAnyFailingTest():
-            artifacts_list = get_artifacts_list(tc.target, d.getVar("TESTIMAGE_FAILED_QA_ARTIFACTS"))
-            if not artifacts_list:
-                bb.warn("Could not load artifacts list, skip artifacts retrieval")
-            else:
-                bb.warn(f"Retrieving artifacts: {d.getVar('TESTIMAGE_FAILED_QA_ARTIFACTS')}")
-                retrieve_test_artifacts(tc.target, artifacts_list, get_testimage_json_result_dir(d))
+            run_failed_tests_post_actions(d, tc)
     except (KeyboardInterrupt, BlockingIOError) as err:
         if isinstance(err, KeyboardInterrupt):
             bb.error('testimage interrupted, shutting down...')
-- 
2.43.1

[OE-Core][PATCH 2/4] testimage: isolate artifacts directory creation in dedicated post action

[Alexis Lothoré]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

In order to be able to create actions that could store new files during
failed test post actions, we need to split artifacts directory creation
from artifacts retrieval.

Create a new dedicated action to create artifacts main directory so we can
add actions creating files in this new directory, without worrying about
actions order if at least this action is set first.

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 .../failed-tests-post-actions.bbclass            | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/meta/classes-recipe/failed-tests-post-actions.bbclass b/meta/classes-recipe/failed-tests-post-actions.bbclass
index 7c7d3391298f..eaf08fb792f3 100644
--- a/meta/classes-recipe/failed-tests-post-actions.bbclass
+++ b/meta/classes-recipe/failed-tests-post-actions.bbclass
@@ -9,6 +9,15 @@
 # Artifacts retrieval
 ##################################################################
 
+def create_artifacts_directory(d, tc):
+    import shutil
+
+    local_artifacts_dir = os.path.join(get_testimage_json_result_dir(d), "artifacts")
+    if os.path.isdir(local_artifacts_dir):
+        shutil.rmtree(local_artifacts_dir)
+
+    os.makedirs(local_artifacts_dir)
+
 def get_artifacts_list(target, raw_list):
     result = []
     # Passed list may contains patterns in paths, expand them directly on target
@@ -25,13 +34,7 @@ def get_artifacts_list(target, raw_list):
     return result
 
 def retrieve_test_artifacts(target, artifacts_list, target_dir):
-    import shutil
-
     local_artifacts_dir = os.path.join(target_dir, "artifacts")
-    if os.path.isdir(local_artifacts_dir):
-        shutil.rmtree(local_artifacts_dir)
-
-    os.makedirs(local_artifacts_dir)
     for artifact_path in artifacts_list:
         if not os.path.isabs(artifact_path):
             bb.warn(f"{artifact_path} is not an absolute path")
@@ -57,6 +60,7 @@ def list_and_fetch_failed_tests_artifacts(d, tc):
 
 def run_failed_tests_post_actions(d, tc):
     post_actions=[
+        create_artifacts_directory,
         list_and_fetch_failed_tests_artifacts
     ]
 
-- 
2.43.1

[OE-Core][PATCH 3/4] testimage: add target disk usage stat as post action

[Alexis Lothoré]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

In order to debug issues related to disk space (see [1]),  add a failed
tests post action to retrieve disk usage on the target. Rely on the test
context object to run the corresponding command onto the target

[1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=15220

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 .../failed-tests-post-actions.bbclass           | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/meta/classes-recipe/failed-tests-post-actions.bbclass b/meta/classes-recipe/failed-tests-post-actions.bbclass
index eaf08fb792f3..a8604fca2b9d 100644
--- a/meta/classes-recipe/failed-tests-post-actions.bbclass
+++ b/meta/classes-recipe/failed-tests-post-actions.bbclass
@@ -5,6 +5,20 @@
 #
 
 
+##################################################################
+# Host/target statistics
+##################################################################
+
+def get_target_disk_usage(d, tc):
+    output_file = os.path.join(get_testimage_json_result_dir(d), "artifacts", "target_disk_usage.txt")
+    try:
+        (status, output) = tc.target.run('df -hl')
+        with open(output_file, 'w') as f:
+            f.write(output)
+            f.write("\n")
+    except Exception as e:
+        bb.warn(f"Can not get target disk usage: {e}")
+
 ##################################################################
 # Artifacts retrieval
 ##################################################################
@@ -61,7 +75,8 @@ def list_and_fetch_failed_tests_artifacts(d, tc):
 def run_failed_tests_post_actions(d, tc):
     post_actions=[
         create_artifacts_directory,
-        list_and_fetch_failed_tests_artifacts
+        list_and_fetch_failed_tests_artifacts,
+        get_target_disk_usage
     ]
 
     for action in post_actions:
-- 
2.43.1

[OE-Core][PATCH 4/4] testimage: add host disk usage stat as post action

[Alexis Lothoré]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

Since the target under test can be a virtualized guest, when some tests
fail because of disk usage (see [1]), also fetch disk usage statistics from
host to allow checking whether a host disk space saturation could affect
running tests.

[1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=15220

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 .../failed-tests-post-actions.bbclass               | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/meta/classes-recipe/failed-tests-post-actions.bbclass b/meta/classes-recipe/failed-tests-post-actions.bbclass
index a8604fca2b9d..a9e92aa2427e 100644
--- a/meta/classes-recipe/failed-tests-post-actions.bbclass
+++ b/meta/classes-recipe/failed-tests-post-actions.bbclass
@@ -19,6 +19,16 @@ def get_target_disk_usage(d, tc):
     except Exception as e:
         bb.warn(f"Can not get target disk usage: {e}")
 
+def get_host_disk_usage(d, tc):
+    import subprocess
+
+    output_file = os.path.join(get_testimage_json_result_dir(d), "artifacts", "host_disk_usage.txt")
+    try:
+        with open(output_file, 'w') as f:
+            output = subprocess.run(['/usr/bin/df', '-hl'], check=True, text=True, stdout=f)
+    except Exception as e:
+        bb.warn(f"Can not get host disk usage: {e}")
+
 ##################################################################
 # Artifacts retrieval
 ##################################################################
@@ -76,7 +86,8 @@ def run_failed_tests_post_actions(d, tc):
     post_actions=[
         create_artifacts_directory,
         list_and_fetch_failed_tests_artifacts,
-        get_target_disk_usage
+        get_target_disk_usage,
+        get_host_disk_usage
     ]
 
     for action in post_actions:
-- 
2.43.1

Re: [OE-Core][PATCH 1/4] testimage: create a list of failed test post actions

[Richard Purdie]

On Tue, 2024-02-20 at 21:01 +0100, Alexis Lothoré via
lists.openembedded.org wrote:
> From: Alexis Lothoré <alexis.lothore@bootlin.com>
> 
> testimage is able to detect whenever a test run leads to some tests
> failing, and execute some actions in this case. The only action
> currently
> defined in such case is to retrieve artifacts from the target under
> test,
> as listed in TESTIMAGE_FAILED_QA_ARTIFACTS
> 
> In order to be able to add multiple actions, define a central
> function to
> gather all "post actions" to run whenever a test has failed
> (run_failed_tests_post_actions). This function contains a table
> listing all
> functions to be called whenever a test fails. Any function in this
> table
> will be provided with bitbake internal data dictionary ("d") and the
> current runtime testing context ("tc"). Isolate all this feature in a
> dedicated bbclass file inherited by testimage.
> This patch does not bring any functional change.
> 
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> ---
>  .../failed-tests-post-actions.bbclass         | 64
> +++++++++++++++++++
>  meta/classes-recipe/testimage.bbclass         | 42 +-----------
>  2 files changed, 66 insertions(+), 40 deletions(-)
>  create mode 100644 meta/classes-recipe/failed-tests-post-
> actions.bbclass
> 
> diff --git a/meta/classes-recipe/failed-tests-post-actions.bbclass
> b/meta/classes-recipe/failed-tests-post-actions.bbclass
> new file mode 100644
> index 000000000000..7c7d3391298f
> --- /dev/null
> +++ b/meta/classes-recipe/failed-tests-post-actions.bbclass
> @@ -0,0 +1,64 @@
> +#
> +# Copyright OpenEmbedded Contributors
> +#
> +# SPDX-License-Identifier: MIT
> +#
> +
> +
> +##################################################################
> +# Artifacts retrieval
> +##################################################################
> +
> +def get_artifacts_list(target, raw_list):
> +    result = []
> +    # Passed list may contains patterns in paths, expand them
> directly on target
> +    for raw_path in raw_list.split():
> +        cmd = f"for p in {raw_path}; do if [ -e $p ]; then echo $p;
> fi; done"
> +        try:
> +            status, output = target.run(cmd)
> +            if status != 0 or not output:
> +                raise Exception()
> +            result += output.split()
> +        except:
> +            bb.note(f"No file/directory matching path {raw_path}")
> +
> +    return result
> +
> +def retrieve_test_artifacts(target, artifacts_list, target_dir):
> +    import shutil
> +
> +    local_artifacts_dir = os.path.join(target_dir, "artifacts")
> +    if os.path.isdir(local_artifacts_dir):
> +        shutil.rmtree(local_artifacts_dir)
> +
> +    os.makedirs(local_artifacts_dir)
> +    for artifact_path in artifacts_list:
> +        if not os.path.isabs(artifact_path):
> +            bb.warn(f"{artifact_path} is not an absolute path")
> +            continue
> +        try:
> +            dest_dir = os.path.join(local_artifacts_dir,
> os.path.dirname(artifact_path[1:]))
> +            os.makedirs(dest_dir, exist_ok=True)
> +            target.copyFrom(artifact_path, dest_dir)
> +        except Exception as e:
> +            bb.warn(f"Can not retrieve {artifact_path} from test
> target: {e}")
> +
> +def list_and_fetch_failed_tests_artifacts(d, tc):
> +    artifacts_list = get_artifacts_list(tc.target,
> d.getVar("TESTIMAGE_FAILED_QA_ARTIFACTS"))
> +    if not artifacts_list:
> +        bb.warn("Could not load artifacts list, skip artifacts
> retrieval")
> +    else:
> +        retrieve_test_artifacts(tc.target, artifacts_list,
> get_testimage_json_result_dir(d))
> +
> +
> +##################################################################
> +# General post actions runner
> +##################################################################
> +
> +def run_failed_tests_post_actions(d, tc):
> +    post_actions=[
> +        list_and_fetch_failed_tests_artifacts
> +    ]
> +
> +    for action in post_actions:
> +        action(d, tc)

Rather than create a bbclass class of python functions, these should
move to lib/oe and become a proper python library file?

Moving functions out of the class files is on my long term todo list so
this seems like an idea opportunity.

Cheers,

Richard

Re: [OE-Core][PATCH 1/4] testimage: create a list of failed test post actions

[Alexis Lothoré]

Hello Richard,

On 2/21/24 08:34, Richard Purdie wrote:
> On Tue, 2024-02-20 at 21:01 +0100, Alexis Lothoré via
> lists.openembedded.org wrote:
>> From: Alexis Lothoré <alexis.lothore@bootlin.com>

[...]

>> +##################################################################
>> +# General post actions runner
>> +##################################################################
>> +
>> +def run_failed_tests_post_actions(d, tc):
>> +    post_actions=[
>> +        list_and_fetch_failed_tests_artifacts
>> +    ]
>> +
>> +    for action in post_actions:
>> +        action(d, tc)
> 
> Rather than create a bbclass class of python functions, these should
> move to lib/oe and become a proper python library file?

ACK, I will do that and create a proper python file for this.

> Moving functions out of the class files is on my long term todo list so
> this seems like an idea opportunity.

So should this series take the opportunity to move all already existing python
functions from testimage to a lib ? I can certainly do that if that's your point :)

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-Core][PATCH 1/4] testimage: create a list of failed test post actions

[Richard Purdie]

On Wed, 2024-02-21 at 08:53 +0100, Alexis Lothoré wrote:
> On 2/21/24 08:34, Richard Purdie wrote:
> > On Tue, 2024-02-20 at 21:01 +0100, Alexis Lothoré via
> > lists.openembedded.org wrote:
> > > From: Alexis Lothoré <alexis.lothore@bootlin.com>
> 
> [...]
> 
> > > +################################################################
> > > ##
> > > +# General post actions runner
> > > +################################################################
> > > ##
> > > +
> > > +def run_failed_tests_post_actions(d, tc):
> > > +    post_actions=[
> > > +        list_and_fetch_failed_tests_artifacts
> > > +    ]
> > > +
> > > +    for action in post_actions:
> > > +        action(d, tc)
> > 
> > Rather than create a bbclass class of python functions, these
> > should
> > move to lib/oe and become a proper python library file?
> 
> ACK, I will do that and create a proper python file for this.
> 
> > Moving functions out of the class files is on my long term todo
> > list so
> > this seems like an idea opportunity.
> 
> So should this series take the opportunity to move all already
> existing python
> functions from testimage to a lib ? I can certainly do that if that's
> your point :)

I'm saying over time I think many of the python functions need to move.
This close to feature freeze may not be the best time, I just wanted to
give a clear view of my intent.

Sometimes the variable dependencies don't work the same way from the
python library so we need to be careful.

Cheers,

Richard

Re: [OE-Core][PATCH 1/4] testimage: create a list of failed test post actions

[Alexis Lothoré]

On 2/21/24 08:56, Richard Purdie wrote:
> On Wed, 2024-02-21 at 08:53 +0100, Alexis Lothoré wrote:
>> On 2/21/24 08:34, Richard Purdie wrote:
>>> On Tue, 2024-02-20 at 21:01 +0100, Alexis Lothoré via
>>> lists.openembedded.org wrote:
>>>> From: Alexis Lothoré <alexis.lothore@bootlin.com>
>>
>> [...]
>>
>>>> +################################################################
>>>> ##
>>>> +# General post actions runner
>>>> +################################################################
>>>> ##
>>>> +
>>>> +def run_failed_tests_post_actions(d, tc):
>>>> +    post_actions=[
>>>> +        list_and_fetch_failed_tests_artifacts
>>>> +    ]
>>>> +
>>>> +    for action in post_actions:
>>>> +        action(d, tc)
>>>
>>> Rather than create a bbclass class of python functions, these
>>> should
>>> move to lib/oe and become a proper python library file?
>>
>> ACK, I will do that and create a proper python file for this.
>>
>>> Moving functions out of the class files is on my long term todo
>>> list so
>>> this seems like an idea opportunity.
>>
>> So should this series take the opportunity to move all already
>> existing python
>> functions from testimage to a lib ? I can certainly do that if that's
>> your point :)
> 
> I'm saying over time I think many of the python functions need to move.
> This close to feature freeze may not be the best time, I just wanted to
> give a clear view of my intent.
> 
> Sometimes the variable dependencies don't work the same way from the
> python library so we need to be careful.

Ok, thanks for the clarification. I will then only fix my series to not create a
new bbclass but a lib for now.

> Cheers,
> 
> Richard
> 
> 
> 

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com