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

chore(ebs): set default volumeType to gp3(under feature flag) #29934

Merged
merged 15 commits into from Apr 30, 2024
10 changes: 7 additions & 3 deletions packages/aws-cdk-lib/aws-ec2/lib/volume.ts
Expand Up @@ -3,8 +3,9 @@ import { CfnVolume } from './ec2.generated';
import { IInstance } from './instance';
import { AccountRootPrincipal, Grant, IGrantable } from '../../aws-iam';
import { IKey, ViaServicePrincipal } from '../../aws-kms';
import { IResource, Resource, Size, SizeRoundingBehavior, Stack, Token, Tags, Names, RemovalPolicy } from '../../core';
import { IResource, Resource, Size, SizeRoundingBehavior, Stack, Token, Tags, Names, RemovalPolicy, FeatureFlags } from '../../core';
import { md5hash } from '../../core/lib/helpers-internal';
import * as cxapi from '../../cx-api';

/**
* Block device
Expand Down Expand Up @@ -65,7 +66,8 @@ export interface EbsDeviceOptionsBase {
* The EBS volume type
* @see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html
*
* @default `EbsDeviceVolumeType.GP2`
* @default `EbsDeviceVolumeType.GENERAL_PURPOSE_SSD` or `EbsDeviceVolumeType.GENERAL_PURPOSE_SSD_GP3` if
* `@aws-cdk/aws-ec2:ebsDefaultGp3Volume` is enabled.
*/
readonly volumeType?: EbsDeviceVolumeType;
}
Expand Down Expand Up @@ -621,7 +623,9 @@ export class Volume extends VolumeBase {
size: props.size?.toGibibytes({ rounding: SizeRoundingBehavior.FAIL }),
snapshotId: props.snapshotId,
throughput: props.throughput,
volumeType: props.volumeType ?? EbsDeviceVolumeType.GENERAL_PURPOSE_SSD,
volumeType: props.volumeType ??
(FeatureFlags.of(this).isEnabled(cxapi.EBS_DEFAULT_GP3) ?
EbsDeviceVolumeType.GENERAL_PURPOSE_SSD_GP3 : EbsDeviceVolumeType.GENERAL_PURPOSE_SSD),
});
resource.applyRemovalPolicy(props.removalPolicy);

Expand Down
18 changes: 18 additions & 0 deletions packages/aws-cdk-lib/aws-ec2/test/volume.test.ts
Expand Up @@ -2,6 +2,7 @@ import { Match, Template } from '../../assertions';
import { AccountRootPrincipal, Role } from '../../aws-iam';
import * as kms from '../../aws-kms';
import * as cdk from '../../core';
import * as cxapi from '../../cx-api';
import {
AmazonLinuxGeneration,
EbsDeviceVolumeType,
Expand Down Expand Up @@ -457,6 +458,23 @@ describe('volume', () => {
});
});

test('EBS_DEFAULT_GP3 feature flag', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
stack.node.setContext(cxapi.EBS_DEFAULT_GP3, true);
new Volume(stack, 'Volume', {
availabilityZone: 'us-east-1a',
size: cdk.Size.gibibytes(500),
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::EC2::Volume', {
VolumeType: 'gp3',
});
});

