Skip to content
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.

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

feat(codebuild): support three arm-based compute types, Medium, X-Large and 2X-Large #31214

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

mazyu36
Copy link
Contributor

@mazyu36 mazyu36 commented Aug 25, 2024

Issue # (if applicable)

Closes #30869.

Reason for this change

Because three new arm-based compute types have become supported due to an update.

Announcement: AWS CodeBuild now supports three new Arm-based compute types

Description of changes

Modify the validate method to allow Medium, X-Large, and 2X-Large as Arm-based compute types.

Description of how you validated changes

Modify unit tests and integ tests.

Checklist


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

@aws-cdk-automation aws-cdk-automation requested a review from a team August 25, 2024 14:55
@github-actions github-actions bot added bug This issue is a bug. p2 distinguished-contributor [Pilot] contributed 50+ PRs to the CDK labels Aug 25, 2024
@@ -1751,12 +1751,12 @@ describe('Linux x86-64 Image', () => {
});

describe('ARM image', () => {
describe('AMAZON_LINUX_2_ARM', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since ARM_LINUX2_ARM is deprecated, I took the opportunity to update it as well.

Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@@ -1815,17 +1820,39 @@ describe('ARM image', () => {
});
});

test('cannot be used in conjunction with ComputeType X2_LARGE', () => {
test('can be used with ComputeType X_LARGE', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A unit test for X_LARGE was missing, so I've added.

@@ -68,17 +73,39 @@ describe('Linux ARM build image', () => {
});
});

test('cannot be used in conjunction with ComputeType X2_LARGE', () => {
test('can be used with ComputeType X_LARGE', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A unit test for X_LARGE was missing, so I've added.

@@ -146,17 +178,39 @@ describe('Linux ARM build image', () => {
});
});

test('cannot be used in conjunction with ComputeType X2_LARGE', () => {
test('can be used with ComputeType X_LARGE', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A unit test for X_LARGE was missing, so I've added.

@@ -224,17 +283,39 @@ describe('Linux ARM build image', () => {
});
});

test('cannot be used in conjunction with ComputeType X2_LARGE', () => {
test('can be used with ComputeType X_LARGE', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A unit test for X_LARGE was missing, so I've added.

@@ -38,17 +38,22 @@ describe('Linux ARM build image', () => {
});
});

test('cannot be used in conjunction with ComputeType MEDIUM', () => {
test('can be used with ComputeType MEDIUM', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although AMAZON_LINUX_2_STANDARD_1_0 is deprecated, there are tests implemented for STANDARD_1_0, so I decided to keep it as I considered removing it inappropriate.
If you think it would be better to remove it, please let me know.

@mazyu36
Copy link
Contributor Author

mazyu36 commented Aug 25, 2024

Exemption Request: As this is only a modification to the validate method and there are no changes affecting the README, I think an update to the README is unnecessary.

@aws-cdk-automation aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Aug 25, 2024
scanlonp
scanlonp previously approved these changes Aug 26, 2024
Copy link
Contributor

@scanlonp scanlonp left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the contribution!

@scanlonp scanlonp added pr-linter/exempt-readme The PR linter will not require README changes and removed pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. labels Aug 26, 2024
@aws-cdk-automation aws-cdk-automation dismissed their stale review August 26, 2024 19:26

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Aug 26, 2024
Copy link
Contributor

mergify bot commented Aug 26, 2024

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

@mergify mergify bot dismissed scanlonp’s stale review August 26, 2024 19:51

Pull request has been modified.

@scanlonp
Copy link
Contributor

@Mergifyio update

Copy link
Contributor

mergify bot commented Aug 26, 2024

update

☑️ Nothing to do

  • #commits-behind>0 [📌 update requirement]
  • -closed [📌 update requirement]
  • -conflict [📌 update requirement]
  • queue-position=-1 [📌 update requirement]

@scanlonp
Copy link
Contributor

@Mergifyio refresh

Copy link
Contributor

mergify bot commented Aug 27, 2024

refresh

✅ Pull request refreshed

@scanlonp
Copy link
Contributor

Hey @mazyu36, apologies for the noise. The build failed on a test with a race condition. Trying to re-run the build so that the PR can be merged.

Copy link
Contributor

@scanlonp scanlonp left a comment

Choose a reason for hiding this comment

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

Trigger codebuild.

scanlonp
scanlonp previously approved these changes Aug 27, 2024
@mergify mergify bot dismissed scanlonp’s stale review August 27, 2024 00:18

Pull request has been modified.

@mazyu36
Copy link
Contributor Author

mazyu36 commented Aug 27, 2024

@scanlonp
Thank you for your help. Could you please approve again?

@mazyu36 mazyu36 requested a review from scanlonp August 27, 2024 13:34
Copy link
Contributor

mergify bot commented Aug 29, 2024

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 1de3f2f
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify mergify bot merged commit 39492e9 into aws:main Aug 29, 2024
9 checks passed
Copy link
Contributor

mergify bot commented Aug 29, 2024

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

Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 29, 2024
@mazyu36 mazyu36 deleted the codebuild-project-30869 branch August 29, 2024 22:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. distinguished-contributor [Pilot] contributed 50+ PRs to the CDK p2 pr-linter/exempt-readme The PR linter will not require README changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

codebuild: ARM images supports all compute types, incorrect validation
3 participants