feat(assets): support platform flag for DockerImageAsset #16770
feat(assets): support platform flag for DockerImageAsset #16770pahud wants to merge 29 commits intoaws:masterfrom pahud:pahud/assets-allow-user-to-pass-12472
Conversation
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
|
|
||
| new DockerImageFunction(this, 'MyLambda', { | ||
| code: DockerImageCode.fromImageAsset(path.join(__dirname, 'docker-arm64-handler'), { | ||
| platform: DockerPlatform.ARM_64, |
There was a problem hiding this comment.
I wonder if there's a way to automatically set the platform based on the value of architectures.
Maybe pass props.architectures to _bind() here?
aws-cdk/packages/@aws-cdk/aws-lambda/lib/image-function.ts
Lines 60 to 69 in 1deea90
then you can set the platform in fromImageAsset?
There was a problem hiding this comment.
Hi @jogold,
This should have been addressed. Can you take a look again?
I think we still need an unit test to spyOn some method being called with correct platform. I've checked your packages/@aws-cdk/aws-lambda-go/test/bundling.test.ts and have been trying to write a similar test but ended up with no luck. Could you advise how to write the unit test for this PR? Any code snippets would be appreciated.
…b.com/pahud/aws-cdk into pahud/assets-allow-user-to-pass-12472
|
Hi @jogold can you take a look again? |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Hi @pahud this PR seems to be a bit out of date, is this still being worked on? |
| * Use this if the platform name is not yet supported by the CDK. | ||
| * @param [platform=linux/amd64] the platform to use for docker build. | ||
| */ | ||
| public static custom(platform?: string) { |
There was a problem hiding this comment.
I don't think it makes sense to provide a default value here for platform. DockerPlatform.custom() doesn't make semantic sense...
| public static custom(platform?: string) { | |
| public static custom(platform: string) { |
| * Specify this property to build images for a specific platform. Support docker API 1.38+. | ||
| * | ||
| * @default - no specific platform | ||
| * |
| configurations. See their docs for more information. | ||
|
|
||
| To deploy a `DockerImageFunction` on Lambda `arm64` architecture, make sure you specify `Architecture.ARM_64` in `architecture`. | ||
| This will bundle docker image assets for `arm64` architecture with `--platform linux/arm64` implicitly even you build it from a `x86_64` architecture. |
There was a problem hiding this comment.
| This will bundle docker image assets for `arm64` architecture with `--platform linux/arm64` implicitly even you build it from a `x86_64` architecture. | |
| This will bundle docker image assets for `arm64` architecture with `--platform linux/arm64` implicitly even you build it within an `x86_64` host. |
| /** | ||
| * determine the platform from `architecture`. | ||
| */ |
There was a problem hiding this comment.
This is not a docstring:
| /** | |
| * determine the platform from `architecture`. | |
| */ | |
| // determine the platform from `architecture`. |
|
@pahud I wouldn't mind if you could address the changes requested by Elad – I want to see this get merged 😄 |
Sorry for the delay. Yes I will address this shortly and continue this PR. Thanks!! |
| /** | ||
| * determine the platform from `architecture`. | ||
| */ | ||
| platform: arch?.dockerPlatform ? DockerPlatform.custom(arch.dockerPlatform) : undefined, |
There was a problem hiding this comment.
Suggestion: align the style like this
| platform: arch?.dockerPlatform ? DockerPlatform.custom(arch.dockerPlatform) : undefined, | |
| ...arch?.dockerPlatform ? { platform: DockerPlatform.custom(arch.dockerPlatform) } : {}, |
|
@pahud what's the status here? |
|
This PR has been in CHANGES REQUESTED for 21 days, and looks abandoned. It will be closed in 10 days if no further commits are pushed to it. |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
|
Will this PR be revived? |
|
@pahud @ericzbeard - Curious if this one is going to be revived? It looks to be close and I was hoping to use this feature in a demo I was building. |
…ker images (#20439) This PR adds support for specifying the desired build platform when building docker images (ie: build an arm64 image on an amd64/x86_64 host). Closes #12472 This PR does NOT touch Lambda builders, only ECR assets. #16770 attempted to implement support for ECR and Lambda but was abandoned. Meanwhile #16858 implemented lambda platform support. This implements the ECR side I have run `yarn integ` ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Based on [this](#16770) PR Add the missing part to add platform support when using lambda `fromImageAsset` As we are not allowed to specify `platform` flag for `DockerImageAsset`, users deploying cdk on x86_64 platform will not be able to bundle lambda.DockerImageFunction for the new arm64 architecture. Similarly, users deploying cdk on arm64 architecture like Mac M1 will not be able to bundle images for AWS Fargate, which is x86_64 only. # builder experience with aws-lambda For x86_64 users deploying Lambda functions with container runtime on Lambda Graviton2(arm64) from local container image assets with multi-architecture docker base images. Specify the platform to ensure the image would be built for ARM64 architecture. ``` new DockerImageFunction(stack, 'Lambda', { code: DockerImageCode.fromImageAsset(path.join(__dirname, 'docker-arm64-handler')), architecture: Architecture.ARM_64, }); ``` Fixes: #12472, #20907 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
As we are not allowed to specify
platformflag forDockerImageAsset, users deploying cdk on x86_64 platform will not be able to bundlelambda.DockerImageFunctionfor the new arm64 architecture. Similarly, users deploying cdk on arm64 architecture like Mac M1 will not be able to bundle images for AWS Fargate, which is x86_64 only.With this support, we are allowed to:
platformproperty ofDockerImageAssetfromaws-ecr-assets.DockerImageFunctionfromaws-lambdaon x86_64.aws-ecson arm64.Fixes: #12472
Background
When we bundle and publish the docker container image assets with
cdk deploy, it's important we must build images of correct architectures for different services. For AWS Fargate, as it only supports x86_64 at this moment, we must build x86_64(amd64) architecture docker images, while for Lambda Graviton2, thearm64would be required.Architectureof the imagesRun
docker inspectto check theArchitecturefor local images:For example:
(The image was built for
arm64platform)How to build images with correct
Architecture?Many docker image registries come with multi-architecture support. Read Introducing multi-architecture container images for Amazon ECR for more details on Amazon ECR. Let's take
public.ecr.aws/lambda/python:latestfor example. Rundocker manifest inspectto check its manifest for architectures:docker manifest inspect public.ecr.aws/lambda/python:latest { "schemaVersion": 2, "mediaType": "application/vnd.docker.distribution.manifest.list.v2+json", "manifests": [ { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "size": 1580, "digest": "sha256:fc52f9b4ca941653c109fca35078fb3d589283f61e37ab88bed05101c607de7a", "platform": { "architecture": "arm64", "os": "linux", "variant": "v8" } }, { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "size": 1580, "digest": "sha256:68ffef9d11b6de8675b657ecfb5eadf152150c720e627fb9b5ea0c69ef6ac8d0", "platform": { "architecture": "amd64", "os": "linux" } } ] }This image supports both
linux/arm64andlinux/amd64from the manifest.Similarly, if we check
python:3.9from docker hub, we can list all platforms/architectures supported from its manifest data.So how can we ensure to build correct architecture from different desktops like Apple Mac M1 or any x86_64 desktops? Let's take the following Dockerfile for example:
FROM public.ecr.aws/lambda/python:latestFor Apple Mac M1 users, if we
docker buildwith this Dockerfile, under the covers the image layers forarm64will be pulled because the Apple Mac M1 isarm64architecture unless:DOCKER_DEFAULT_PLATFORMenv var(see (assets): allow user to pass platform option to Docker builds #12472 (comment))--platformin the Dockerfile likedocker buildwith--platformflagLet's take M1 for example, if we
docker buildwithout passing--platform, we'll build anarm64image.However, if we pass
--platform='linux/amd64'we'll be able to buildamd64image from exactly the same Dockerfile.Similarly, for
x86_64desktop users, we'll buildamd64images by default andarm64images by passing--platform='linux/arm64'.When we bundle container image assets for AWS Fargate, we must build
amd64images. For Lambda arm64 architecture, we must buildarm64images for Lambda container runtime, no matter which platform you synthesize and deploy the assets. The key is always to pass correct--platformflag with Dockerfile from a base image with multiple-platform support.However, what if we use
python:latest-arm64as the base image for ourDockerfile? Well, we will lose the advantages of multi-architecture support as this will only build arm64 images.builder experience with
aws-ecr-assetsSpecify
platformwithARM_64orAMD_64from docker image assets with multi-architecture docker base imagesbuilder experience with
aws-ecsFor Mac M1 users deploying Amazon ECS services on AWS Fargate from local container image assets with multi-architecture docker base images. Specify the
platformto ensure the image would be built forAMD_64architecture.builder experience with
aws-lambdaFor
x86_64users deploying Lambda functions with container runtime on Lambda Graviton2(arm64) from local container image assets with multi-architecture docker base images. Specify theplatformto ensure the image would be built forARM64architecture.builder experience with
aws-apprunnerAWS App Runner supports
x86_64architecture only at this moment. For Mac M1 users, useplatformto ensure theAMD_64architecture for theDockerImageAsset.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license