Boot test AMIs in AWS #93

achilleas-k · 2024-01-08T20:54:31Z

When building a test AMI, upload it to AWS and boot it there, running the same smoke test as we do for the qcow2.
The local testing of the AMI is still run as before.

Added two subclasses to the VM class:

QEMU: is the old code that initialises and boots an image in qemu.
AWS: new code that launches (and terminates) an AMI.

mvo5

Thanks for the PR! This is super nice and it's great to see how well this all seem to fit. I have a bunch of nipticks/ideas inline but all can be followups if you prefer that :)

test/vm.py

plans/all.fmf

test/vm.py

test/testutil.py

test/test_smoke.py

test/testutil.py

test/vm.py

test/test_smoke.py

achilleas-k · 2024-01-09T16:15:41Z

#93 (comment)

Added a new pytest flag, --force-aws-upload. Setting this flag makes the AMI build fail if the credentials aren't set. Without it, the test is skipped.
Apply the following patch and run the test to quickly see how it works:

diff --git a/test/test_smoke.py b/test/test_smoke.py
index 9703240..eb6ddc2 100644
--- a/test/test_smoke.py
+++ b/test/test_smoke.py
@@ -119,6 +119,8 @@ def image_type_fixture(tmpdir_factory, build_container, request, force_aws_uploa
                 # upload forced but credentials aren't set
                 raise RuntimeError("AWS credentials not available (upload forced)")
 
+            return
+
         # run container to deploy an image into a bootable disk and upload to a cloud service if applicable
         subprocess.check_call([
             "podman", "run", "--rm",

FAIL

$ sudo pytest -k test_ami_boots -v --force-aws-upload
================================================= test session starts =================================================
platform linux -- Python 3.11.6, pytest-7.4.4, pluggy-1.3.0 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/achilleas/projects/osbuild/bootc-image-builder
plugins: forked-1.6.0, xdist-3.5.0, anyio-4.2.0
collected 14 items / 13 deselected / 1 selected

test/test_smoke.py::test_ami_boots_in_aws[ami] ERROR                                                            [100%]

======================================================= ERRORS ========================================================
____________________________________ ERROR at setup of test_ami_boots_in_aws[ami] _____________________________________

<snip>

test/test_smoke.py:120: RuntimeError
=============================================== short test summary info ===============================================
ERROR test/test_smoke.py::test_ami_boots_in_aws[ami] - RuntimeError: AWS credentials not available (upload forced)
=========================================== 13 deselected, 1 error in 1.89s ===========================================

SKIP

$ sudo pytest -k test_ami_boots -v
================================================= test session starts =================================================
platform linux -- Python 3.11.6, pytest-7.4.4, pluggy-1.3.0 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/achilleas/projects/osbuild/bootc-image-builder
plugins: forked-1.6.0, xdist-3.5.0, anyio-4.2.0
collected 14 items / 13 deselected / 1 selected

test/test_smoke.py::test_ami_boots_in_aws[ami] SKIPPED (AWS credentials not available (upload not forced))      [100%]

========================================== 1 skipped, 13 deselected in 1.79s ==========================================

mvo5

I love it!

achilleas-k · 2024-01-09T16:58:24Z

I love it!

We'll anyway need to rebase once #95 gets merged. I was hoping that would be merged while I was working on the review comments here, otherwise I would have added it in this one.

achilleas-k · 2024-01-10T10:31:29Z

Seems the GH secrets aren't being picked up:

Added them as envs to the workflow
Added them as org secrets
But they're not being picked up in the run: log: step setup and log: test failure

(The value should appear as *** on lines 5 and 6 in the log, like here)

Feel like I'm missing something obvious.

mvo5 · 2024-01-10T10:58:14Z

Seems the GH secrets aren't being picked up:

* Added them as [envs to the workflow](https://github.com/osbuild/bootc-image-builder/blob/51a3d7f5895fc4da49b5b9ec4aa152a3836298bc/.github/workflows/tests.yml#L73-L81)

* Added them as [org secrets](https://github.com/osbuild/bootc-image-builder/settings/secrets/actions)

* But they're not being picked up in the run: [log: step setup](https://github.com/osbuild/bootc-image-builder/actions/runs/7464604252/job/20311939290?pr=93#step:6:5) and [log: test failure](https://github.com/osbuild/bootc-image-builder/actions/runs/7464604252/job/20311939290?pr=93#step:6:862)

(The value should appear as *** on lines 5 and 6 in the log, like here)

Feel like I'm missing something obvious.

I (strongly) suspect that this is because GH secrets are not available in repo forks. The easiest fix might be to just move this test into testingfarm.yml that already has the (sightly involved) setup to workaround this.

achilleas-k · 2024-01-10T13:07:23Z

Seems the GH secrets aren't being picked up:
* Added them as [envs to the workflow](https://github.com/osbuild/bootc-image-builder/blob/51a3d7f5895fc4da49b5b9ec4aa152a3836298bc/.github/workflows/tests.yml#L73-L81)

* Added them as [org secrets](https://github.com/osbuild/bootc-image-builder/settings/secrets/actions)

* But they're not being picked up in the run: [log: step setup](https://github.com/osbuild/bootc-image-builder/actions/runs/7464604252/job/20311939290?pr=93#step:6:5) and [log: test failure](https://github.com/osbuild/bootc-image-builder/actions/runs/7464604252/job/20311939290?pr=93#step:6:862)
(The value should appear as *** on lines 5 and 6 in the log, like here)
Feel like I'm missing something obvious.
I (strongly) suspect that this is because GH secrets are not available in repo forks. The easiest fix might be to just move this test into testingfarm.yml that already has the (sightly involved) setup to workaround this.

Right, yes, as we discussed offline (repeating here for posterity), the secrets are only available in tests from forks if they run on pull_request_target and we don't want to use that to trigger our tests because we want to block them when PR authors don't have write access to the repo, and that rule is overridden when the trigger is pull_request_target.

One solution we use in other repos, like images and osbuild-composer, is to have a helper workflow that runs on pull_request and starts a second workflow that runs the tests with a trigger that depends on a workflow_run (the helper workflow) completing successfully.

I think we can do that in a later PR though. For now I'll add the force-aws-upload flag to the TF tests and include the secrets in that workflow.

achilleas-k · 2024-01-10T19:06:51Z

Rebased and fixed conflict. Still needs #97.

Split out the qemu bits from the VM class and make a subclass called QEMU. The base VM class will be subclassed to make more implementations for other virtualisation environments and cloud platforms.

Linter complains about redefining the `type` built-in and `tb` not conforming to naming style. Renamed to match the names as they are in the docs.

Add the AWS python SDK to the test requirements.

mvo5

Thank you!

The AWS class manages a single EC2 instance from a given AMI. Entering the context defined by the class (i.e. with AWS(id) ...) performs the following actions (defined in start()): - Create a security group that allows SSH logins from any IP. - Create an instance from the given AMI ID and include the new security group. - Block until the instance is running. On exit, the instance is terminated and the security group deleted (in that order). Credentials aren't managed by the class. They are assumed to be managed externally (env vars). Based heavily on examples found in the aws sdk docs: https://github.com/awsdocs/aws-doc-sdk-examples/blob/afb3309b385db90e6ff01c0268429cde9610f53e/python/example_code/ec2/

Add a new running() method to the base class so that each subclass can define their own condition for determining if the VM is running. This and the IP address were the only things that differed in the two run() implementations.

write_aws_creds(): writes the AWS credentials defined in the environment to a file in the format required by the AWS SDK. deregister_ami(): deregister an image from EC2 given its ID.

When building AMIs, include upload options and credentials: - the `--aws-...` options are added as upload_args to the build command. - credentials are read from the environment, written to a temporary file, and mounted into the container. The environment variables will be set through the CI config.

Add the image type name and a metadata dict to the ImageBuildResult. We will use the image type to decide what we can do with the build result (like where and how to boot it). The metadata can be used to communicate information to downstream tests. Here we use it to include the AMI ID in the result. We could have let the tests that use the ImageBuildResult parse the log for the AMI ID, but it's probably better if we only have to do it once and also, parsing it in the image_type_fixture() lets us register the finalizer that will deregister the AMI from EC2. Parsing the log for the AMI ID is a bit dirty and is certainly not the most robust way to get it. We could later print it in a more machine-readable format.

A variant of the test_image_boots() test that only runs for the AMI image type. The function uses the AWS VM class to boot an AMI (given the AMI ID), run a couple of commands over SSH, then tears it down. As opposed to the local boot test, this one also runs on macOS.

Add a new pytest flag, --force-aws-upload, which controls how the AWS build (and boot) test should behave when the AWS credentials aren't set: - if set, the image build will raise a RuntimeError, making any tests that depend on the fixture fail. - if not set, the upload flags will not be set and the AWS boot test is skipped.

Add the AWS secrets to the testing farm action and enable `force-aws-upload` on the pytest call in the testing farm plan. Run these only in testing farm which runs on pull_request_target and can access GitHub secrets.

achilleas-k force-pushed the test/boot-aws branch from 0726716 to 4dfc4d2 Compare January 8, 2024 21:00

achilleas-k mentioned this pull request Jan 9, 2024

github: add AWS secrets to env vars for integration tests #95

Merged

mvo5 reviewed Jan 9, 2024

View reviewed changes

test/test_smoke.py Show resolved Hide resolved

achilleas-k force-pushed the test/boot-aws branch 2 times, most recently from 94b93ab to dc924d8 Compare January 9, 2024 16:06

mvo5 previously approved these changes Jan 9, 2024

View reviewed changes

achilleas-k force-pushed the test/boot-aws branch from c24c355 to d22bdcb Compare January 9, 2024 17:22

achilleas-k dismissed mvo5’s stale review via 1c9e117 January 10, 2024 13:12

achilleas-k force-pushed the test/boot-aws branch from d22bdcb to 1c9e117 Compare January 10, 2024 13:12

achilleas-k mentioned this pull request Jan 10, 2024

github: add AWS secrets to testing farm action #97

Merged

mvo5 previously approved these changes Jan 10, 2024

View reviewed changes

achilleas-k dismissed mvo5’s stale review via 5eb8dda January 10, 2024 19:06

achilleas-k force-pushed the test/boot-aws branch from 1c9e117 to 5eb8dda Compare January 10, 2024 19:06

achilleas-k force-pushed the test/boot-aws branch from 5eb8dda to 2985d42 Compare January 10, 2024 22:02

achilleas-k added 3 commits January 11, 2024 12:45

test: make QEMU subclass of VM class

78a9858

Split out the qemu bits from the VM class and make a subclass called QEMU. The base VM class will be subclassed to make more implementations for other virtualisation environments and cloud platforms.

test/vm: rename __exit__ args

f9ca5f2

Linter complains about redefining the `type` built-in and `tb` not conforming to naming style. Renamed to match the names as they are in the docs.

test: add boto3 to requirements.txt and test deps

3f8a60b

Add the AWS python SDK to the test requirements.

achilleas-k force-pushed the test/boot-aws branch 2 times, most recently from 4ffef69 to 0de356a Compare January 11, 2024 18:37

mvo5 previously approved these changes Jan 12, 2024

View reviewed changes

achilleas-k added 3 commits January 12, 2024 11:56

test: unify vm.run() implementations

2d53323

Add a new running() method to the base class so that each subclass can define their own condition for determining if the VM is running. This and the IP address were the only things that differed in the two run() implementations.

testutil: AWS utility functions

7d7de63

write_aws_creds(): writes the AWS credentials defined in the environment to a file in the format required by the AWS SDK. deregister_ami(): deregister an image from EC2 given its ID.

achilleas-k added 5 commits January 12, 2024 11:56

test: enable force-aws-upload for testing farm tests

21871cf

Add the AWS secrets to the testing farm action and enable `force-aws-upload` on the pytest call in the testing farm plan. Run these only in testing farm which runs on pull_request_target and can access GitHub secrets.

achilleas-k dismissed mvo5’s stale review via 21871cf January 12, 2024 10:56

achilleas-k force-pushed the test/boot-aws branch from 0de356a to 21871cf Compare January 12, 2024 10:56

mvo5 approved these changes Jan 12, 2024

View reviewed changes

ondrejbudai added this pull request to the merge queue Jan 12, 2024

Merged via the queue into osbuild:main with commit 73330dd Jan 12, 2024
10 checks passed

achilleas-k deleted the test/boot-aws branch January 12, 2024 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Boot test AMIs in AWS #93

Boot test AMIs in AWS #93

achilleas-k commented Jan 8, 2024 •

edited

Loading

mvo5 left a comment

achilleas-k commented Jan 9, 2024 •

edited

Loading

mvo5 left a comment

achilleas-k commented Jan 9, 2024

achilleas-k commented Jan 10, 2024

mvo5 commented Jan 10, 2024

achilleas-k commented Jan 10, 2024

achilleas-k commented Jan 10, 2024

mvo5 left a comment

Boot test AMIs in AWS #93

Boot test AMIs in AWS #93

Conversation

achilleas-k commented Jan 8, 2024 • edited Loading

mvo5 left a comment

Choose a reason for hiding this comment

achilleas-k commented Jan 9, 2024 • edited Loading

mvo5 left a comment

Choose a reason for hiding this comment

achilleas-k commented Jan 9, 2024

achilleas-k commented Jan 10, 2024

mvo5 commented Jan 10, 2024

achilleas-k commented Jan 10, 2024

achilleas-k commented Jan 10, 2024

mvo5 left a comment

Choose a reason for hiding this comment

achilleas-k commented Jan 8, 2024 •

edited

Loading

achilleas-k commented Jan 9, 2024 •

edited

Loading