feat(codebuild): fleet attribute-based compute type #32574
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
p2
distinguished-contributor
nmussy
commented
Dec 18, 2024
Comment on lines
+207
to
+214
| if (props.computeConfiguration && props.computeType !== FleetComputeType.ATTRIBUTE_BASED) { | ||
| throw new Error('computeConfiguration can only be specified if computeType is "ATTRIBUTE_BASED"'); | ||
| } | ||
|
|
||
| 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"'); | ||
| } |
There was a problem hiding this comment.
I'm only doing base prop validation here. I think currently invalid attribute configurations, such as the 192 GiB of memory and NVME machine type described in the integ test, shouldn't be validated by the CDK. These limitations are bound to change over time and across regions.
nmussy
commented
Dec 18, 2024
Comment on lines
+42
to
+73
| // No NVME instance is available with this much memory, and the deployment fails. | ||
| // Throws error: The specified compute configurations do not match any available instance type | ||
| // When creating a fleet in the web console with the same constraints, | ||
| // "reserved.x86-64.96cpu.192gib" is selected as the instance type, despite not meeting the NVME criteria | ||
| /* { | ||
| computeConfiguration: { | ||
| memory: 192, | ||
| machineType: codebuild.FleetComputeConfigurationMachineType.NVME, | ||
| }, | ||
| expectedInstanceType: 'reserved.x86-64.96cpu.192gib', | ||
| }, */ | ||
|
|
||
| // A default account wasn't allowed to deploy the smallest available NVME instance in us-east-1 | ||
| // Throws error: The number of instances of type c5d.12xlarge exceeded limit 0 | ||
| // Skipping this test instead of opening a quota increase support ticket to save myself and future integ runners the trouble | ||
| /* { | ||
| computeConfiguration: { | ||
| machineType: codebuild.FleetComputeConfigurationMachineType.NVME, | ||
| disk: 824, | ||
| memory: 96, | ||
| vCpu: 48, | ||
| }, | ||
| expectedInstanceType: 'reserved.x86-64.48cpu.96gib.NVME', | ||
| }, */ | ||
|
|
||
| // Prior to setting default values for numeric attributes internally, the CloudFormation template fails on deployment: | ||
| // Throws error: Resource handler returned message: "Cannot invoke "java.lang.Integer.intValue()" because the return value of "software.amazon.codebuild.fleet.ComputeConfiguration.getMemory()" is null" | ||
| // { vCpu: 1 }, | ||
| // Throws error: Resource handler returned message: "Cannot invoke "java.lang.Integer.intValue()" because the return value of "software.amazon.codebuild.fleet.ComputeConfiguration.getvCpu()" is null" | ||
| // { memory: 1 }, | ||
| // Throws error: Resource handler returned message: "Cannot invoke "java.lang.Integer.intValue()" because the return value of "software.amazon.codebuild.fleet.ComputeConfiguration.getDisk()" is null" | ||
| // { memory: 1, vCpu: 1 }, |
There was a problem hiding this comment.
Apologies if this seems very verbose, but I think laying out my testing process is helpful both for this PR's reviewer and future maintainers, given how little the official documentation explains any of this
nmussy
changed the title
2 tasks
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
aws-cdk-automation
added
the
pr/needs-community-review
1 task
|
Duplicate of #32251 |
|
Comments on closed issues and PRs are hard for our team to see. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue # (if applicable)
None that I could find, related to #30458
Reason for this change
Implement missing attribute-based compute feature to the L2 CodeBuild
Fleetconstruct. See docs for (sparse) referenceDescription of changes
computeConfigurationproperty to theFleetresourceATTRIBUTE_BASEDvalue to the existingFleetComputeTypeenumDescribe any new or updated permissions being added
None
Description of how you validated changes
Added unit and integration tests. The failed configurations were also listed in the integration, with an explanation given as to why their deployment had resulted in an error
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license