[PATCH 0/2] oeqa/utils/postactions: fix disk storage consumption

[PATCH 0/2] oeqa/utils/postactions: fix disk storage consumption

[Alexis Lothoré]

Hello,
this small series aims to fix Bugzilla issue 15536 [1], which results in a
big disk storage consumption and/or inode max count issue. The main root
cause is that in its current form, the artifacts retriever convert symlinks
to the targeted files. This series changes the way those files are
retrieved to make sure the symlinks are preserved. To do so, it replaces
the series of scp performed on each file by a single tar command which
output the generated compressed archive on its stdout. It is captured and
returned as part of an ssh process, and it it then fed to another tar
command run this time on host, to extract the artifacts, at the same place
as before (tmp/log/oeqa/artifacts)

The first commit is a small update to SSH target in ssh.py to allow to
retrieve raw output (ie raw bytes, without any decoding or formatting) from
a command run on the target. This allows to get the compressed archive
directly as an output from the remote command.
The second commit uses this new feature to generate, retrieve and extract
this archive on the fly, without ever storing it onto the target nor the
host.

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

Alexis Lothoré (2):
  oeqa/ssh: allow to retrieve raw, unformatted ouput
  oeqa/utils/postactions: transfer whole archive over ssh instead of
    doing individual copies

 meta/lib/oeqa/core/target/ssh.py   | 16 ++++++++--------
 meta/lib/oeqa/utils/postactions.py | 19 +++++++++----------
 2 files changed, 17 insertions(+), 18 deletions(-)

-- 
2.45.2

[PATCH 1/2] oeqa/ssh: allow to retrieve raw, unformatted ouput

[Alexis Lothoré]

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

The ssh target is currently well tailored to easily retrieve textual output
from a command run on a remote target. It could also be used to retrieve
raw data from a command run onto a remote target (for example, to feed this
data directly to another program), but it currently suffers two minor
issues preventing such use case:
- stderr is piped to stdout, so any error log will be mixed in the program
  output
- the final output is decoded as utf-8 and stripped

Allow to return the raw, unmodified output by adding an optional "raw"
parameter. Keep it to False by default to preserve the current behavior.
When enabled, do not return a string but the raw output as bytes.

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 meta/lib/oeqa/core/target/ssh.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/meta/lib/oeqa/core/target/ssh.py b/meta/lib/oeqa/core/target/ssh.py
index 09cdd14c759d..d4734693848e 100644
--- a/meta/lib/oeqa/core/target/ssh.py
+++ b/meta/lib/oeqa/core/target/ssh.py
@@ -55,14 +55,14 @@ class OESSHTarget(OETarget):
     def stop(self, **kwargs):
         pass
 
-    def _run(self, command, timeout=None, ignore_status=True):
+    def _run(self, command, timeout=None, ignore_status=True, raw=False):
         """
             Runs command in target using SSHProcess.
         """
         self.logger.debug("[Running]$ %s" % " ".join(command))
 
         starttime = time.time()
-        status, output = SSHCall(command, self.logger, timeout)
+        status, output = SSHCall(command, self.logger, timeout, raw)
         self.logger.debug("[Command returned '%d' after %.2f seconds]"
                  "" % (status, time.time() - starttime))
 
@@ -72,7 +72,7 @@ class OESSHTarget(OETarget):
 
         return (status, output)
 
