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

aws-batch: Malformed Instance Type thrown for new InstanceType("optimal") regardless of architecture #31762

Closed
1 task
DaveMadsen opened this issue Oct 15, 2024 · 2 comments
Assignees
Labels
@aws-cdk/aws-batch Related to AWS Batch bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@DaveMadsen
Copy link

Describe the bug

We use new InstanceType("optimal") in our platform for batch resources. In the past, this has "just worked." Now we are getting an error, Resolution error: Malformed instance type identifier. I expect this to "just work".

it looks like this commit broke us in 2.161.1. If I understand what is happening, a check was added to block optimal use (or at least warn against its use) on ARM64 instances, but it looks like the architecture check is breaking because optimal is not yet resolved into a valid architecture.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

2.160.0

Expected Behavior

I am able to use new InstanceType("optimal") on x86-64 architecture.

Current Behavior

new InstanceType("optimal") throws Resolution error: Malformed instance type identifier` on all architectures.

Reproduction Steps

let instanceType = new InstanceType("optimal");

Possible Solution

"optimal" needs to resolve into the target instance types before doing the architecture check. Longer term, "optimal" should be deprecated/removed or expanded to ARM64 archs.

Additional Information/Context

No response

CDK CLI Version

2.161.1 (build 0a606c9)

Framework Version

No response

Node.js Version

v18.20.4

OS

Linux 5.10 x86_64

Language

TypeScript

Language Version

TypeScript ~4.7.4

Other information

No response

@DaveMadsen DaveMadsen added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 15, 2024
@github-actions github-actions bot added the @aws-cdk/aws-batch Related to AWS Batch label Oct 15, 2024
@DaveMadsen DaveMadsen changed the title (module name): (short issue description) aws-batch: Malformed Instance Type thrown for new InstanceType("optimal") regardless of architecture Oct 15, 2024
@khushail khushail added needs-reproduction This issue needs reproduction. p2 and removed needs-triage This issue or PR still needs to be triaged. labels Oct 15, 2024
@khushail khushail self-assigned this Oct 15, 2024
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-reproduction This issue needs reproduction. labels Oct 17, 2024
@khushail
Copy link
Contributor

khushail commented Oct 17, 2024

Hi @DaveMadsen , thanks for reaching out.

this is what I found about 'Optimal' types in cdk docs-

https://github.com/aws/aws-cdk/tree/main/packages/aws-cdk-lib/aws-batch

Note: Batch does not allow specifying instance types or classes with different architectures. For example, InstanceClass.A1 cannot be specified alongside 'optimal', because A1 uses ARM and 'optimal' uses x86_64. You can specify both 'optimal' alongside several different instance types in the same compute environment:

I also deployed the cdk stack with CDK Version 2.160 and 2.162, both deployed successfully for me.
Sharing the code and snippets -

CDK Version 2.160 - Typescript code -

    const vpc01 = ec2.Vpc.fromLookup(this, 'myVpc', {
      isDefault: true,
      });
      
    const computeEnvironment = new batch.ManagedEc2EcsComputeEnvironment(this, 'myEc2ComputeEnv', {
      instanceTypes: [ec2.InstanceType.of(ec2.InstanceClass.M5AD, ec2.InstanceSize.LARGE)],
      useOptimalInstanceClasses: true, // default
      vpc: vpc01,
    });

synthesized template snippet -

  "myEc2ComputeEnvF0625F52": {
   "Type": "AWS::Batch::ComputeEnvironment",
   "Properties": {
    "ComputeResources": {
     "AllocationStrategy": "BEST_FIT_PROGRESSIVE",
     "InstanceRole": {
      "Fn::GetAtt": [
       "myEc2ComputeEnvInstanceProfileEACE4CE2",
       "Arn"
      ]
     },
     "InstanceTypes": [
      "m5ad.large",
      "optimal"
     ],
     "MaxvCpus": 256,
     "MinvCpus": 0,
     "SecurityGroupIds": [
      {
       "Fn::GetAtt": [
        "myEc2ComputeEnvSecurityGroupD0EB1CF5",
        "GroupId"
       ]
      }
     ],

Same code deployed with CDK Version 2.162.0 and it deployed successfully -

Screenshot 2024-10-17 at 2 29 55 PM

I think this is what you are trying to do. Please feel free to correct if this is misunderstood.

Also would request you to please share a self contained minimal repro code for the above mentioned issue as I am not able to repro it on my side.

@khushail khushail added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Oct 17, 2024
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-batch Related to AWS Batch bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

2 participants