feat(codebuild): improved support for ARM build images #19052

tmmvn · 2022-02-19T00:01:52Z

Motivation

CDK currently has poor and hidden support for using ARM build images for CodeBuild that do not match what you can do with the Console. Currently, CDK has under LinuxBuildImage two constants not mentioned in the documentation. The constants internally map to a hidden ArmBuildImage class, which provides support for the standard CodeBuild ARM build images. That is the extent of the support, making ARM a second class citizen compared to x86-64 Linux and Windows build images as, for example, you can't use custom aarch64 ECR images.

Changes

This pull request addresses the missing support by:

renaming the previously hidden class ArmBuildImage to LinuxArmBuildImage (in case there are Windows ARM Build Images in the future).
exporting LinuxArmBuildImage so it can be used.
adding the two ARM constants present in LinuxBuildImage also to LinuxArmBuildImage. The constants are also left under LinuxBuildImage to not break backwards compatibility.
adding the method fromEcrRepository() to support custom ARM build images.
making the LinuxArmBuildImage closer to the LinuxBuildImage and WindowsBuildImage (built with props instead of just image name).
updating documentation to show examples of ARM and highlighting the LinuxBuildImage is for x86-64.

Testing

The unit test for ARM image is still valid.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…y supported by CDK by: - renaming the previously hidden class ArmBuildImage to LinuxArmBuildImage (in case there are Windows ARM Build Images in the future). - exporting LinuxArmBuildImage so it can be used. - adding the two ARM constants present in LinuxBuildImage also to LinuxArmBuildImage. - adding the method fromEcrRepository to support custom ARM build images. - making the LinuxArmBuildImage more akin to the LinuxBuildImage and WindowsBuildImage (built with props instead of just image name). - updating documentation to show examples of ARM and highlighting the LinuxBuildImage is for x86-64.

gitpod-io · 2022-02-19T00:01:54Z

skinny85 · 2022-02-19T00:43:04Z

@codemeddler any chance of adding a few unit tests? 🙂

tmmvn · 2022-02-19T15:48:49Z

@codemeddler any chance of adding a few unit tests? 🙂

Sure if you can point me where to add them and which ones you want. I saw the GPU ones had their own file, the codebuild.test has ARM image testing in it, and only one for the fromEcrRepository is in integ.ecr.lit.

skinny85 · 2022-02-21T19:39:37Z

Let's create a new file, linux-arm-build-image.test.ts, and model it after the existing linux-gpu-build-image.test.ts file.

tmmvn · 2022-02-23T16:34:24Z

@skinny85 Does it matter which values are used for the image repositories in the tests? I can see the public ECR registry has many foobar repositories, and not sure which one the GPU one points to, or if it matters what it points to.

skinny85 · 2022-02-23T22:54:08Z

The tests should use the constants defined in the new class. If you're asking about testing the fromEcrRepository() method - no, it doesn't have to use real images. Any ECR repo, including a fake "imported" one (Repository.fromRepositoryName()), is fine for these tests.

…y supported by CDK by: - renaming the previously hidden class ArmBuildImage to LinuxArmBuildImage (in case there are Windows ARM Build Images in the future). - exporting LinuxArmBuildImage so it can be used. - adding the two ARM constants present in LinuxBuildImage also to LinuxArmBuildImage. - adding the method fromEcrRepository to support custom ARM build images. - making the LinuxArmBuildImage more akin to the LinuxBuildImage and WindowsBuildImage (built with props instead of just image name). - updating documentation to show examples of ARM and highlighting the LinuxBuildImage is for x86-64.

…ce it is an exact copy from existing tests)

tmmvn · 2022-02-27T00:26:08Z

packages/@aws-cdk/aws-codebuild/lib/project.ts