-    def run(self, command, timeout=None, ignore_status=True):
+    def run(self, command, timeout=None, ignore_status=True, raw=False):
         """
             Runs command in target.
 
@@ -91,7 +91,7 @@ class OESSHTarget(OETarget):
         else:
             processTimeout = self.timeout
 
-        status, output = self._run(sshCmd, processTimeout, ignore_status)
+        status, output = self._run(sshCmd, processTimeout, ignore_status, raw)
         self.logger.debug('Command: %s\nStatus: %d Output:  %s\n' % (command, status, output))
 
         return (status, output)
@@ -206,7 +206,7 @@ class OESSHTarget(OETarget):
                 remoteDir = os.path.join(remotePath, tmpDir.lstrip("/"))
                 self.deleteDir(remoteDir)
 
-def SSHCall(command, logger, timeout=None, **opts):
+def SSHCall(command, logger, timeout=None, raw=False, **opts):
 
     def run():
         nonlocal output
@@ -265,7 +265,7 @@ def SSHCall(command, logger, timeout=None, **opts):
         else:
             output_raw = process.communicate()[0]
 
-        output = output_raw.decode('utf-8', errors='ignore')
+        output = output_raw if raw else output_raw.decode('utf-8', errors='ignore')
         logger.debug('Data from SSH call:\n%s' % output.rstrip())
 
         # timout or not, make sure process exits and is not hanging
@@ -292,7 +292,7 @@ def SSHCall(command, logger, timeout=None, **opts):
 
     options = {
         "stdout": subprocess.PIPE,
-        "stderr": subprocess.STDOUT,
+        "stderr": subprocess.STDOUT if not raw else None,
         "stdin": None,
         "shell": False,
         "bufsize": -1,
@@ -320,4 +320,4 @@ def SSHCall(command, logger, timeout=None, **opts):
         logger.debug('Something went wrong, killing SSH process')
         raise
 
-    return (process.returncode, output.rstrip())
+    return (process.returncode, output if raw else output.rstrip())
-- 
2.45.2

[PATCH 2/2] oeqa/utils/postactions: transfer whole archive over ssh instead of doing individual copies

[Alexis Lothoré]

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

Fixes [YOCTO 15536]

The postactions retrieval actions currently rely on scp executed
individually on any file or directory expanded from
TESTIMAGE_FAILED_QA_ARTIFACTS. Unfortunately, symlinks are not preserved
with this mechanism, which lead to big storage space consumption. Things
may go even worse if those symlinks create some circular chains. This
mechanism then needs to be updated to preserve symlinks instead of
following them during copy. There are multiple ways to do it:
- create a local archive on the target and execute scp on this file
- use rsync instead of scp for all files
- create an archive and pipe it to ssh instead of storing it onto the
  target

The first solution may create pressure on targets storage space, while the
second assumes that rsync is installed on the target, which may not be
true. So the third one is a compromise: tar is very likely present, at
least through busybox, and no disk space is used on the target.

Replace the current per-file scp call by a single call to tar run on the
target. Retrieve the generated compressed archive directly from SSH output,
and feed it to another tar process but on host, to uncompress and extract
it at the same place as before.

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 meta/lib/oeqa/utils/postactions.py | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/meta/lib/oeqa/utils/postactions.py b/meta/lib/oeqa/utils/postactions.py
index ecdddd2d40e3..f7ce25f2af8e 100644
--- a/meta/lib/oeqa/utils/postactions.py
+++ b/meta/lib/oeqa/utils/postactions.py
@@ -62,17 +62,16 @@ def get_artifacts_list(target, raw_list):
     return result
 
 def retrieve_test_artifacts(target, artifacts_list, target_dir):
+    import io, subprocess
     local_artifacts_dir = os.path.join(target_dir, "artifacts")
-    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}")
+    try:
+        cmd = f"tar zcf - {" ".join(artifacts_list)}"
+        (status, output) = target.run(cmd, raw = True)
+        if status != 0 or not output:
+            raise Exception("Error while fetching compressed artifacts")
+        p = subprocess.run(["tar", "zxf", "-", "-C", local_artifacts_dir], input=output)
+    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"))
-- 
2.45.2

Patchtest results for [PATCH 2/2] oeqa/utils/postactions: transfer whole archive over ssh instead of doing individual copies

[patchtest]

[-- Attachment #1: Type: text/plain, Size: 3120 bytes --]

Thank you for your submission. Patchtest identified one
or more issues with the patch. Please see the log below for
more information:

---
Testing patch /home/patchtest/share/mboxes/2-2-oeqa-utils-postactions-transfer-whole-archive-over-ssh-instead-of-doing-individual-copies.patch

FAIL: test bugzilla entry format: Bugzilla issue ID is not correctly formatted - specify it with format: "[YOCTO #<bugzilla ID>]" (test_mbox.TestMbox.test_bugzilla_entry_format)

PASS: pretest pylint (test_python_pylint.PyLint.pretest_pylint)
PASS: test Signed-off-by presence (test_mbox.TestMbox.test_signed_off_by_presence)
PASS: test author valid (test_mbox.TestMbox.test_author_valid)
PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence)
PASS: test max line length (test_metadata.TestMetadata.test_max_line_length)
PASS: test mbox format (test_mbox.TestMbox.test_mbox_format)
PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade)
PASS: test pylint (test_python_pylint.PyLint.test_pylint)
PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format)
PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length)

SKIP: pretest src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.pretest_src_uri_left_files)
SKIP: test CVE check ignore: No modified recipes or older target branch, skipping test (test_metadata.TestMetadata.test_cve_check_ignore)
SKIP: test CVE tag format: No new CVE patches introduced (test_patch.TestPatch.test_cve_tag_format)
SKIP: test Signed-off-by presence: No new CVE patches introduced (test_patch.TestPatch.test_signed_off_by_presence)
SKIP: test Upstream-Status presence: No new CVE patches introduced (test_patch.TestPatch.test_upstream_status_presence_format)
SKIP: test lic files chksum modified not mentioned: No modified recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence)
SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence)
SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head)
SKIP: test src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.test_src_uri_left_files)
SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence)
SKIP: test target mailing list: Series merged, no reason to check other mailing lists (test_mbox.TestMbox.test_target_mailing_list)

---

Please address the issues identified and
submit a new revision of the patch, or alternatively, reply to this
email with an explanation of why the patch should be accepted. If you
believe these results are due to an error in patchtest, please submit a
bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category
under 'Yocto Project Subprojects'). For more information on specific
failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank
you!

Re: [OE-core] [PATCH 2/2] oeqa/utils/postactions: transfer whole archive over ssh instead of doing individual copies

[Richard Purdie]

On Fri, 2024-07-05 at 16:46 +0200, Alexis Lothoré via lists.openembedded.org wrote:
> From: Alexis Lothoré <alexis.lothore@bootlin.com>
> 
> Fixes [YOCTO 15536]
> 
> The postactions retrieval actions currently rely on scp executed
> individually on any file or directory expanded from
> TESTIMAGE_FAILED_QA_ARTIFACTS. Unfortunately, symlinks are not preserved
> with this mechanism, which lead to big storage space consumption. Things
> may go even worse if those symlinks create some circular chains. This
> mechanism then needs to be updated to preserve symlinks instead of
> following them during copy. There are multiple ways to do it:
> - create a local archive on the target and execute scp on this file
> - use rsync instead of scp for all files
> - create an archive and pipe it to ssh instead of storing it onto the
>   target
> 
> The first solution may create pressure on targets storage space, while the
> second assumes that rsync is installed on the target, which may not be
> true. So the third one is a compromise: tar is very likely present, at
> least through busybox, and no disk space is used on the target.
> 
> Replace the current per-file scp call by a single call to tar run on the
> target. Retrieve the generated compressed archive directly from SSH output,
> and feed it to another tar process but on host, to uncompress and extract
> it at the same place as before.
> 
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> ---
>  meta/lib/oeqa/utils/postactions.py | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/meta/lib/oeqa/utils/postactions.py b/meta/lib/oeqa/utils/postactions.py
> index ecdddd2d40e3..f7ce25f2af8e 100644
> --- a/meta/lib/oeqa/utils/postactions.py
> +++ b/meta/lib/oeqa/utils/postactions.py
> @@ -62,17 +62,16 @@ def get_artifacts_list(target, raw_list):
>      return result
>  
>  def retrieve_test_artifacts(target, artifacts_list, target_dir):
> +    import io, subprocess
>      local_artifacts_dir = os.path.join(target_dir, "artifacts")
> -    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}")
> +    try:
> +        cmd = f"tar zcf - {" ".join(artifacts_list)}"

Thanks for the patch. The syntax above causes failures:

https://valkyrie.yoctoproject.org/#/builders/95/builds/59/steps/14/logs/stdio

I've put a fix on master-next which I can squash in but it does make me
wonder how it was tested.

Cheers,

Richard


> +        (status, output) = target.run(cmd, raw = True)
> +        if status != 0 or not output:
> +            raise Exception("Error while fetching compressed artifacts")
> +        p = subprocess.run(["tar", "zxf", "-", "-C", local_artifacts_dir], input=output)
> +    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"))
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#201610): https://lists.openembedded.org/g/openembedded-core/message/201610
> Mute This Topic: https://lists.openembedded.org/mt/107054834/1686473
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [richard.purdie@linuxfoundation.org]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

Re: [OE-core] [PATCH 2/2] oeqa/utils/postactions: transfer whole archive over ssh instead of doing individual copies

[Alexis Lothoré]

Hello Richard,

On 7/7/24 10:10, Richard Purdie wrote:
> On Fri, 2024-07-05 at 16:46 +0200, Alexis Lothoré via lists.openembedded.org wrote:
>> From: Alexis Lothoré <alexis.lothore@bootlin.com>
>> +    try:
>> +        cmd = f"tar zcf - {" ".join(artifacts_list)}"
> 
> Thanks for the patch. The syntax above causes failures:
> 
> https://valkyrie.yoctoproject.org/#/builders/95/builds/59/steps/14/logs/stdio
> 
> I've put a fix on master-next which I can squash in but it does make me
> wonder how it was tested.

Sorry about that. This error made me doubt about my testing setup, so I ran new
tests, and I confirm that this line is being executed and returns a correct tar
command on my machine, which confused me even more because it indeed looks broken.

My best guess about those differences is a mismatch on python versions between
the autobuilder and my machine. I am running python 3.12, and it appears that it
has brought many improvements to python f-strings, especially "quote reuse",
which makes python not raise an exception anymore when the expression inside an
f-string reuses the same kind of quotes as for the parent f-string (see [1]). I
ran a quick test with Python 3.11 instead of 3.12, and I now I see the exception.

I guess it would be better for me to keep working with the same python version
as in the autobuilder. I see mentions of python 3.6 in README-WALKTHROUGHS.md
from the autobuilder2 repository, but I am not sure how up-to-date it is. Could
you please confirm whether this is the correct version or not?

Thanks,

Alexis

[1]
https://docs.python.org/3/whatsnew/3.12.html#pep-701-syntactic-formalization-of-f-strings

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

Re: [OE-core] [PATCH 2/2] oeqa/utils/postactions: transfer whole archive over ssh instead of doing individual copies

[Richard Purdie]

On Mon, 2024-07-08 at 23:10 +0200, Alexis Lothoré wrote:
> Hello Richard,
> 
> On 7/7/24 10:10, Richard Purdie wrote:
> > On Fri, 2024-07-05 at 16:46 +0200, Alexis Lothoré via lists.openembedded.org wrote:
> > > From: Alexis Lothoré <alexis.lothore@bootlin.com>
> > > +    try:
> > > +        cmd = f"tar zcf - {" ".join(artifacts_list)}"
> > 
> > Thanks for the patch. The syntax above causes failures:
> > 
> > https://valkyrie.yoctoproject.org/#/builders/95/builds/59/steps/14/logs/stdio
> > 
> > I've put a fix on master-next which I can squash in but it does make me
> > wonder how it was tested.
> 
> Sorry about that. This error made me doubt about my testing setup, so I ran new
> tests, and I confirm that this line is being executed and returns a correct tar
> command on my machine, which confused me even more because it indeed looks broken.
> 
> My best guess about those differences is a mismatch on python versions between
> the autobuilder and my machine. I am running python 3.12, and it appears that it
> has brought many improvements to python f-strings, especially "quote reuse",
> which makes python not raise an exception anymore when the expression inside an
> f-string reuses the same kind of quotes as for the parent f-string (see [1]). I
> ran a quick test with Python 3.11 instead of 3.12, and I now I see the exception.

That explains things. I'm very surprised any version of python accepts
that formatting to be honest!

> I guess it would be better for me to keep working with the same python version
> as in the autobuilder. I see mentions of python 3.6 in README-WALKTHROUGHS.md
> from the autobuilder2 repository, but I am not sure how up-to-date it is. Could
> you please confirm whether this is the correct version or not?

The docs are out of date and the definitive version is the one checked in bitbake:

https://git.openembedded.org/bitbake/tree/lib/bb/__init__.py#n15

"Sorry, python 3.8.0 or later is required for this version of bitbake"

so 3.8+ is our current requirement.

Cheers,

Richard

Re: [OE-core] [PATCH 2/2] oeqa/utils/postactions: transfer whole archive over ssh instead of doing individual copies

[Alexis Lothoré]

On 7/8/24 23:37, Richard Purdie wrote:
> On Mon, 2024-07-08 at 23:10 +0200, Alexis Lothoré wrote:
>> Hello Richard,

[...]

>> My best guess about those differences is a mismatch on python versions between
>> the autobuilder and my machine. I am running python 3.12, and it appears that it
>> has brought many improvements to python f-strings, especially "quote reuse",
>> which makes python not raise an exception anymore when the expression inside an
>> f-string reuses the same kind of quotes as for the parent f-string (see [1]). I
>> ran a quick test with Python 3.11 instead of 3.12, and I now I see the exception.
> 
> That explains things. I'm very surprised any version of python accepts
> that formatting to be honest!
> 
>> I guess it would be better for me to keep working with the same python version
>> as in the autobuilder. I see mentions of python 3.6 in README-WALKTHROUGHS.md
>> from the autobuilder2 repository, but I am not sure how up-to-date it is. Could
>> you please confirm whether this is the correct version or not?
> 
> The docs are out of date and the definitive version is the one checked in bitbake:
> 
> https://git.openembedded.org/bitbake/tree/lib/bb/__init__.py#n15
> 
> "Sorry, python 3.8.0 or later is required for this version of bitbake"
> 
> so 3.8+ is our current requirement.

ACK, I'll then align with this version for any future development.

Thanks,

Alexis

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