From 522658028c30676a72212ff7cd4087b9b56d41a0 Mon Sep 17 00:00:00 2001 From: George Boukeas Date: Tue, 5 Nov 2024 19:42:08 +0000 Subject: [PATCH] [CERTTF-424] feat: support a reference directory for attachments (#374) * feat(cli): introduce support for attachments reference directory * feat: attachment reference directory support in the submit Github action * feat: change default behaviour to consider attachments relative to job file * chore: add tests for packing attachments (include using a reference) * docs: add documentation for new command-line argument * docs: update documentation to reflect change in default attachment behaviour --- .github/actions/submit/action.yaml | 9 +- README.md | 2 +- cli/testflinger_cli/__init__.py | 62 +++++++---- cli/testflinger_cli/tests/test_cli.py | 141 +++++++++++++++++++++++++- docs/reference/test-phases.rst | 26 ++++- 5 files changed, 214 insertions(+), 26 deletions(-) diff --git a/.github/actions/submit/action.yaml b/.github/actions/submit/action.yaml index 529c3972..3dec7e8e 100644 --- a/.github/actions/submit/action.yaml +++ b/.github/actions/submit/action.yaml @@ -19,6 +19,9 @@ inputs: description: The Testflinger server to use required: false default: testflinger.canonical.com + attachments-relative-to: + description: The reference directory for relative attachment paths + required: false outputs: id: description: 'The ID of the submitted job' @@ -81,8 +84,12 @@ runs: shell: bash env: SERVER: https://${{ inputs.server }} + RELATIVE_TO: ${{ inputs.attachments-relative-to }} run: | - JOB_ID=$(testflinger --server $SERVER submit --quiet "$JOB") + if [ -n "$RELATIVE_TO" ]; then + RELATIVE="--attachments-relative-to $RELATIVE_TO" + fi + JOB_ID=$(testflinger --server $SERVER submit --quiet "$RELATIVE" "$JOB") echo "job id: $JOB_ID" echo "id=$JOB_ID" >> $GITHUB_OUTPUT diff --git a/README.md b/README.md index b6760f64..35256b1e 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ This monorepo is organized in a way that is consistant with the components descr # Github actions -If you need to submit a job to a testflinger server through a Github action (instead, for example, of using the command-line tool), you can use the [`submit` action](https://github.com/canonical/testflinger/blob/main/.github/actions/submit/action.yaml) in a CI workflow. +If you need to submit a job to a testflinger server through a Github action (instead, for example, of using the command-line tool), you can use the [`submit` action](https://github.com/canonical/testflinger/blob/main/.github/actions/submit/action.yaml) in a CI workflow. Please refer to the `inputs` field of the action for a complete list of the arguments that the action can receive. The corresponding step in the workflow would look like this: ```yaml diff --git a/cli/testflinger_cli/__init__.py b/cli/testflinger_cli/__init__.py index 02bebd3a..699d5c68 100644 --- a/cli/testflinger_cli/__init__.py +++ b/cli/testflinger_cli/__init__.py @@ -125,15 +125,6 @@ def _print_queue_message(): ) -def make_relative(path: Path) -> Path: - """Return resolved relative `path`""" - if path.is_absolute(): - # strip leading '/' from absolute path - path = path.relative_to(path.root) - # resolve the path and make relative again - return path.resolve().relative_to(Path.cwd()) - - class AttachmentError(Exception): """Exception thrown when attachments fail to be submitted""" @@ -292,6 +283,12 @@ def get_args(self): arg_submit.add_argument("--poll", "-p", action="store_true") arg_submit.add_argument("--quiet", "-q", action="store_true") arg_submit.add_argument("filename") + relative = arg_submit.add_mutually_exclusive_group() + relative.add_argument( + "--attachments-relative-to", + dest="relative", + help="The reference directory for relative attachment paths", + ) self.args = parser.parse_args() self.help = parser.format_help() @@ -347,17 +344,19 @@ def configure(self): @staticmethod def extract_attachment_data(job_data: dict) -> Optional[dict]: """Pull together the attachment data per phase from the `job_data`""" - attachment_data = {} + attachments = {} for phase in ("provision", "firmware_update", "test"): - phase_str = f"{phase}_data" try: - attachment_data[phase] = job_data[phase_str]["attachments"] + attachments[phase] = [ + attachment + for attachment in job_data[f"{phase}_data"]["attachments"] + if attachment.get("local") + ] except KeyError: pass - return attachment_data or None + return attachments or None - @staticmethod - def pack_attachments(archive: str, attachment_data: dict): + def pack_attachments(self, archive: str, attachment_data: dict): """Pack the attachments specifed by `attachment_data` into `archive` Use `tarfile` instead of `shutil` because: @@ -367,15 +366,42 @@ def pack_attachments(archive: str, attachment_data: dict): > owner. Ref: https://docs.python.org/3/library/tarfile.html """ + # determine the reference directory for relative attachment paths + if self.args.relative: + # provided as a command-line argument + reference = Path(self.args.relative).resolve(strict=True) + else: + # retrieved from the directory where the job file is contained + reference = Path(self.args.filename).parent.resolve(strict=True) + with tarfile.open(archive, "w:gz") as tar: for phase, attachments in attachment_data.items(): phase_path = Path(phase) for attachment in attachments: local_path = Path(attachment["local"]) - # determine archive name for attachment + if not local_path.is_absolute(): + # make relative attachment path absolute + local_path = reference / local_path + local_path = local_path.resolve() + # determine the archive path for the attachment # (essentially: the destination path on the agent host) - agent_path = Path(attachment.get("agent", local_path)) - agent_path = make_relative(agent_path) + try: + agent_path = Path(attachment["agent"]) + if agent_path.is_absolute(): + # strip leading '/' from absolute path + agent_path = agent_path.relative_to( + agent_path.root + ) + except KeyError: + # no agent path provided: determine it from local path + try: + # make agent path relative to the reference path + agent_path = local_path.relative_to(reference) + except ValueError: + # unable to determine the agent path (cannot make + # the local path relative to the reference path): + # just use the filename + agent_path = local_path.name archive_path = phase_path / agent_path tar.add(local_path, arcname=archive_path) # side effect: strip "local" information diff --git a/cli/testflinger_cli/tests/test_cli.py b/cli/testflinger_cli/tests/test_cli.py index 7d9c6d8c..ad35ef0b 100644 --- a/cli/testflinger_cli/tests/test_cli.py +++ b/cli/testflinger_cli/tests/test_cli.py @@ -19,6 +19,8 @@ """ import json +import os +from pathlib import Path import re import sys import tarfile @@ -107,6 +109,143 @@ def test_submit_bad_data(tmp_path, requests_mock): ) +def test_pack_attachments(tmp_path): + """Make sure attachments are packed correctly""" + + attachments = [ + Path() / "file_0.bin", + Path() / "folder" / "file_1.bin", + ] + attachment_path = tmp_path / "attachments" + + # create attachment files in the attachment path + for attachment in attachments: + attachment = attachment_path / attachment + attachment.parent.mkdir(parents=True) + attachment.write_bytes(os.urandom(128)) + + attachment_data = { + "test": [ + { + # relative local file path + "local": str(attachments[0]) + }, + { + # relative local file path in folder + "local": str(attachments[1]) + }, + { + # absolute local file path, agent path in folder + "local": str((attachment_path / attachments[0]).resolve()), + "agent": "folder/file_2.bin", + }, + { + # relative local path is a directory + "local": str(attachments[1].parent), + "agent": "folder/deeper/", + }, + { + # agent path is absolute (stripped, becomes relative) + "local": str(attachments[0]), + "agent": "/file_3.bin", + }, + ] + } + + # the job.yaml is also in the attachments path + # (and relative attachment paths are interpreted in relation to that) + sys.argv = ["", "submit", f"{attachment_path}/job.yaml"] + tfcli = testflinger_cli.TestflingerCli() + archive = tmp_path / "attachments.tar.gz" + tfcli.pack_attachments(archive, attachment_data) + + with tarfile.open(archive) as archive: + filenames = archive.getnames() + print(filenames) + assert "test/file_0.bin" in filenames + assert "test/folder/file_1.bin" in filenames + assert "test/folder/file_2.bin" in filenames + assert "test/folder/deeper/file_1.bin" in filenames + assert "test/file_3.bin" in filenames + + +def test_pack_attachments_with_reference(tmp_path): + """Make sure attachments are packed correctly when using a reference""" + + attachments = [ + Path() / "file_0.bin", + Path() / "folder" / "file_1.bin", + ] + attachment_path = tmp_path / "attachments" + + # create attachment files + for attachment in attachments: + attachment = attachment_path / attachment + attachment.parent.mkdir(parents=True) + attachment.write_bytes(os.urandom(128)) + + attachment_data = { + "test": [ + { + # relative local file path + "local": str(attachments[0]) + }, + { + # relative local file path in folder + "local": str(attachments[1]) + }, + { + # absolute local file path, agent path in folder + "local": str((attachment_path / attachments[0]).resolve()), + "agent": "folder/file_2.bin", + }, + { + # relative local path is a directory + "local": str(attachments[1].parent), + "agent": "folder/deeper/", + }, + { + # agent path is absolute (stripped, becomes relative) + "local": str(attachments[0]), + "agent": "/file_3.bin", + }, + ] + } + + # the job.yaml is in the tmp_path + # (so packing the attachments with fail without specifying that + # attachments are relative to the attachments path) + sys.argv = ["", "submit", f"{tmp_path}/job.yaml"] + tfcli = testflinger_cli.TestflingerCli() + archive = tmp_path / "attachments.tar.gz" + with pytest.raises(FileNotFoundError): + # this fails because the job file is in `tmp_path` whereas + # attachments are in `tmp_path/attachments` + tfcli.pack_attachments(archive, attachment_data) + + # the job.yaml is in the tmp_path + # (but now attachments are relative to the attachments path) + sys.argv = [ + "", + "submit", + f"{tmp_path}/job.yaml", + "--attachments-relative-to", + f"{attachment_path}", + ] + tfcli = testflinger_cli.TestflingerCli() + archive = tmp_path / "attachments.tar.gz" + tfcli.pack_attachments(archive, attachment_data) + + with tarfile.open(archive) as archive: + filenames = archive.getnames() + print(filenames) + assert "test/file_0.bin" in filenames + assert "test/folder/file_1.bin" in filenames + assert "test/folder/file_2.bin" in filenames + assert "test/folder/deeper/file_1.bin" in filenames + assert "test/file_3.bin" in filenames + + def test_submit_with_attachments(tmp_path): """Make sure jobs with attachments are submitted correctly""" @@ -157,7 +296,7 @@ def test_submit_with_attachments(tmp_path): with tarfile.open("attachments.tar.gz") as attachments: filenames = attachments.getnames() assert len(filenames) == 1 - attachments.extract(filenames[0], filter="data") + attachments.extract(filenames[0]) with open(filenames[0], "r", encoding="utf-8") as attachment: assert json.load(attachment) == job_data diff --git a/docs/reference/test-phases.rst b/docs/reference/test-phases.rst index 6a4e1690..3ef962fe 100644 --- a/docs/reference/test-phases.rst +++ b/docs/reference/test-phases.rst @@ -215,7 +215,9 @@ Example agent configuration: Attachments ------------ -In the `provisioning`, `firmware_update` and `test` phases, it is also possible to specify attachments, i.e. local files that are to be copied over to the Testflinger agent host. +In the `provisioning`, `firmware_update` and `test` phases, it is also possible +to specify attachments, i.e. local files that are to be copied over to +the Testflinger agent host. * Example job definition: @@ -239,11 +241,17 @@ In the `provisioning`, `firmware_update` and `test` phases, it is also possible chmod u+x attachments/test/script.sh attachments/test/script.sh - The `local` fields specify where the attachments are to be found locally, e.g. on the machine where the CLI is executed. For this particular example, this sort of file tree is expected: + The `local` fields specify where the attachments are to be found locally, + e.g. on the machine where the CLI is executed. Unless otherwise specified, + relative paths are interpreted in relation to the location of the Testflinger + job file (which is convenient since the job file and the attachments are + usually stored together). + So for this particular example, this sort of file tree is expected: .. code-block:: bash . + ├── job.yaml ├── config.json ├── images │ └── ubuntu-logo.png @@ -251,7 +259,10 @@ In the `provisioning`, `firmware_update` and `test` phases, it is also possible │ └── my_test_script.sh └── ubuntu-22.04.4-preinstalled-desktop-arm64+raspi.img.xz - On the agent host, the attachments are placed under the `attachments` folder and distributed in separate sub-folders according to phase. If an `agent` field is provided, the attachments are also moved or renamed accordingly. For the example above, the file tree on the agent host would look like this: + On the agent host, the attachments are placed under the `attachments` folder + and distributed in separate sub-folders according to phase. If an `agent` + field is provided, the attachments are also moved or renamed accordingly. + For the example above, the file tree on the agent host would look like this: .. code-block:: bash @@ -267,8 +278,13 @@ In the `provisioning`, `firmware_update` and `test` phases, it is also possible │ └── ubuntu-logo.png └── script.sh -In this example, there is no `url` field under the `provision_data` to specify where to download the provisioning image from. -Instead, there is a `use_attachment` field that indicates which attachment should be used as a provisioning image. +The Testflinger CLI also accepts an optional `--attachments-relative-to` argument. +When provided, relative paths are interpreted in relation to this reference path, +instead of the default, i.e. the location of the Testflinger job file. + +In the example above, there is no `url` field under the `provision_data` to specify +where to download the provisioning image from. Instead, there is a `use_attachment` +field that indicates which attachment should be used as a provisioning image. The presence of *either* `url` or `use_attachment` is required. At the moment, only the :ref:`muxpi` device connector supports provisioning using an attached image.