Skip to content

Commit 6daf706

Browse files
marekaivmarekaiv
authored
Do not abort SAR loop on throttling (#2240)
* Do not abort SAR loop on throttling * Address code review comments * Simplify SAR polling loop and add test coverage
1 parent 323312a commit 6daf706

File tree

2 files changed

+132
-49
lines changed

2 files changed

+132
-49
lines changed

‎samtranslator/plugins/application/serverless_app_plugin.py‎

Lines changed: 58 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ def _handle_create_cfn_template_request(self, app_id, semver, key, logical_id):
195195
ApplicationId=self._sanitize_sar_str_param(app_id), SemanticVersion=self._sanitize_sar_str_param(semver)
196196
)
197197
response = self._sar_service_call(create_cfn_template, logical_id, app_id, semver)
198+
198199
LOG.info("Requested to create CFN template {}/{} in serverless application repo.".format(app_id, semver))
199200
self._applications[key] = response[self.TEMPLATE_URL_KEY]
200201
if response["Status"] != "ACTIVE":
@@ -303,57 +304,73 @@ def on_after_transform_template(self, template):
303304
:param dict template: Dictionary of the SAM template
304305
:return: Nothing
305306
"""
306-
if self._wait_for_template_active_status and not self._validate_only:
307-
start_time = time()
308-
while (time() - start_time) < self.TEMPLATE_WAIT_TIMEOUT_SECONDS:
309-
temp = self._in_progress_templates
310-
self._in_progress_templates = []
311-
312-
# Check each resource to make sure it's active
313-
LOG.info("Checking resources in serverless application repo...")
314-
for application_id, template_id in temp:
315-
get_cfn_template = (
316-
lambda application_id, template_id: self._sar_client.get_cloud_formation_template(
317-
ApplicationId=self._sanitize_sar_str_param(application_id),
318-
TemplateId=self._sanitize_sar_str_param(template_id),
319-
)
320-
)
321-
response = self._sar_service_call(get_cfn_template, application_id, application_id, template_id)
322-
self._handle_get_cfn_template_response(response, application_id, template_id)
323-
LOG.info("Finished checking resources in serverless application repo.")
307+
if not self._wait_for_template_active_status or self._validate_only:
308+
return
324309

325-
# Don't sleep if there are no more templates with PREPARING status
326-
if len(self._in_progress_templates) == 0:
327-
break
310+
start_time = time()
311+
while (time() - start_time) < self.TEMPLATE_WAIT_TIMEOUT_SECONDS:
312+
# Check each resource to make sure it's active
313+
LOG.info("Checking resources in serverless application repo...")
314+
idx = 0
315+
while idx < len(self._in_progress_templates):
316+
application_id, template_id = self._in_progress_templates[idx]
317+
get_cfn_template = lambda application_id, template_id: self._sar_client.get_cloud_formation_template(
318+
ApplicationId=self._sanitize_sar_str_param(application_id),
319+
TemplateId=self._sanitize_sar_str_param(template_id),
320+
)
328321

329-
# Sleep a little so we don't spam service calls
330-
sleep(self.SLEEP_TIME_SECONDS)
322+
try:
323+
response = self._sar_service_call(get_cfn_template, application_id, application_id, template_id)
324+
except ClientError as e:
325+
error_code = e.response["Error"]["Code"]
326+
if error_code == "TooManyRequestsException":
327+
LOG.debug("SAR call timed out for application id {}".format(application_id))
328+
break # We were throttled by SAR, break out to a sleep
329+
else:
330+
raise e
331+
332+
if self._is_template_active(response, application_id, template_id):
333+
self._in_progress_templates.remove((application_id, template_id))
334+
else:
335+
idx += 1 # check next template
336+
337+
LOG.info("Finished checking resources in serverless application repo.")
338+
339+
# Don't sleep if there are no more templates with PREPARING status
340+
if len(self._in_progress_templates) == 0:
341+
break
342+
343+
# Sleep a little so we don't spam service calls
344+
sleep(self._get_sleep_time_sec())
345+
346+
# Not all templates reached active status
347+
if len(self._in_progress_templates) != 0:
348+
application_ids = [items[0] for items in self._in_progress_templates]
349+
raise InvalidResourceException(
350+
application_ids, "Timed out waiting for nested stack templates " "to reach ACTIVE status."
351+
)
331352

332-
# Not all templates reached active status
333-
if len(self._in_progress_templates) != 0:
334-
application_ids = [items[0] for items in self._in_progress_templates]
335-
raise InvalidResourceException(
336-
application_ids, "Timed out waiting for nested stack templates " "to reach ACTIVE status."
337-
)
353+
def _get_sleep_time_sec(self):
354+
return self.SLEEP_TIME_SECONDS
338355

339-
def _handle_get_cfn_template_response(self, response, application_id, template_id):
356+
def _is_template_active(self, response, application_id, template_id):
340357
"""
341-
Handles the response from the SAR service call
358+
Checks the response from a SAR service call; returns True if the template is active,
359+
throws an exception if the request expired and returns False in all other cases.
342360
343361
:param dict response: the response dictionary from the app repo
344362
:param string application_id: the ApplicationId
345363
:param string template_id: the unique TemplateId for this application
346364
"""
347-
status = response["Status"]
348-
if status != "ACTIVE":
349-
# Other options are PREPARING and EXPIRED.
350-
if status == "EXPIRED":
351-
message = (
352-
"Template for {} with id {} returned status: {}. Cannot access an expired "
353-
"template.".format(application_id, template_id, status)
354-
)
355-
raise InvalidResourceException(application_id, message)
356-
self._in_progress_templates.append((application_id, template_id))
365+
status = response["Status"] # options: PREPARING, EXPIRED or ACTIVE
366+
367+
if status == "EXPIRED":
368+
message = "Template for {} with id {} returned status: {}. Cannot access an expired " "template.".format(
369+
application_id, template_id, status
370+
)
371+
raise InvalidResourceException(application_id, message)
372+
373+
return status == "ACTIVE"
357374

358375
@cw_timer(prefix="External", name="SAR")
359376
def _sar_service_call(self, service_call_lambda, logical_id, *args):
@@ -372,9 +389,6 @@ def _sar_service_call(self, service_call_lambda, logical_id, *args):
372389
error_code = e.response["Error"]["Code"]
373390
if error_code in ("AccessDeniedException", "NotFoundException"):
374391
raise InvalidResourceException(logical_id, e.response["Error"]["Message"])
375-
376-
# 'ForbiddenException'- SAR rejects connection
377-
LOG.exception(e)
378392
raise e
379393

380394
def _resource_is_supported(self, resource_type):

‎tests/plugins/application/test_serverless_app_plugin.py‎

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import boto3
22
import itertools
3+
from botocore.exceptions import ClientError
34

45
from mock import Mock, patch
56
from unittest import TestCase
67
from parameterized import parameterized, param
78

89
from samtranslator.plugins.application.serverless_app_plugin import ServerlessAppPlugin
910
from samtranslator.plugins.exceptions import InvalidPluginException
11+
from samtranslator.model.exceptions import InvalidResourceException
1012

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

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

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

256-
# class TestServerlessAppPlugin_on_after_transform_template(TestCase):
257258

258-
# def setUp(self):
259-
# self.plugin = SeverlessAppPlugin()
260-
261-
# # TODO: test this lifecycle event
259+
class TestServerlessAppPlugin_on_after_transform_template(TestCase):
260+
def test_sar_throttling_doesnt_stop_processing(self):
261+
client = Mock()
262+
client.get_cloud_formation_template = Mock()
263+
client.get_cloud_formation_template.side_effect = ClientError(
264+
{"Error": {"Code": "TooManyRequestsException"}}, "GetCloudFormationTemplate"
265+
)
266+
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
267+
plugin._get_sleep_time_sec = Mock()
268+
plugin._get_sleep_time_sec.return_value = 0.02
269+
plugin._in_progress_templates = [("appid", "template"), ("appid2", "template2")]
270+
plugin.TEMPLATE_WAIT_TIMEOUT_SECONDS = 0.2
271+
with self.assertRaises(InvalidResourceException):
272+
plugin.on_after_transform_template("template")
273+
# confirm we had at least two attempts to call SAR and that we executed a sleep
274+
self.assertGreater(client.get_cloud_formation_template.call_count, 1)
275+
self.assertGreaterEqual(plugin._get_sleep_time_sec.call_count, 1)
276+
277+
def test_unexpected_sar_error_stops_processing(self):
278+
client = Mock()
279+
client.get_cloud_formation_template = Mock()
280+
client.get_cloud_formation_template.side_effect = ClientError(
281+
{"Error": {"Code": "BadBadError"}}, "GetCloudFormationTemplate"
282+
)
283+
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
284+
plugin._in_progress_templates = [("appid", "template")]
285+
with self.assertRaises(ClientError):
286+
plugin.on_after_transform_template("template")
287+
288+
def test_sar_success_one_app(self):
289+
client = Mock()
290+
client.get_cloud_formation_template = Mock()
291+
client.get_cloud_formation_template.return_value = {"Status": STATUS_ACTIVE}
292+
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
293+
plugin._in_progress_templates = [("appid", "template")]
294+
plugin.on_after_transform_template("template")
295+
# should have exactly one call to SAR
296+
self.assertEqual(client.get_cloud_formation_template.call_count, 1)
297+
298+
def test_sar_success_two_apps(self):
299+
client = Mock()
300+
client.get_cloud_formation_template = Mock()
301+
client.get_cloud_formation_template.return_value = {"Status": STATUS_ACTIVE}
302+
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
303+
plugin._in_progress_templates = [("appid1", "template1"), ("appid2", "template2")]
304+
plugin.on_after_transform_template("template")
305+
# should have exactly one call to SAR per app
306+
self.assertEqual(client.get_cloud_formation_template.call_count, 2)
307+
308+
def test_expired_sar_app_throws(self):
309+
client = Mock()
310+
client.get_cloud_formation_template = Mock()
311+
client.get_cloud_formation_template.return_value = {"Status": STATUS_EXPIRED}
312+
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
313+
plugin._in_progress_templates = [("appid1", "template1"), ("appid2", "template2")]
314+
with self.assertRaises(InvalidResourceException):
315+
plugin.on_after_transform_template("template")
316+
# should have exactly one call to SAR since the first app will be expired
317+
self.assertEqual(client.get_cloud_formation_template.call_count, 1)
318+
319+
def test_sleep_between_sar_checks(self):
320+
client = Mock()
321+
client.get_cloud_formation_template = Mock()
322+
client.get_cloud_formation_template.side_effect = [{"Status": STATUS_PREPARING}, {"Status": STATUS_ACTIVE}]
323+
plugin = ServerlessAppPlugin(sar_client=client, wait_for_template_active_status=True, validate_only=False)
324+
plugin._in_progress_templates = [("appid1", "template1")]
325+
plugin._get_sleep_time_sec = Mock()
326+
plugin._get_sleep_time_sec.return_value = 0.001
327+
plugin.on_after_transform_template("template")
328+
# should have exactly two calls to SAR
329+
self.assertEqual(client.get_cloud_formation_template.call_count, 2)
330+
self.assertEqual(plugin._get_sleep_time_sec.call_count, 1) # make sure we slept once

0 commit comments

Comments
 (0)