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
CFn: Parametrize Fn::GetAZs test across all regions #10681
base: master
Are you sure you want to change the base?
Changes from all commits
dd2d47e
a622025
fd2d463
8e7af35
49aad81
c2651db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
from werkzeug import Request, Response | ||
|
||
from localstack import config | ||
from localstack.aws.connect import ServiceLevelClientFactory | ||
from localstack.constants import ( | ||
AWS_REGION_US_EAST_1, | ||
SECONDARY_TEST_AWS_ACCOUNT_ID, | ||
|
@@ -955,13 +956,9 @@ def format_event(event: dict) -> str: | |
|
||
@pytest.fixture | ||
def deploy_cfn_template( | ||
cleanup_stacks, | ||
cleanup_changesets, | ||
is_change_set_created_and_available, | ||
is_change_set_finished, | ||
aws_client, | ||
aws_client: ServiceLevelClientFactory, | ||
): | ||
state = [] | ||
state: list[tuple[str, Callable]] = [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Always nice to see more typehints added to project :) |
||
|
||
def _deploy( | ||
*, | ||
|
@@ -970,11 +967,12 @@ def _deploy( | |
change_set_name: Optional[str] = None, | ||
template: Optional[str] = None, | ||
template_path: Optional[str | os.PathLike] = None, | ||
template_mapping: Optional[Dict[str, any]] = None, | ||
template_mapping: Optional[Dict[str, Any]] = None, | ||
parameters: Optional[Dict[str, str]] = None, | ||
role_arn: Optional[str] = None, | ||
max_wait: Optional[int] = None, | ||
delay_between_polls: Optional[int] = 2, | ||
custom_aws_client: Optional[ServiceLevelClientFactory] = None, | ||
) -> DeployResult: | ||
if is_update: | ||
assert stack_name | ||
|
@@ -1005,17 +1003,18 @@ def _deploy( | |
if role_arn is not None: | ||
kwargs["RoleARN"] = role_arn | ||
|
||
response = aws_client.cloudformation.create_change_set(**kwargs) | ||
cfn_aws_client = custom_aws_client if custom_aws_client is not None else aws_client | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can add a comment here stating why was this custom aws client required |
||
|
||
response = cfn_aws_client.cloudformation.create_change_set(**kwargs) | ||
|
||
change_set_id = response["Id"] | ||
stack_id = response["StackId"] | ||
state.append({"stack_id": stack_id, "change_set_id": change_set_id}) | ||
|
||
aws_client.cloudformation.get_waiter(WAITER_CHANGE_SET_CREATE_COMPLETE).wait( | ||
cfn_aws_client.cloudformation.get_waiter(WAITER_CHANGE_SET_CREATE_COMPLETE).wait( | ||
ChangeSetName=change_set_id | ||
) | ||
aws_client.cloudformation.execute_change_set(ChangeSetName=change_set_id) | ||
stack_waiter = aws_client.cloudformation.get_waiter( | ||
cfn_aws_client.cloudformation.execute_change_set(ChangeSetName=change_set_id) | ||
stack_waiter = cfn_aws_client.cloudformation.get_waiter( | ||
WAITER_STACK_UPDATE_COMPLETE if is_update else WAITER_STACK_CREATE_COMPLETE | ||
) | ||
|
||
|
@@ -1029,28 +1028,30 @@ def _deploy( | |
) | ||
except botocore.exceptions.WaiterError as e: | ||
raise StackDeployError( | ||
aws_client.cloudformation.describe_stacks(StackName=stack_id)["Stacks"][0], | ||
aws_client.cloudformation.describe_stack_events(StackName=stack_id)["StackEvents"], | ||
cfn_aws_client.cloudformation.describe_stacks(StackName=stack_id)["Stacks"][0], | ||
cfn_aws_client.cloudformation.describe_stack_events(StackName=stack_id)[ | ||
"StackEvents" | ||
], | ||
) from e | ||
|
||
describe_stack_res = aws_client.cloudformation.describe_stacks(StackName=stack_id)[ | ||
describe_stack_res = cfn_aws_client.cloudformation.describe_stacks(StackName=stack_id)[ | ||
"Stacks" | ||
][0] | ||
outputs = describe_stack_res.get("Outputs", []) | ||
|
||
mapped_outputs = {o["OutputKey"]: o.get("OutputValue") for o in outputs} | ||
|
||
def _destroy_stack(): | ||
aws_client.cloudformation.delete_stack(StackName=stack_id) | ||
aws_client.cloudformation.get_waiter(WAITER_STACK_DELETE_COMPLETE).wait( | ||
cfn_aws_client.cloudformation.delete_stack(StackName=stack_id) | ||
cfn_aws_client.cloudformation.get_waiter(WAITER_STACK_DELETE_COMPLETE).wait( | ||
StackName=stack_id, | ||
WaiterConfig={ | ||
"Delay": delay_between_polls, | ||
"MaxAttempts": max_wait / delay_between_polls, | ||
}, | ||
) | ||
# TODO: fix in localstack. stack should only be in DELETE_COMPLETE state after all resources have been deleted | ||
time.sleep(2) | ||
|
||
state.append((stack_id, _destroy_stack)) | ||
|
||
return DeployResult( | ||
change_set_id, stack_id, stack_name, change_set_name, mapped_outputs, _destroy_stack | ||
|
@@ -1059,20 +1060,11 @@ def _destroy_stack(): | |
yield _deploy | ||
|
||
# delete the stacks in the reverse order they were created in case of inter-stack dependencies | ||
for entry in state[::-1]: | ||
entry_stack_id = entry.get("stack_id") | ||
for stack_id, teardown in state[::-1]: | ||
try: | ||
if entry_stack_id: | ||
aws_client.cloudformation.delete_stack(StackName=entry_stack_id) | ||
aws_client.cloudformation.get_waiter(WAITER_STACK_DELETE_COMPLETE).wait( | ||
StackName=entry_stack_id, | ||
WaiterConfig={ | ||
"Delay": 2, | ||
"MaxAttempts": 120, | ||
}, | ||
) | ||
teardown() | ||
except Exception as e: | ||
LOG.debug(f"Failed cleaning up stack {entry_stack_id=}: {e}") | ||
LOG.debug(f"Failed cleaning up stack {stack_id=}: {e}") | ||
|
||
|
||
@pytest.fixture | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,14 @@ | ||
import base64 | ||
import json | ||
import os | ||
import re | ||
from copy import deepcopy | ||
|
||
import botocore.exceptions | ||
import pytest | ||
import yaml | ||
|
||
from localstack.aws.api.lambda_ import Runtime | ||
from localstack.constants import AWS_REGION_US_EAST_1 | ||
from localstack.services.cloudformation.engine.yaml_parser import parse_yaml | ||
from localstack.testing.aws.cloudformation_utils import load_template_file, load_template_raw | ||
from localstack.testing.pytest import markers | ||
|
@@ -234,25 +234,41 @@ def test_cidr_function(self, deploy_cfn_template): | |
|
||
assert deployed.outputs["Address"] == "10.0.0.0/24" | ||
|
||
@pytest.mark.parametrize( | ||
"region", | ||
[ | ||
"us-east-1", | ||
"us-east-2", | ||
"us-west-1", | ||
"us-west-2", | ||
"ap-southeast-2", | ||
"ap-northeast-1", | ||
"eu-central-1", | ||
"eu-west-1", | ||
], | ||
) | ||
@markers.aws.validated | ||
def test_get_azs_function(self, deploy_cfn_template, snapshot): | ||
def test_get_azs_function(self, deploy_cfn_template, region, aws_client_factory): | ||
""" | ||
TODO parametrize this test. | ||
For that we need to be able to parametrize the client region. The docs show the we should be | ||
able to put any region in the parameters but it doesn't work. It only accepts the same region from the client config | ||
if you put anything else it just returns an empty list. | ||
""" | ||
|
||
template_path = os.path.join( | ||
os.path.dirname(__file__), "../../templates/functions_get_azs.yml" | ||
) | ||
|
||
aws_client = aws_client_factory(region_name=region) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic looks good to me. |
||
deployed = deploy_cfn_template( | ||
template_path=template_path, | ||
custom_aws_client=aws_client, | ||
parameters={"DeployRegion": region}, | ||
) | ||
|
||
snapshot.add_transformer(snapshot.transform.regex(AWS_REGION_US_EAST_1, "<region>")) | ||
snapshot.match("azs", deployed.outputs["Zones"].split(";")) | ||
azs = deployed.outputs["Zones"].split(";") | ||
assert len(azs) > 0 | ||
assert all(re.match(f"{region}[a-f]", az) for az in azs) | ||
|
||
@markers.aws.validated | ||
def test_sub_not_ready(self, deploy_cfn_template): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good with this changes for Kinesis.
We should probably check in future, if we are missing clean up for any other service