feat(codebuild): attribute-based compute type for Fleet by badmintoncryer · Pull Request #32251 · aws/aws-cdk

badmintoncryer · 2024-11-22T13:30:19Z

Issue # (if applicable)

None

Reason for this change

AWS Codebuild supports for creating Fleet with attribute based compute type.
https://docs.aws.amazon.com/codebuild/latest/userguide/fleets.html#fleets.attribute-compute

You can specify minimum vCPU, disk and memory sizes. Codebuild automatically selects the instance type based on the compute configuration.

Description of changes

Add computeConfiguraion prop to FleetProps.

Description of how you validated changes

Add both unit and integ tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

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.

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

codecov · 2024-11-22T13:53:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.83%. Comparing base (29a9a6d) to head (350b159).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32251   +/-   ##
=======================================
  Coverage   80.83%   80.83%           
=======================================
  Files         236      236           
  Lines       14253    14253           
  Branches     2490     2490           
=======================================
  Hits        11522    11522           
  Misses       2446     2446           
  Partials      285      285

Flag	Coverage Δ
suite.unit	`80.83% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`79.57% <ø> (ø)`
packages/aws-cdk-lib/core	`82.20% <ø> (ø)`

badmintoncryer · 2024-11-28T10:43:24Z

CodeQL job has failed, but this is caused by automatically generated test code and I think this is not a problem.

nmussy · 2024-12-19T05:02:02Z

I swear I searched for issues and PRs before starting #32574 🤦 I'll close mine and give yours a review today

nmussy

Had a quick first look by comparing our implementations, I'll try again with a more neutral outlook later. Thanks!

packages/aws-cdk-lib/aws-codebuild/lib/compute-type.ts

nmussy · 2024-12-19T05:10:47Z

packages/aws-cdk-lib/aws-codebuild/lib/fleet.ts

+        disk: props.computeConfiguration.disk?.toGibibytes(),
+        machineType: props.computeConfiguration.machineType,
+        memory: props.computeConfiguration.memory?.toGibibytes(),
+        vCpu: props.computeConfiguration.vCpu,


Despite what the CloudFormation documentation says, the numeric props are not optional, see https://github.com/nmussy/aws-cdk/blob/6120d8f54103f81c4ec601221c1f56e5a4549236/packages/%40aws-cdk-testing/framework-integ/test/aws-codebuild/test/integ.project-fleet-attribute-based-compute.ts#L67-L73

Could you give them a default value, like this for example, and add an integ with just { machineType: GENERAL }?

nmussy · 2024-12-19T05:13:14Z

packages/aws-cdk-lib/aws-codebuild/lib/fleet.ts

      throw new Error('baseCapacity must be greater than or equal to 1');
    }

+    if (props.computeConfiguration) {


I also had this validation to make sure the user saw how ATTRIBUTE_BASED_COMPUTE worked, leaving it up to you if you want to add it as well:

Suggested change

if (props.computeConfiguration) {

if (props.computeType === FleetComputeType.ATTRIBUTE_BASED &&

(!props.computeConfiguration || Object.keys(props.computeConfiguration).length === 0)) {

throw new Error('At least one compute configuration criteria must be specified if computeType is "ATTRIBUTE_BASED"');

}

if (props.computeConfiguration) {

nmussy · 2024-12-19T05:17:55Z

packages/aws-cdk-lib/aws-codebuild/lib/fleet.ts

You're missing a getter for the fromFleetArn Import class:

public get computeConfiguration(): FleetComputeConfiguration | undefined { throw new Error('Cannot retrieve computeConfiguration property from an imported Fleet'); }

GavinZZ

LGTM, thank you as always!

GavinZZ · 2025-02-05T19:12:16Z

...sting/framework-integ/test/aws-codebuild/test/integ.project-fleet-attribute-based-compute.ts

+const listFleets = test.assertions.awsApiCall('Codebuild', 'listFleets');
+for (const { fleet, project } of stacks) {
+  listFleets.expect(integ.ExpectedResult.objectLike({
+    fleets: integ.Match.arrayWith([fleet.fleetArn]),
+  }));
+
+  const startBuild = test.assertions.awsApiCall('Codebuild', 'startBuild', { projectName: project.projectName });


Nice assertions!

mergify · 2025-02-05T19:13:15Z

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).

Pull request has been modified.

mergify · 2025-02-05T22:50:38Z

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 · 2025-02-05T23:23:03Z

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 · 2025-02-05T23:28:38Z

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 · 2025-02-06T04:05:55Z

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 · 2025-02-06T18:10:23Z

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).

mrgrain · 2025-02-06T19:30:27Z

@Mergifyio dequeue

mergify · 2025-02-06T19:30:37Z

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #32251 has been dequeued by a dequeue command

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

mergify · 2025-02-06T19:30:37Z

dequeue

✅ The pull request has been removed from the queue `default-squash`

mrgrain · 2025-02-06T19:33:57Z

@Mergifyio dequeue

mergify · 2025-02-06T19:34:06Z

dequeue

☑️ The pull request is not queued

mrgrain · 2025-02-06T19:53:03Z

@Mergifyio requeue

mergify · 2025-02-06T19:53:11Z

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

aws-cdk-automation · 2025-02-06T20:26:01Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 350b159
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2025-02-06T20:26:25Z

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).

github-actions · 2025-02-06T21:42:57Z

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.

badmintoncryer added 4 commits November 20, 2024 22:08

feat: attribute based compute type

4bd1584

compute configuration

83f2140

add integ test

211d820

add unit test

605fd16

github-actions bot added the p2 label Nov 22, 2024

aws-cdk-automation requested a review from a team November 22, 2024 13:30

github-actions bot added the distinguished-contributor [Pilot] contributed 50+ PRs to the CDK label Nov 22, 2024

aws-cdk-automation previously requested changes Nov 22, 2024

View reviewed changes

update readme

8ee1d1c

badmintoncryer marked this pull request as ready for review November 22, 2024 13:41

Merge branch 'main' into compute-config

4ed820d

badmintoncryer added 2 commits November 22, 2024 23:17

Update README.md

47d1de2

fix readme

c8f21ef

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 22, 2024

badmintoncryer added 4 commits November 23, 2024 09:11

Merge branch 'main' into compute-config

b895761

Merge branch 'main' into compute-config

decf3a3

Merge branch 'main' into compute-config

52b4ef6

Merge branch 'main' into compute-config

1359119

github-actions bot mentioned this pull request Dec 1, 2024

Monthly PR metrics report #32340

Closed

badmintoncryer added 4 commits December 2, 2024 13:10

Merge branch 'main' into compute-config

8d1bead

Merge branch 'main' into compute-config

d978959

Merge branch 'main' into compute-config

382571d

Merge branch 'main' into compute-config

fffcf47

nmussy mentioned this pull request Dec 19, 2024

feat(codebuild): fleet attribute-based compute type #32574

Closed

1 task

nmussy suggested changes Dec 19, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Dec 19, 2024

badmintoncryer added 2 commits December 29, 2024 11:07

Merge branch 'main' into compute-config

cec7cfa

Merge branch 'main' into compute-config

6a1e46f

GavinZZ previously approved these changes Feb 5, 2025

View reviewed changes

Merge branch 'main' into compute-config

1b4ef4f

GavinZZ approved these changes Feb 5, 2025

View reviewed changes

Merge branch 'main' into compute-config

a65cafa

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 5, 2025

Merge branch 'main' into compute-config

3c26b93

Merge branch 'main' into compute-config

91cba44

QuantumNeuralCoder assigned GavinZZ Feb 6, 2025

Merge branch 'main' into compute-config

350b159

mergify bot merged commit 9fd62cb into aws:main Feb 6, 2025
19 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Feb 6, 2025

-    if (props.computeConfiguration) {
+    if (props.computeType === FleetComputeType.ATTRIBUTE_BASED &&
+      (!props.computeConfiguration || Object.keys(props.computeConfiguration).length === 0)) {
+      throw new Error('At least one compute configuration criteria must be specified if computeType is "ATTRIBUTE_BASED"');
+    }
+    if (props.computeConfiguration) {

Conversation

badmintoncryer commented Nov 22, 2024

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

badmintoncryer commented Nov 28, 2024

Uh oh!

nmussy commented Dec 19, 2024

Uh oh!

nmussy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nmussy Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

nmussy Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

nmussy Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

GavinZZ left a comment

Choose a reason for hiding this comment

Uh oh!

GavinZZ Feb 5, 2025

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Feb 5, 2025

Uh oh!

mergify bot commented Feb 5, 2025

Uh oh!

mergify bot commented Feb 5, 2025

Uh oh!

mergify bot commented Feb 5, 2025

Uh oh!

mergify bot commented Feb 6, 2025

Uh oh!

mergify bot commented Feb 6, 2025

Uh oh!

mrgrain commented Feb 6, 2025

Uh oh!

mergify bot commented Feb 6, 2025

Uh oh!

mergify bot commented Feb 6, 2025

✅ The pull request has been removed from the queue default-squash

Uh oh!

mrgrain commented Feb 6, 2025

Uh oh!

mergify bot commented Feb 6, 2025

☑️ The pull request is not queued

Uh oh!

mrgrain commented Feb 6, 2025

Uh oh!

mergify bot commented Feb 6, 2025

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

Uh oh!

aws-cdk-automation commented Feb 6, 2025

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Feb 6, 2025

Uh oh!

Uh oh!

github-actions bot commented Feb 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

codecov bot commented Nov 22, 2024 •

edited

Loading

✅ The pull request has been removed from the queue `default-squash`