Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scripts for testing the project using testflinger #234

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

nadzyah
Copy link
Collaborator

@nadzyah nadzyah commented Nov 6, 2024

This PR adds the scripts/tf-tests/ with the information regarding how to run the hwctl client on different machines that we have available via Testflinger.
The description of these tests can be found in README.md in this directory

@nadzyah nadzyah requested review from plars and boukeas November 6, 2024 15:34
@nadzyah nadzyah changed the title Add scripts for running tests using testflinger Add scripts for testing client using testflinger Nov 6, 2024
@nadzyah nadzyah changed the title Add scripts for testing client using testflinger Add scripts for testing the project using testflinger Nov 6, 2024
scripts/tf-tests/README.md Outdated Show resolved Hide resolved
scripts/tf-tests/README.md Outdated Show resolved Hide resolved
scripts/tf-tests/run-jobs.py Outdated Show resolved Hide resolved
scripts/tf-tests/check-status.py Outdated Show resolved Hide resolved
scripts/tf-tests/README.md Outdated Show resolved Hide resolved
scripts/tf-tests/check-status.py Outdated Show resolved Hide resolved
scripts/tf-tests/check-status.py Outdated Show resolved Hide resolved
scripts/tf-tests/README.md Outdated Show resolved Hide resolved
@nadzyah nadzyah mentioned this pull request Nov 11, 2024
@nadzyah nadzyah requested a review from boukeas November 12, 2024 16:23
Copy link
Collaborator

@plars plars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this out, the hwctl tool said status: Not Seen for all the things I tried it on, but maybe they weren't new enough. A few fixes below...

Then copy the created file to this directory:

```sh
cp target/release/hwctl scripts/tf-tests/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the hwctl binary there but it didn't work, I got an error saying that it expected it to be in scripts/tf-tests/job_outputs instead:

FileNotFoundError: [Errno 2] No such file or directory: '/home/plars/work/cert/hardware-api/scripts/tf-tests/job_outputs/hwctl'

That's because of a recent change in testflinger-cli that defaults to assuming that attachments will be in the same directory as your job yaml file. There's an override for it though that I'll propose below as a diff.

build --release` in the project root directory. You're supposed to
build the binary on the same Ubuntu release that is specified in
`tf-job.yaml`. The machines should also be of the same architecture
the binary is build for.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you build it under a different release? "You're supposed to..." is a little vague there I think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you get an error finding the right version of glibc. For testing purposes only, I wonder if it would be worth mentioning how to compile statically so that you can run it on something even if you aren't sure of the version? RUSTFLAGS='-C target-feature=+crt-static' cargo build --release --target x86_64-unknown-linux-gnu worked for me to do this.

@@ -0,0 +1,11 @@
job_queue: "$CANONICAL_ID"
provision_data:
distro: jammy
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth mentioning in the README that this tf-job.yaml will ONLY work on maas provisioned machines, and not on things like muxpi, or oemscript provisioned systems.

"""Submits the job and returns the job UUID."""
try:
result = subprocess.run(
["testflinger", "submit", str(temp_job_file)],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
["testflinger", "submit", str(temp_job_file)],
["testflinger", "submit", "--attachments-relative-to", ".", str(temp_job_file)],

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does --attachments-relative-to mean that hwctl is located in the same dir where I call testflinger-cli submit?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, --attachments-relative-to . means that any attachments that you specify using a relative path will be resolved against ., i.e. the current working directory.

@nadzyah
Copy link
Collaborator Author

nadzyah commented Nov 16, 2024

I tried this out, the hwctl tool said status: Not Seen for all the things I tried it on, but maybe they weren't new enough. A few fixes below...

Have you tried it on certified amd64 machines?

Comment on lines +62 to +73
def create_job_yaml(template_file: Path, canonical_id: str) -> str:
"""Creates a modified job YAML for a specific Canonical ID."""
with open(template_file, "r", encoding="utf8") as file:
job_yaml = file.read()
return job_yaml.replace("$CANONICAL_ID", canonical_id)


def write_temp_job_file(job_yaml: str, output_dir: Path, canonical_id: str) -> Path:
"""Writes the modified job YAML to a temporary file."""
temp_job_file = output_dir / f"{canonical_id}_tf-job.yaml"
temp_job_file.write_text(job_yaml)
return temp_job_file
Copy link

@boukeas boukeas Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not major but the split in two functions here seems artificial. You can even see proof of that in the docstring for write_temp_job_file: it is not easy to describe what this function does without referring to the output of the other function, i.e. "the modified job YAML" and you probably wouldn't use the second function without the first one. I would suggest merging these two into one function: what you are really doing here is instantiating the template with a value for $CANONICAL_ID.

I would also suggest accepting the path for the output (instantiated) file as an argument, rather than hardcoding the temp_job_file in the function itself. So the signature for the merged function would be something like:

def instantiate_job_yaml(template: Path, output: Path, canonical_id: str):

save_job_uuid(job_uuid, output_dir, canonical_id)
job_ids[job_uuid] = output_dir / canonical_id

temp_job_file.unlink() # Clean up temporary YAML file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you if you use it or not, but whenever there's this pattern of creating and deleting temporary files, you can also use the contexts in tempfile.

results_data = json.loads(results_result.stdout)
test_output = results_data.get("test_output", "")
(id_dir / "output.txt").write_text(test_output)
status = extract_status_from_output(test_output)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little confused here: can't we get the exit status of the test phase in the same way we get it's output, i.e. status = results_data.get("test_status")?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants