aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

aws-opensearchservice/domain: `r7gd` not usable #32138

Open alex-at-cascade opened 23 hours ago

alex-at-cascade commented 23 hours ago

Describe the bug

When trying to create a domain with dataNodeInstanceType of r7gd.4xlarge.search, CDK reports Error: EBS volumes are required when using instance types other than R3, I3, R6GD, I4G, or IM4GN. - however, gd instances have integrated storage and do not use EBS storage.

Regression Issue

Last Known Working CDK Version

No response

Expected Behavior

I expected r7gd instances to be allowed, as the AWS console is urging me to upgrade to the newer generation.

Current Behavior

I get the error noted above.

Reproduction Steps

See description.

Possible Solution

Correct the list. (Having a hard-coded list seems really brittle - is there another way to achieve this?)

Additional Information/Context

No response

CDK CLI Version

2.166.0 (build 7bb9203)

Framework Version

No response

Node.js Version

v18.18.2

OS

macOS Monterey Version 12.7.6

Language

TypeScript

Language Version

typescript 5.4.3

Other information

No response

khushail commented 22 hours ago

Hi @alex-at-cascade , thanks for reporting this. I see the error is generated because of the code section given below. The list needs to be updated with r7gd instance as its a Graviton instance and does not need internal EBS Storage.

Screenshot 2024-11-14 at 12 15 33 PM

https://github.com/aws/aws-cdk/blob/d14a01c5ca75fcec01810bb6fc9ac708c719ee7f/packages/aws-cdk-lib/aws-opensearchservice/lib/domain.ts#L1598

    // Validate against instance type restrictions, per
    // https://docs.aws.amazon.com/opensearch-service/latest/developerguide/supported-instance-types.html
    if (isSomeInstanceType('i3', 'r6gd', 'i4g', 'im4gn') && ebsEnabled) {
      throw new Error('I3, R6GD, I4G, and IM4GN instance types do not support EBS storage volumes.');
    }

I am marking this as P2 which means it won't be immediately addressed by the team but would be on their radar and would also be open for community contribution.

You are right in saying there should be a generalised way of adding these instance rather than hardcoded values. Suggestions are welcome in this regard.

I have been working on adding i4i instance with this PR and would see if I can take up this one too.

alex-at-cascade commented 22 hours ago

Great, thank you! One approach that might be less of an obstruction would be to have two lists : those that are known to need EBS, and those that are known not to. If the instance type is on one of those lists, throw an error if the config is wrong. But if it's a new instance type not on either list, log a prominent warning (not error) about a currently unknown instance type, and urging to verify the setting in either case (and maybe also recommend to file a support ticket 😉 )