+  /** Image "aws/codebuild/amazonlinux2-aarch64-standard:2.0". */
+  public static readonly AMAZON_LINUX_2_ARM_2 = LinuxArmBuildImage.codeBuildImage('aws/codebuild/amazonlinux2-aarch64-standard:2.0');
+
+  /**


@skinny85 Any idea why the linter complains about @aws-cdk/aws-codebuild: error: [awslint:docs-public-apis:@aws-cdk/aws-codebuild.LinuxArmBuildImage.fromEcrRepository] Public API element must have a docstring?

If I'm not mistaken this should be it.

Try putting the @returns at the end (where it should be, honestly 😛).

I just copied what was already there in the file 🤷‍♂️. Seems to have fixed it though.

tmmvn · 2022-03-01T02:34:19Z

@skinny85 Passes tests but build fails with something that seems unrelated:

npm ERR! code ETARGET
npm ERR! notarget No matching version found for @aws-cdk/aws-iotevents-actions@1.144.0.
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.

Want further actions from me or can this PR be considered ready?

skinny85 · 2022-03-01T02:38:14Z

Try upgrading with the latest master - that should help.

tmmvn · 2022-03-01T13:42:17Z

@skinny85 That seems to have fixed that. Seems only validate-pr is missing any more. Not sure what that is?

Btw. I did a generic search for ARM in the GitHub issues and found ticket #9817, which I think is related. I probably did too specific of a search initially (or might have used aarch instead of ARM) so missed that one when creating my issue. I think this PR would solve that other ticket as well, though a bit differently (they were expecting a similar architecture setting / switching as I did).

skinny85 · 2022-03-01T18:18:12Z

Yes, let's add #9817 to the list of issued fixed by this PR 🙂.

skinny85

Looks great @codemeddler! Just a few minor notes on the code organization.

packages/@aws-cdk/aws-codebuild/README.md

packages/@aws-cdk/aws-codebuild/lib/project.ts

packages/@aws-cdk/aws-codebuild/test/linux-arm-build-image.test.ts

packages/@aws-cdk/aws-codebuild/lib/project.ts

…circular dependency after all.

…es. Instead of importing the whole codebuild project, import only the needed bits to linux-arm-build-image.test.ts.

…ArmBuildImage and back up on Project. Might actually still be due to the test includes, but guess that is the next go.

… and make sure the imports are sensible.

… Will wait for discussions before proceeding.

skinny85

Thanks for all your efforts on this @codemeddler! One last question.

packages/@aws-cdk/aws-codebuild/test/linux-arm-build-image.test.ts

# Conflicts: # packages/@aws-cdk/aws-codebuild/test/linux-arm-build-image.test.ts

…be proper.

tmmvn · 2022-03-05T17:35:13Z

Build was successful with the last discussion points as expected.

skinny85

Thanks for the contribution @codemeddler!

mergify · 2022-03-07T21:26:31Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-03-07T22:08:23Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: ef4fc87
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2022-03-07T22:08:34Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Fixes aws#18916 Fixes aws#9817 ### Motivation CDK currently has poor and hidden support for using ARM build images for CodeBuild that do not match what you can do with the Console. Currently, CDK has under LinuxBuildImage two constants not mentioned in the documentation. The constants internally map to a hidden ArmBuildImage class, which provides support for the standard CodeBuild ARM build images. That is the extent of the support, making ARM a second class citizen compared to x86-64 Linux and Windows build images as, for example, you can't use custom aarch64 ECR images. ### Changes This pull request addresses the missing support by: - renaming the previously hidden class ArmBuildImage to LinuxArmBuildImage (in case there are Windows ARM Build Images in the future). - exporting LinuxArmBuildImage so it can be used. - adding the two ARM constants present in LinuxBuildImage also to LinuxArmBuildImage. The constants are also left under LinuxBuildImage to not break backwards compatibility. - adding the method fromEcrRepository() to support custom ARM build images. - making the LinuxArmBuildImage closer to the LinuxBuildImage and WindowsBuildImage (built with props instead of just image name). - updating documentation to show examples of ARM and highlighting the LinuxBuildImage is for x86-64. ### Testing The unit test for ARM image is still valid. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

github-actions bot added the @aws-cdk/aws-codebuild Related to AWS CodeBuild label Feb 19, 2022

github-actions bot assigned skinny85 Feb 19, 2022

skinny85 changed the title ~~feat(aws-codebuild): add better support for arm build images~~ feat(codebuild): improve support for ARM build images Feb 19, 2022

tmmvn added 5 commits February 23, 2022 18:55

Merge remote-tracking branch 'origin/master'

5364c47

Fixed ImagePullPrincipalType error that crept in from IDE amend merge.

058bda0

Should fix linter issues (not sure why the props ones didn't fire sin…

a62eb86

…ce it is an exact copy from existing tests)

Added missing docstrings to make the linter happy.

3743377

tmmvn commented Feb 27, 2022

View reviewed changes

tmmvn added 6 commits February 28, 2022 18:25

Moved some @returns around to make the linter happy.

c57999a

Fixing CloudFormation layouts.

0ed3144

Linter fixes.

ec0d0f8

More CloudFormation template tweaks.

a0f8973

Removed one square bracket set too many.

14e987a

Fixes to the CloudFormation test.

8cd7282

Merge branch 'master' into master

0c3a4d6

tmmvn changed the title ~~feat(codebuild): improve support for ARM build images~~ feat(codebuild): improve support for arm build images Mar 1, 2022

skinny85 previously requested changes Mar 1, 2022

View reviewed changes

skinny85 reviewed Mar 1, 2022

View reviewed changes

packages/@aws-cdk/aws-codebuild/lib/project.ts Outdated Show resolved Hide resolved

tmmvn added 10 commits March 4, 2022 03:01

Moved LinuxArmBuildImage back to the end as it does seem to create a …

8194234

…circular dependency after all.

Went affray with the test includes earlier and solving the dependenci…

5ff6132

…es. Instead of importing the whole codebuild project, import only the needed bits to linux-arm-build-image.test.ts.

Linter fixes.

1edb3ea

Include slipped back up. Moved back down.

944a6da

Reversing the circular dep solving moving imports to the end in Linux…

303682f

…ArmBuildImage and back up on Project. Might actually still be due to the test includes, but guess that is the next go.

Back to square one. Need to debug the issue on a less gungho attitude…

5b28243

… and make sure the imports are sensible.

Think the module load order is now fixed.

09a8826

Missed one ComputeType.

8517bf1

Moved the arm build image import to the end of project.

a0ee6a7

This will likely solve the circular dependency issue, but looks ugly.…

8131da4

… Will wait for discussions before proceeding.

rix0rrr added feature-request A feature should be added or improved. p1 and removed p1 feature-request A feature should be added or improved. @aws-cdk/aws-codebuild Related to AWS CodeBuild labels Mar 4, 2022

skinny85 reviewed Mar 5, 2022

View reviewed changes

packages/@aws-cdk/aws-codebuild/test/linux-arm-build-image.test.ts Outdated Show resolved Hide resolved

tmmvn added 4 commits March 5, 2022 11:25

Changed to a qualified import as per discussion.

51f0b88

Changed to a qualified import as per discussion.

bd98377

Merge remote-tracking branch 'origin/master'

ab094b6

# Conflicts: # packages/@aws-cdk/aws-codebuild/test/linux-arm-build-image.test.ts

Epic merge fail. Guess adding a semicolon is a challenge. Should now …

f83d001

…be proper.

skinny85 approved these changes Mar 7, 2022

View reviewed changes

Merge branch 'master' into master

ef4fc87

mergify bot merged commit 4eac4de into aws:master Mar 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(codebuild): improved support for ARM build images #19052

feat(codebuild): improved support for ARM build images #19052

tmmvn commented Feb 19, 2022 •

edited

Loading

gitpod-io bot commented Feb 19, 2022

skinny85 commented Feb 19, 2022

tmmvn commented Feb 19, 2022

skinny85 commented Feb 21, 2022

tmmvn commented Feb 23, 2022

skinny85 commented Feb 23, 2022 •

edited

Loading

tmmvn Feb 27, 2022

skinny85 Feb 28, 2022

tmmvn Feb 28, 2022

tmmvn commented Mar 1, 2022

skinny85 commented Mar 1, 2022

tmmvn commented Mar 1, 2022

skinny85 commented Mar 1, 2022

skinny85 left a comment

skinny85 left a comment

tmmvn commented Mar 5, 2022

skinny85 left a comment

mergify bot commented Mar 7, 2022

aws-cdk-automation commented Mar 7, 2022

mergify bot commented Mar 7, 2022

feat(codebuild): improved support for ARM build images #19052

feat(codebuild): improved support for ARM build images #19052

Conversation

tmmvn commented Feb 19, 2022 • edited Loading

Motivation

Changes

Testing

The unit test for ARM image is still valid.

gitpod-io bot commented Feb 19, 2022

skinny85 commented Feb 19, 2022

tmmvn commented Feb 19, 2022

skinny85 commented Feb 21, 2022

tmmvn commented Feb 23, 2022

skinny85 commented Feb 23, 2022 • edited Loading

tmmvn Feb 27, 2022

Choose a reason for hiding this comment

skinny85 Feb 28, 2022

Choose a reason for hiding this comment

tmmvn Feb 28, 2022

Choose a reason for hiding this comment

tmmvn commented Mar 1, 2022

skinny85 commented Mar 1, 2022

tmmvn commented Mar 1, 2022

skinny85 commented Mar 1, 2022

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

tmmvn commented Mar 5, 2022

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Mar 7, 2022

aws-cdk-automation commented Mar 7, 2022

AWS CodeBuild CI Report

mergify bot commented Mar 7, 2022

tmmvn commented Feb 19, 2022 •

edited

Loading

skinny85 commented Feb 23, 2022 •

edited

Loading