Skip to content

Do not abort SAR loop on throttling #2240

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 14, 2021
Merged

Do not abort SAR loop on throttling #2240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2a2af2
Do not abort SAR loop on throttling
marekaiv Nov 23, 2021
54a7c51
Address code review comments
marekaiv Nov 29, 2021
292e9f4
Simplify SAR polling loop and add test coverage
marekaiv Dec 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 58 additions & 44 deletions samtranslator/plugins/application/serverless_app_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ def _handle_create_cfn_template_request(self, app_id, semver, key, logical_id):
ApplicationId=self._sanitize_sar_str_param(app_id), SemanticVersion=self._sanitize_sar_str_param(semver)
)
response = self._sar_service_call(create_cfn_template, logical_id, app_id, semver)

LOG.info("Requested to create CFN template {}/{} in serverless application repo.".format(app_id, semver))
self._applications[key] = response[self.TEMPLATE_URL_KEY]
if response["Status"] != "ACTIVE":
Expand Down Expand Up @@ -303,57 +304,73 @@ def on_after_transform_template(self, template):
:param dict template: Dictionary of the SAM template
:return: Nothing
"""
if self._wait_for_template_active_status and not self._validate_only:
start_time = time()
while (time() - start_time) < self.TEMPLATE_WAIT_TIMEOUT_SECONDS:
temp = self._in_progress_templates
self._in_progress_templates = []

# Check each resource to make sure it's active
LOG.info("Checking resources in serverless application repo...")
for application_id, template_id in temp:
get_cfn_template = (
lambda application_id, template_id: self._sar_client.get_cloud_formation_template(
ApplicationId=self._sanitize_sar_str_param(application_id),
TemplateId=self._sanitize_sar_str_param(template_id),
)
)
response = self._sar_service_call(get_cfn_template, application_id, application_id, template_id)
self._handle_get_cfn_template_response(response, application_id, template_id)
LOG.info("Finished checking resources in serverless application repo.")
if not self._wait_for_template_active_status or self._validate_only:
return

# Don't sleep if there are no more templates with PREPARING status
if len(self._in_progress_templates) == 0:
break
start_time = time()
while (time() - start_time) < self.TEMPLATE_WAIT_TIMEOUT_SECONDS:
# Check each resource to make sure it's active
LOG.info("Checking resources in serverless application repo...")
idx = 0
while idx < len(self._in_progress_templates):
application_id, template_id = self._in_progress_templates[idx]
get_cfn_template = lambda application_id, template_id: self._sar_client.get_cloud_formation_template(
ApplicationId=self._sanitize_sar_str_param(application_id),
TemplateId=self._sanitize_sar_str_param(template_id),
)

# Sleep a little so we don't spam service calls
sleep(self.SLEEP_TIME_SECONDS)
try:
response = self._sar_service_call(get_cfn_template, application_id, application_id, template_id)
except ClientError as e:
error_code = e.response["Error"]["Code"]
if error_code == "TooManyRequestsException":
LOG.debug("SAR call timed out for application id {}".format(application_id))
break # We were throttled by SAR, break out to a sleep
else:
raise e

if self._is_template_active(response, application_id, template_id):
self._in_progress_templates.remove((application_id, template_id))
else:
idx += 1 # check next template

LOG.info("Finished checking resources in serverless application repo.")

# Don't sleep if there are no more templates with PREPARING status
if len(self._in_progress_templates) == 0:
break

# Sleep a little so we don't spam service calls
sleep(self._get_sleep_time_sec())

# Not all templates reached active status
if len(self._in_progress_templates) != 0:
application_ids = [items[0] for items in self._in_progress_templates]
raise InvalidResourceException(
application_ids, "Timed out waiting for nested stack templates " "to reach ACTIVE status."
)

# Not all templates reached active status
if len(self._in_progress_templates) != 0:
application_ids = [items[0] for items in self._in_progress_templates]
raise InvalidResourceException(
application_ids, "Timed out waiting for nested stack templates " "to reach ACTIVE status."
)
def _get_sleep_time_sec(self):
return self.SLEEP_TIME_SECONDS

def _handle_get_cfn_template_response(self, response, application_id, template_id):
def _is_template_active(self, response, application_id, template_id):
"""
Handles the response from the SAR service call
Checks the response from a SAR service call; returns True if the template is active,
throws an exception if the request expired and returns False in all other cases.

