From 7d0800acdf287c249b2ff8aa60bcf12865e0c2d5 Mon Sep 17 00:00:00 2001 From: Khoi Pham <132031702+dkphm@users.noreply.github.com> Date: Sun, 1 Sep 2024 08:19:37 -0700 Subject: [PATCH] fix: Race conditions with multiple downloads on the same layer (#7422) * fix: Race conditions with multiple downloads on the same layer * Update format * Fix lint * Using uuid for download filename --- samcli/local/layers/layer_downloader.py | 3 +- .../local/start_api/test_start_api.py | 14 +++ ...template-warm-containers-multi-layers.yaml | 88 +++++++++++++++++++ .../unit/local/layers/test_download_layers.py | 6 +- 4 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 tests/integration/testdata/start_api/template-warm-containers-multi-layers.yaml diff --git a/samcli/local/layers/layer_downloader.py b/samcli/local/layers/layer_downloader.py index a600a032fe..aff6f84da0 100644 --- a/samcli/local/layers/layer_downloader.py +++ b/samcli/local/layers/layer_downloader.py @@ -3,6 +3,7 @@ """ import logging +import uuid from pathlib import Path from typing import List @@ -106,7 +107,7 @@ def download(self, layer: LayerVersion, force=False) -> LayerVersion: LOG.info("%s is already cached. Skipping download", layer.arn) return layer - layer_zip_path = layer.codeuri + ".zip" + layer_zip_path = f"{layer.codeuri}_{uuid.uuid4().hex}.zip" layer_zip_uri = self._fetch_layer_uri(layer) unzip_from_uri( layer_zip_uri, diff --git a/tests/integration/local/start_api/test_start_api.py b/tests/integration/local/start_api/test_start_api.py index 9b96310585..f0d75e5dc5 100644 --- a/tests/integration/local/start_api/test_start_api.py +++ b/tests/integration/local/start_api/test_start_api.py @@ -3220,6 +3220,20 @@ def test_can_invoke_lambda_layer_successfully(self): self.assertEqual(response.content.decode("utf-8"), '"Layer1"') +class TestWarmContainersMultipleRemoteLayersInvoke(WarmContainersWithRemoteLayersBase): + template_path = "/testdata/start_api/template-warm-containers-multi-layers.yaml" + container_mode = ContainersInitializationMode.EAGER.value + mode_env_variable = str(uuid.uuid4()) + parameter_overrides = {"ModeEnvVariable": mode_env_variable} + + @pytest.mark.flaky(reruns=3) + @pytest.mark.timeout(timeout=600, method="thread") + def test_can_invoke_lambda_layer_successfully(self): + response = requests.get(self.url + "/", timeout=300) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.content.decode("utf-8"), '"Layer1"') + + class TestDisableAuthorizer(StartApiIntegBaseClass): # integration test for scenario: 'sam local start-api --disable-authorizer' template_path = "/testdata/start_api/lambda_authorizers/serverless-api-props.yaml" diff --git a/tests/integration/testdata/start_api/template-warm-containers-multi-layers.yaml b/tests/integration/testdata/start_api/template-warm-containers-multi-layers.yaml new file mode 100644 index 0000000000..24f743b535 --- /dev/null +++ b/tests/integration/testdata/start_api/template-warm-containers-multi-layers.yaml @@ -0,0 +1,88 @@ +AWSTemplateFormatVersion : '2010-09-09' +Transform: AWS::Serverless-2016-10-31 + +Parameters: + ModeEnvVariable: + Type: String + LayerArn: + Default: arn:aws:lambda:us-west-2:111111111111:layer:layer:1 + Type: String + +Resources: + ApiGatewayApi: + Type: AWS::Serverless::Api + Properties: + StageName: prod + CacheClusterEnabled: true + CacheClusterSize: '0.5' + MethodSettings: + - ResourcePath: / + HttpMethod: GET + CachingEnabled: true + CacheTtlInSeconds: 300 + HelloWorldFunction: + Type: AWS::Serverless::Function + Properties: + Handler: main-layers.custom_layer_handler + Runtime: python3.9 + FunctionName: customname + CodeUri: . + Timeout: 600 + Environment: + Variables: + MODE: !Ref ModeEnvVariable + Layers: + # Test remote layers with warm containers. + - Ref: LayerArn + Events: + ApiEvent: + Type: Api + Properties: + Path: / + Method: get + RestApiId: + Ref: ApiGatewayApi + HelloWorldFunction2: + Type: AWS::Serverless::Function + Properties: + Handler: main-layers.custom_layer_handler + Runtime: python3.9 + FunctionName: customname + CodeUri: . + Timeout: 600 + Environment: + Variables: + MODE: !Ref ModeEnvVariable + Layers: + # Test remote layers with warm containers. + - Ref: LayerArn + Events: + ApiEvent: + Type: Api + Properties: + Path: / + Method: get + RestApiId: + Ref: ApiGatewayApi + HelloWorldFunction3: + Type: AWS::Serverless::Function + Properties: + Handler: main-layers.custom_layer_handler + Runtime: python3.9 + FunctionName: customname + CodeUri: . + Timeout: 600 + Environment: + Variables: + MODE: !Ref ModeEnvVariable + Layers: + # Test remote layers with warm containers. + - Ref: LayerArn + Events: + ApiEvent: + Type: Api + Properties: + Path: / + Method: get + RestApiId: + Ref: ApiGatewayApi diff --git a/tests/unit/local/layers/test_download_layers.py b/tests/unit/local/layers/test_download_layers.py index f769e57257..2ab2e60f26 100644 --- a/tests/unit/local/layers/test_download_layers.py +++ b/tests/unit/local/layers/test_download_layers.py @@ -98,6 +98,10 @@ def test_download_layer_that_was_template_defined(self, create_cache_patch, reso def test_download_layer( self, is_layer_cached_patch, create_cache_patch, fetch_layer_uri_patch, unzip_from_uri_patch ): + class AnyStringWith(str): + def __eq__(self, other): + return self in other + is_layer_cached_patch.return_value = False download_layers = LayerDownloader("/home", ".", Mock()) @@ -118,7 +122,7 @@ def test_download_layer( fetch_layer_uri_patch.assert_called_once_with(layer_mock) unzip_from_uri_patch.assert_called_once_with( "layer/uri", - str(Path("/home/layer1.zip").resolve()), + AnyStringWith(str(Path("/home/layer1_"))), unzip_output_dir=str(Path("/home/layer1").resolve()), progressbar_label="Downloading arn:layer:layer1", )