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

AutoScalingGroup Constructor: Overloaded Constructor / Different Props #26439

Closed
1 of 2 tasks
NeilGGopal opened this issue Jul 19, 2023 · 2 comments
Closed
1 of 2 tasks
Assignees
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. documentation This is a problem with documentation. feature-request A feature should be added or improved. p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@NeilGGopal
Copy link

NeilGGopal commented Jul 19, 2023

Describe the feature

Create overloaded constructor(s) for AutoScalingGroup with different properties. These alternate constructors can contain either launchTemplate or mixedInstancesPolicy while simultaneously removing all properties that must not be specified when either of these two props are passed into the original constructor.

Use Case

When migrating my CFN templates to CDK, it is difficult for me to debug my AutoScalingGroup CDK code because of the way ASGs are described in the CDK documentation and because of the way ASGs are defined in code. Building my templates takes a long time (even for a single package), and running into an error with launchTemplate, mixedInstancesPolicy and machineImage is frustrating.

Properties such as instanceType specify that the launchTemplate / mixedInstancesPolicy must not be specified. However, launchTemplate and mixedInstancesPolicy do not specify the other properties that break them in return.

To fix these issues, we can both add more detail to launchTemplate / mixedInstancesPolicy in the documentation and split the AutoScalingGroupProps interface into different interfaces to prevent this collision in the first place.

Proposed Solution

constructor(scope: Construct, id: string, props: ASGLaunchTemplateProps);

// ASGLaunchTemplateProps is the same as AutoScalingGroupProps minus the properties
// that state launchTemplate must not be specified
export interface ASGLaunchTemplateProps extends CommonASGLaunchTemplateProps {
readonly vpc: ec2.IVpc;
readonly launchTemplate?: ec2.ILaunchTemplate;
readonly init?: ec2.CloudFormationInit;
readonly initOptions?: ApplyCloudFormationInitOptions;
readonly requireImdsv2?: boolean;
}

// CommonASGLaunchTemplateProps is the same as CommonAutoScalingGroupProps minus the properties
// that state launchTemplate must not be specified
export interface CommonAutoScalingGroupProps {
readonly minCapacity?: number;
readonly maxCapacity?: number;
readonly desiredCapacity?: number;
readonly vpcSubnets?: ec2.SubnetSelection;
readonly notifications?: NotificationConfiguration[];
readonly allowAllOutbound?: boolean;
readonly ignoreUnmodifiedSizeProperties?: boolean;
readonly cooldown?: Duration;
readonly healthCheck?: HealthCheck;
readonly maxInstanceLifetime?: Duration;
readonly groupMetrics?: GroupMetrics[];
readonly signals?: Signals;
readonly updatePolicy?: UpdatePolicy;
readonly newInstancesProtectedFromScaleIn?: boolean;
readonly autoScalingGroupName?: string;
readonly terminationPolicies?: TerminationPolicy[];
readonly defaultInstanceWarmup?: Duration;
readonly ssmSessionPermissions?: boolean;
}

// Note: The original CommonAutoScalingGroupProps can also be modified to move the properties
// that state "launchTemplate must not be specified" to AutoScalingGroupProps

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.87.0

Environment details (OS name and version, etc.)

OSX / Using AL2_x86_64

@NeilGGopal NeilGGopal added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jul 19, 2023
@github-actions github-actions bot added the @aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling label Jul 19, 2023
@khushail khushail added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Jul 19, 2023
@khushail khushail self-assigned this Jul 19, 2023
@khushail khushail removed the needs-triage This issue or PR still needs to be triaged. label Jul 19, 2023
@khushail
Copy link
Contributor

khushail commented Jul 19, 2023

Hi @NeilGGopal ,thanks for reaching out with this request.
As you mentioned about 2 workarounds -

  1. updating the documentation => Yes this could be done provided what larifications are needed exactly and how should we improve the existing documentation which might help in easier debugging. Sharing a snippet of error code or insights could be helpful here.
  2. Splitting the interface into multiple interfaces => this might not be a good idea as it contradicts with the design of cdk and we won't recommend doing so.

@khushail khushail added p2 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. documentation This is a problem with documentation. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Jul 19, 2023
@github-actions
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 Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. documentation This is a problem with documentation. feature-request A feature should be added or improved. 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