:param dict response: the response dictionary from the app repo
:param string application_id: the ApplicationId
:param string template_id: the unique TemplateId for this application
"""
status = response["Status"]
if status != "ACTIVE":
# Other options are PREPARING and EXPIRED.
if status == "EXPIRED":
message = (
"Template for {} with id {} returned status: {}. Cannot access an expired "
"template.".format(application_id, template_id, status)
)
raise InvalidResourceException(application_id, message)
self._in_progress_templates.append((application_id, template_id))
Copy link
Contributor

Choose a reason for hiding this comment

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

this one was hidden here, I like the new change

status = response["Status"] # options: PREPARING, EXPIRED or ACTIVE

if status == "EXPIRED":
message = "Template for {} with id {} returned status: {}. Cannot access an expired " "template.".format(
application_id, template_id, status
)
raise InvalidResourceException(application_id, message)

return status == "ACTIVE"

@cw_timer(prefix="External", name="SAR")
def _sar_service_call(self, service_call_lambda, logical_id, *args):
Expand All @@ -372,9 +389,6 @@ def _sar_service_call(self, service_call_lambda, logical_id, *args):
error_code = e.response["Error"]["Code"]
if error_code in ("AccessDeniedException", "NotFoundException"):
raise InvalidResourceException(logical_id, e.response["Error"]["Message"])

# 'ForbiddenException'- SAR rejects connection
LOG.exception(e)
raise e

def _resource_is_supported(self, resource_type):
Expand Down
79 changes: 74 additions & 5 deletions tests/plugins/application/test_serverless_app_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import boto3
import itertools
from botocore.exceptions import ClientError

from mock import Mock, patch
from unittest import TestCase
from parameterized import parameterized, param

from samtranslator.plugins.application.serverless_app_plugin import ServerlessAppPlugin
from samtranslator.plugins.exceptions import InvalidPluginException
from samtranslator.model.exceptions import InvalidResourceException

# TODO: run tests when AWS CLI is not configured (so they can run in brazil)

Expand Down Expand Up @@ -253,9 +255,76 @@ def __init__(self, app_id="app_id", semver="1.3.5"):

# self.plugin.on_before_transform_resource(app_resources[0][0], 'AWS::Serverless::Application', app_resources[0][1].properties)

# class TestServerlessAppPlugin_on_after_transform_template(TestCase):

# def setUp(self):
# self.plugin = SeverlessAppPlugin()

# # TODO: test this lifecycle event
class TestServerlessAppPlugin_on_after_transform_template(TestCase):
def test_sar_throttling_doesnt_stop_processing(self):
client = Mock()
client.get_cloud_formation_template = Mock()
client.get_cloud_formation_template.side_effect = ClientError(
{"Error": {"Code": "TooManyRequestsException"}}, "GetCloudFormationTemplate"
)
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
plugin._get_sleep_time_sec = Mock()
plugin._get_sleep_time_sec.return_value = 0.02
plugin._in_progress_templates = [("appid", "template"), ("appid2", "template2")]
plugin.TEMPLATE_WAIT_TIMEOUT_SECONDS = 0.2
with self.assertRaises(InvalidResourceException):
plugin.on_after_transform_template("template")
# confirm we had at least two attempts to call SAR and that we executed a sleep
self.assertGreater(client.get_cloud_formation_template.call_count, 1)
self.assertGreaterEqual(plugin._get_sleep_time_sec.call_count, 1)

def test_unexpected_sar_error_stops_processing(self):
client = Mock()
client.get_cloud_formation_template = Mock()
client.get_cloud_formation_template.side_effect = ClientError(
{"Error": {"Code": "BadBadError"}}, "GetCloudFormationTemplate"
)
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
plugin._in_progress_templates = [("appid", "template")]
with self.assertRaises(ClientError):
plugin.on_after_transform_template("template")

def test_sar_success_one_app(self):
client = Mock()
client.get_cloud_formation_template = Mock()
client.get_cloud_formation_template.return_value = {"Status": STATUS_ACTIVE}
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
plugin._in_progress_templates = [("appid", "template")]
plugin.on_after_transform_template("template")
# should have exactly one call to SAR
self.assertEqual(client.get_cloud_formation_template.call_count, 1)

def test_sar_success_two_apps(self):
client = Mock()
client.get_cloud_formation_template = Mock()
client.get_cloud_formation_template.return_value = {"Status": STATUS_ACTIVE}
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
plugin._in_progress_templates = [("appid1", "template1"), ("appid2", "template2")]
plugin.on_after_transform_template("template")
# should have exactly one call to SAR per app
self.assertEqual(client.get_cloud_formation_template.call_count, 2)

def test_expired_sar_app_throws(self):
client = Mock()
client.get_cloud_formation_template = Mock()
client.get_cloud_formation_template.return_value = {"Status": STATUS_EXPIRED}
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
plugin._in_progress_templates = [("appid1", "template1"), ("appid2", "template2")]
with self.assertRaises(InvalidResourceException):
plugin.on_after_transform_template("template")
# should have exactly one call to SAR since the first app will be expired
self.assertEqual(client.get_cloud_formation_template.call_count, 1)

def test_sleep_between_sar_checks(self):
client = Mock()
client.get_cloud_formation_template = Mock()
client.get_cloud_formation_template.side_effect = [{"Status": STATUS_PREPARING}, {"Status": STATUS_ACTIVE}]
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
plugin._in_progress_templates = [("appid1", "template1")]
plugin._get_sleep_time_sec = Mock()
plugin._get_sleep_time_sec.return_value = 0.001
plugin.on_after_transform_template("template")
# should have exactly two calls to SAR
self.assertEqual(client.get_cloud_formation_template.call_count, 2)
self.assertEqual(plugin._get_sleep_time_sec.call_count, 1) # make sure we slept once