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

fix ailing integration tests in test_public_api #429

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

a-dubs
Copy link
Collaborator

@a-dubs a-dubs commented Sep 24, 2024

Fixes: GH #428 🎉


Testing of changes:

EC2 minimal images:

manually verified my changes by modifying and running examples/ec2.py:

diff --git a/examples/ec2.py b/examples/ec2.py
index 84f025e..9a362c4 100755
--- a/examples/ec2.py
+++ b/examples/ec2.py
@@ -137,24 +137,15 @@ def demo():
     through a number of examples.
     """
     with pycloudlib.EC2(tag="examples") as ec2:
-        key_name = "test-ec2"
-        handle_ssh_key(ec2, key_name)
-
-        daily = ec2.daily_image(release="bionic")
-        daily_pro = ec2.daily_image(release="bionic", image_type=ImageType.PRO)
-        daily_pro_fips = ec2.daily_image(
-            release="bionic", image_type=ImageType.PRO_FIPS
-        )
-
-        launch_basic(ec2, daily)
-        launch_pro(ec2, daily_pro)
-        launch_pro_fips(ec2, daily_pro_fips)
-        custom_vpc(ec2, daily)
-        snapshot(ec2, daily)
-        launch_multiple(ec2, daily)
-        hot_add(ec2, daily)
+        from pprint import pprint
+        pprint(ec2.find_matching_images("minimal")[0])
+        pprint(ec2.find_matching_images("ubuntu*server")[0])
+        pprint(ec2.released_image(
+            release="noble",
+            image_type=ImageType.MINIMAL,
+        ))
 
 
 if __name__ == "__main__":
-    logging.basicConfig(level=logging.DEBUG)
+    logging.basicConfig(level=logging.INFO)
     demo()
diff --git a/pycloudlib/ec2/cloud.py b/pycloudlib/ec2/cloud.py
index fc57e40..ba5ea48 100644
--- a/pycloudlib/ec2/cloud.py
+++ b/pycloudlib/ec2/cloud.py
@@ -194,6 +194,37 @@ class EC2(BaseCloud):
                 "Values": [arch],
             },
         ]
+    
+    # function that will find all images matching a substring
+    def find_matching_images(
+        self,
+        substring: str,
+        include_deprecated: bool = False,
+    ):
+        filters = [
+            {
+                "Name": "name",
+                "Values": [
+                    f"*{substring}*"
+                ],
+            },
+        ]
+        owner = self._get_owner(image_type=ImageType.GENERIC)
+
+        images = self.client.describe_images(
+            Owners=[owner],
+            Filters=filters,
+            IncludeDeprecated=include_deprecated,
+        )
+
+        if not images.get("Images"):
+            raise ValueError(
+                "Could not find image matching {}".format(
+                    substring
+                )
+            )
+
+        return images["Images"]
 
     def _find_latest_image(
         self,

THEN run the ec2 public api integration tests:

tox -e integration-tests -- tests/integration_tests/test_public_api.py -k "ec2"

Testing of GCE service account issues

Ran the following locally (I have service account) and @mstpn ran locally on his machine:

tox -e integration-tests -- tests/integration_tests/test_public_api.py -k "gce""

This runs both normal and minimal public api integration tests for GCE.

Testing of SSH key failed re-uploading

This is tested by running ec2 public api integrations as mentioned earlier.

@a-dubs a-dubs marked this pull request as draft September 24, 2024 18:01
@a-dubs a-dubs force-pushed the GH-428-fix-ci-integration-tests branch from 9f86586 to beff451 Compare September 24, 2024 21:14
@mstpn
Copy link
Member

mstpn commented Sep 24, 2024

Tested this locally and both offending test_public_api tests from #428 now pass.

tox -e integration-tests -- tests/integration_tests/test_public_api.py::test_public_api -k "gce"
tox -e integration-tests -- tests/integration_tests/test_public_api.py::test_public_api_minimal_images -k "gce"

@a-dubs a-dubs force-pushed the GH-428-fix-ci-integration-tests branch from beff451 to 2c78910 Compare September 25, 2024 14:18
@a-dubs a-dubs requested a review from blackboxsw September 25, 2024 14:18
@a-dubs a-dubs changed the title fix(gce): fix camel case naming for service_account fix ailing integration tests in test_public_api Sep 25, 2024
@a-dubs a-dubs force-pushed the GH-428-fix-ci-integration-tests branch from 2c78910 to 4a82a9d Compare September 25, 2024 15:31
@a-dubs
Copy link
Collaborator Author

a-dubs commented Sep 25, 2024

@blackboxsw @mstpn The holy trinity has landed. Feast your eyes

@a-dubs
Copy link
Collaborator Author

a-dubs commented Sep 25, 2024

also: PLEASE rebase when merging - all 3 commits are separate logical commits

@a-dubs a-dubs marked this pull request as ready for review September 25, 2024 15:48
@a-dubs a-dubs force-pushed the GH-428-fix-ci-integration-tests branch from 4a82a9d to 5cab887 Compare September 25, 2024 16:13
@mstpn
Copy link
Member

mstpn commented Sep 25, 2024

LGTM, tox formatting/linting notwithstanding. Thanks for working through this with me and helping resolve something I missed in #409.

@a-dubs a-dubs force-pushed the GH-428-fix-ci-integration-tests branch from 566ed4c to f453fd2 Compare September 25, 2024 16:31
@a-dubs
Copy link
Collaborator Author

a-dubs commented Sep 25, 2024

okay finally properly ready for review @blackboxsw

base_location,
release,
"-daily" if daily else "",
"-minimal" if image_type == ImageType.MINIMAL else "",
"minimal" if image_type == ImageType.MINIMAL else "server",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for this fix, this explains the inability to find minimal images on my PR. thx

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yessir 😈😤💯🔥

Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Ship it! Ran through minimal ec2 integration tests for focal, jammy, noble and oracular ensuring we can obtain minimal images appropriately. Validated GCP as well continues to work for service_account_email config in both pycloudlib.toml and GCE ~/.config/gcloud/*creds.json

@blackboxsw blackboxsw merged commit 3752add into canonical:main Sep 25, 2024
5 checks passed
@a-dubs
Copy link
Collaborator Author

a-dubs commented Sep 25, 2024

@blackboxsw thank you so much for thoroughly retesting this!

@a-dubs
Copy link
Collaborator Author

a-dubs commented Sep 25, 2024

LGTM, tox formatting/linting notwithstanding. Thanks for working through this with me and helping resolve something I missed in #409.

My pleasure! Thanks for helping out with this :)

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