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.

Sign up for GitHub

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

Jump to bottom

(autoscaling) maxInstanceLifetime limits doesnt match cloudformation documentation #12588

Closed
trondhindenes opened this issue Jan 19, 2021 · 4 comments
Closed

(autoscaling) maxInstanceLifetime limits doesnt match cloudformation documentation #12588

trondhindenes opened this issue Jan 19, 2021 · 4 comments
Assignees
@NetaNir
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@trondhindenes
Copy link

I'm unable to configure the maxInstanceLifetime setting in autoscaling group to lower than 7 days

Reproduction Steps

        runner_asg = autoscaling.AutoScalingGroup(
            self,
            'runner_asg',
            vpc=vpc,
            vpc_subnets=ec2.SubnetSelection(subnet_type=ec2.SubnetType.PRIVATE),
            instance_type=ec2.InstanceType(instance_type_identifier=ec2_instance_size),
            machine_image=ec2.MachineImage.latest_amazon_linux(),
            signals=autoscaling.Signals.wait_for_all(),
            init=ec2.CloudFormationInit.from_config(init_config),
            role=iam_role,
            block_devices=[
                autoscaling.BlockDevice(
                    device_name='/dev/xvda',
                    volume=autoscaling.BlockDeviceVolume.ebs(30)
                )
            ],
            max_instance_lifetime=core.Duration.days(2)
        )

What did you expect to happen?

It should succeed

What actually happened?

it fails with this error:
jsii.errors.JSIIError: maxInstanceLifetime must be between 7 and 365 days (inclusive).

According to cloudformation, anything above one day is acceptable: "The default is null. If specified, the value must be either 0 or a number equal to or greater than 86,400 seconds (1 day)."

Environment

  • **CDK CLI Version : 1.78.0 (build 2c74f4c)

Other

This is 🐛 Bug Report
@trondhindenes trondhindenes added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 19, 2021
@NGL321 NGL321 changed the title autoscaling: maxInstanceLifetime limits doesnt match cloudformation documentation (autoscaling) maxInstanceLifetime limits doesnt match cloudformation documentation Jan 25, 2021
@github-actions github-actions bot added the @aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling label Jan 25, 2021
@NetaNir
Copy link
Contributor

NetaNir commented Jan 26, 2021

Have you tried using CloudFormation directly to set a number < 7 || > 365? The CDK will often encode undocumented CFN limits which are only encounter during deployment.

@NetaNir NetaNir added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jan 26, 2021
@trondhindenes
Copy link
Author

I'm afraid I havent.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jan 27, 2021
@NetaNir NetaNir closed this as completed Feb 24, 2021
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@jungseoklee
Copy link
Contributor

This is a bug as reported. An user can specify maxInstanceLifetime less than 7 days. Also, we should allow 0. Otherwise, an user cannot clear maxInstanceLifetime through CFN.

@jungseoklee jungseoklee mentioned this issue Mar 28, 2022
Merged
4 tasks
mergify bot pushed a commit that referenced this issue Apr 8, 2022
@jungseoklee
fix(autoscaling): update validation on maxInstanceLifetime (#19584)
d115b47 
### Summary

This patch fixes the following bugs in `maxInstanceLifetime` validation by aligning with [CFN doc](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-as-group.html#cfn-as-group-maxinstancelifetime):

 - A lower bound of `maxInstanceLifetime` is 1 day, not 7-days.
 - `maxInstanceLifetime` can have 0 which is used to clear a previously set value.

### Test

- Run unit test.
```
PASS test/auto-scaling-group.test.js

=============================== Coverage summary ===============================
Statements   : 93.11% ( 419/450 )
Branches     : 86.93% ( 286/329 )
Functions    : 91.81% ( 101/110 )
Lines        : 92.84% ( 402/433 )
================================================================================
```
- Deploy CFN template having `maxInstanceLifetime` as 86,400 and 0.

### Notes

Originally, this issue was reported by #12588.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
otaviomacedo pushed a commit that referenced this issue Apr 11, 2022
@jungseoklee @otaviomacedo
fix(autoscaling): update validation on maxInstanceLifetime (#19584)
2e32550 
### Summary

This patch fixes the following bugs in `maxInstanceLifetime` validation by aligning with [CFN doc](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-as-group.html#cfn-as-group-maxinstancelifetime):

 - A lower bound of `maxInstanceLifetime` is 1 day, not 7-days.
 - `maxInstanceLifetime` can have 0 which is used to clear a previously set value.

### Test

- Run unit test.
```
PASS test/auto-scaling-group.test.js

=============================== Coverage summary ===============================
Statements   : 93.11% ( 419/450 )
Branches     : 86.93% ( 286/329 )
Functions    : 91.81% ( 101/110 )
Lines        : 92.84% ( 402/433 )
================================================================================
```
- Deploy CFN template having `maxInstanceLifetime` as 86,400 and 0.

### Notes

Originally, this issue was reported by #12588.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
StevePotter pushed a commit to StevePotter/aws-cdk that referenced this issue Apr 27, 2022
@jungseoklee
fix(autoscaling): update validation on maxInstanceLifetime (aws#19584)
721f479 
### Summary

This patch fixes the following bugs in `maxInstanceLifetime` validation by aligning with [CFN doc](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-as-group.html#cfn-as-group-maxinstancelifetime):

 - A lower bound of `maxInstanceLifetime` is 1 day, not 7-days.
 - `maxInstanceLifetime` can have 0 which is used to clear a previously set value.

### Test

- Run unit test.
```
PASS test/auto-scaling-group.test.js

=============================== Coverage summary ===============================
Statements   : 93.11% ( 419/450 )
Branches     : 86.93% ( 286/329 )
Functions    : 91.81% ( 101/110 )
Lines        : 92.84% ( 402/433 )
================================================================================
```
- Deploy CFN template having `maxInstanceLifetime` as 86,400 and 0.

### Notes

Originally, this issue was reported by aws#12588.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@NetaNir NetaNir

Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@trondhindenes @NetaNir @jungseoklee