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 custom amis import #5697

Merged
merged 10 commits into from Nov 30, 2022
Merged

Conversation

rafcio19
Copy link
Contributor

If custom amis are specified via MOTO_AMIS_PATH env var, then do not import default amis.

Resolves #5677

if custom amis are specified via MOTO_AMIS_PATH env var, then
do not import default amis
@bblommers
Copy link
Collaborator

Some context, in case you hadn't seen it yet: all tests in tests/test_ec2 are run in parallel, to speed things up. So testing this particular feature, and assert we have only one AMI available, is a bit tricky.

I haven't really sat down to think about how to do this - but maybe we can simply run this test from a separate folder, something like tests/test_ec2_custom_ami/test.py. That way we can continue running all 'regular' EC2 tests in parallel, and we can be sure that this particular test is always run on it's own, without interference from other tests.

Let me know if you want me to take a closer look.

@rafcio19
Copy link
Contributor Author

Thanks for looking @bblommers . I saw this behaviour when testing locally:

 pytest tests/test_ec2/test_*ami*

but running these fellas individually works:

pytest tests/test_ec2/test_amis.py
pytest tests/test_ec2/test_custom_amis.py

I will see if a bit of tinkering with the makefile will help.

@rafcio19 rafcio19 marked this pull request as ready for review November 29, 2022 19:17
@rafcio19
Copy link
Contributor Author

Hey @bblommers this is ready for a review, now lemme know what you think. I changed the Unit tests in Server Mode workflow to run the test_server make target and the tests pass this way.. 😬

Clearly this changes the CI a little bit but because the "regular" unit tests pass then perhaps it's ok.

Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

Thanks @rafcio19! I've refactored the test a bit - it now resets and re-imports the EC2-backend after setting the env variable. That way we can remove any existing AMI's, and only load our custom AMI, but without having to worry about running pytest multiple times.

@bblommers bblommers added this to the 4.0.11 milestone Nov 30, 2022
@bblommers bblommers merged commit ceffba0 into getmoto:master Nov 30, 2022
@github-actions
Copy link
Contributor

This is now part of moto >= 4.0.11.dev27

@rafcio19 rafcio19 deleted the feature/GH5667_default_amis branch January 6, 2023 15:38
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.

EC2 client describe_images returning default AMIs even with MOTO_AMIS_PATH set
2 participants