describe('grantAttachVolume to any instance with encryption', () => {
test('with default key policies', () => {
// GIVEN
Expand Down
19 changes: 16 additions & 3 deletions packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
Expand Up @@ -68,8 +68,7 @@ Flags come in three types:
| [@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2](#aws-cdkaws-codepipelinedefaultpipelinetypetov2) | Enables Pipeline to set the default pipeline type to V2. | 2.133.0 | (default) |
| [@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope](#aws-cdkaws-kmsreducecrossaccountregionpolicyscope) | When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only. | 2.134.0 | (fix) |
| [@aws-cdk/aws-eks:nodegroupNameAttribute](#aws-cdkaws-eksnodegroupnameattribute) | When enabled, nodegroupName attribute of the managed EKS NodeGroup will not have the cluster name prefix. | 2.138.0 | (fix) |


| [@aws-cdk/aws-ec2:ebsDefaultGp3Volume](#aws-cdkaws-ec2ebsdefaultgp3volume) | When enabled, the default volume type of the EBS volume will be GP3 | 2.139.0 | (default) |

<!-- END table -->

Expand Down Expand Up @@ -128,7 +127,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-codepipeline:crossAccountKeysDefaultValueToFalse": true,
"@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2": true,
"@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope": true,
"@aws-cdk/aws-eks:nodegroupNameAttribute": true
"@aws-cdk/aws-eks:nodegroupNameAttribute": true,
"@aws-cdk/aws-ec2:ebsDefaultGp3Volume": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to have something like "@aws-cdk/aws-ec2:ebsDefaultVolume": "gp3"? The current setup makes future changes in the default volume type a little more awkward, we'll have to have multiple aws-cdk/aws-ec2:ebsDefaultXXXVolume flags, make sure there are no conflicts, etc.

Copy link
Contributor Author

@pahud pahud Apr 23, 2024

Choose a reason for hiding this comment

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

Let's say if we introduce "@aws-cdk/aws-ec2:ebsDefaultVolume": "gp3" in this PR and after a while AWS announces gp4 as a new default and we modify this feature flag to "@aws-cdk/aws-ec2:ebsDefaultVolume": "gp4", this would cause breaking changes for those opting in gp3 and we still need a check to see if they have explicitly enable this flag as gp3. I am not sure if that would be easier to maintain. Another consideration is conventionally our feature flags are boolean except for target partitions and we generally don't update value of existing feature flags. Will check this with the maintainers.

Copy link
Contributor

Choose a reason for hiding this comment

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

we modify this feature flag to "@aws-cdk/aws-ec2:ebsDefaultVolume": "gp4", this would cause breaking changes for those opting in gp3 and we still need a check to see if they have explicitly enable this flag as gp3

Wouldn't we only change the recommended value to gp4? Users that opted in to gp3 would be in the same situation as those who enabled ebsDefaultGp3Volume if we introduce ebsDefaultGp4Volume. Arguably even worse off, since they would both need to disable ebsDefaultGp3Volume and enable ebsDefaultGp4Volume, vs just updating ebsDefaultVolume to gp4.

I'm leaving it up to you guys, I might not be seeing the whole picture, just throwing it out there 👍

}
}
```
Expand Down Expand Up @@ -1280,5 +1280,18 @@ When this feature flag is enabled, the nodegroupName attribute will be exactly t
| (not in v1) | | |
| 2.138.0 | `false` | `true` |

### @aws-cdk/aws-ec2:ebsDefaultGp3Volume

*When enabled, the default volume type of the EBS volume will be GP3* (default)

When this featuer flag is enabled, the default volume type of the EBS volume will be `EbsDeviceVolumeType.GENERAL_PURPOSE_SSD_GP3`.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| 2.139.0 | `false` | `true` |

**Compatibility with old behavior:** Pass `volumeType: EbsDeviceVolumeType.GENERAL_PURPOSE_SSD` to `Volume` construct to restore the previous behavior.

<!-- END details -->
13 changes: 13 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Expand Up @@ -102,6 +102,7 @@ export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/
export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';
export const KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE = '@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope';
export const EKS_NODEGROUP_NAME = '@aws-cdk/aws-eks:nodegroupNameAttribute';
export const EBS_DEFAULT_GP3 = '@aws-cdk/aws-ec2:ebsDefaultGp3Volume';

export const FLAGS: Record<string, FlagInfo> = {
//////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1047,6 +1048,18 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
},

//////////////////////////////////////////////////////////////////////
[EBS_DEFAULT_GP3]: {
type: FlagType.ApiDefault,
summary: 'When enabled, the default volume type of the EBS volume will be GP3',
detailsMd: `
When this featuer flag is enabled, the default volume type of the EBS volume will be \`EbsDeviceVolumeType.GENERAL_PURPOSE_SSD_GP3\`.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
compatibilityWithOldBehaviorMd: 'Pass `volumeType: EbsDeviceVolumeType.GENERAL_PURPOSE_SSD` to `Volume` construct to restore the previous behavior.',
},
};

const CURRENT_MV = 'v2';
Expand Down