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-applicationautoscaling, aws-autoscaling: Scheduled actions missing support to specify timezone property #22645

Closed
1 of 2 tasks
ShadowDog007 opened this issue Oct 25, 2022 · 9 comments · Fixed by #29116
Closed
1 of 2 tasks
Labels
@aws-cdk/aws-applicationautoscaling Related to AWS Application Auto Scaling effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@ShadowDog007
Copy link

Describe the feature

Scheduled actions for both applicationautoscaling ScalableTarget and autoscaling support specifying the timezone property.

This should be exposed from CDK apis.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-as-scheduledaction.html#cfn-as-scheduledaction-timezone

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-applicationautoscaling-scalabletarget-scheduledaction.html#cfn-applicationautoscaling-scalabletarget-scheduledaction-timezone

Use Case

Supporting scaling actions which adjust for daylight savings

Proposed Solution

No response

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.47.0

Environment details (OS name and version, etc.)

n/a

@ShadowDog007 ShadowDog007 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 25, 2022
@github-actions github-actions bot added the @aws-cdk/aws-applicationautoscaling Related to AWS Application Auto Scaling label Oct 25, 2022
@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 3, 2022
@peterwoodworth
Copy link
Contributor

Yes, we should be able to support this. Thanks for the request.

@bora-7 do you want to work on this?

@bora-7
Copy link
Contributor

bora-7 commented Nov 4, 2022

Yes, I will work on this.

@ShadowDog007
Copy link
Author

@bora-7 just FYI #17330 added timeszone to aws-autoscaling but it's still missing from aws-applicationautoscaling

@sothawo
Copy link

sothawo commented Oct 27, 2023

A pity. This is now one year old, and CDK still does not support timezones in aws-applicationautoscaling.

@neilferreira
Copy link
Contributor

It looks like @bora-7 had a crack at this but it was never meged

@neilferreira
Copy link
Contributor

nevermind - this was actioend in 2.97.1 with the new Schedule class! https://github.com/aws/aws-cdk/releases/tag/v2.97.1

This can be closed.

@sothawo
Copy link

sothawo commented Jan 11, 2024

I am using aws-cdk 2.115.0. I am creating an ECS service

            const scaledTask = loadBalancedFargateService.service.autoScaleTaskCount({
                maxCapacity: props.ecsService.maxScaledContainerCount,
                minCapacity: props.ecsService.desiredContainerCount,
            })

loadBalancedFargateService.service is a FargateService; and then when trying to set a schedule

            scaledTask.scaleOnSchedule("sched1", {
                startTime: someTime,
                endTime: someTime,
                // no timezone here
            })

there still is no possibility to set the timezone

@saudansari
Copy link

This was brought in version 2.97.1 but then it was taken out. Now I'm stuck at 2.97 and unable to move forward without the timezone capability for my fargate ecs tasks.

@mergify mergify bot closed this as completed in #29116 Feb 15, 2024
mergify bot pushed a commit that referenced this issue Feb 15, 2024
Closes #22645
Closes #27754

Spiritual successor of #27052

Somewhat related to #21181 but that might be another PR down the road.

@pahud ✋ Please review. I'm not particularly fond of how `aws-autoscaling` module ([here](https://github.com/aws/aws-cdk/blob/256cca4017a80f8643c5f5a5999a2ce0383eebf0/packages/aws-cdk-lib/aws-autoscaling/lib/scheduled-action.ts#L21)) is not using `cdk.TimeZone` class, hence why used it in this PR instead. I think we should we change `aws-autoscaling` implementation to do the same? It would be a breaking change... and most likely a brand new PR. LMK what you think. ✌️ 

Also, I may be slightly OCD but I kinda like better `timezone` vs `timeZone`, but I went with latter one to follow what `aws-autoscaling` did.

cc-ing @kaizencc for his input too 🙌  ... possibly related to #27105

### Reason for this change



Timezones have been supported in `AWS::ApplicationAutoScaling::ScalableTarget ScheduledAction` for a while now.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-applicationautoscaling-scalabletarget-scheduledaction.html#cfn-applicationautoscaling-scalabletarget-scheduledaction-timezone


### Description of changes

Just added the support for `timezones` in `scalableTarget.scaleOnSchedule`

### Description of how you validated changes

Added unit tests for this feature.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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.

GavinZZ pushed a commit that referenced this issue Feb 22, 2024
Closes #22645
Closes #27754

Spiritual successor of #27052

Somewhat related to #21181 but that might be another PR down the road.

@pahud ✋ Please review. I'm not particularly fond of how `aws-autoscaling` module ([here](https://github.com/aws/aws-cdk/blob/256cca4017a80f8643c5f5a5999a2ce0383eebf0/packages/aws-cdk-lib/aws-autoscaling/lib/scheduled-action.ts#L21)) is not using `cdk.TimeZone` class, hence why used it in this PR instead. I think we should we change `aws-autoscaling` implementation to do the same? It would be a breaking change... and most likely a brand new PR. LMK what you think. ✌️ 

Also, I may be slightly OCD but I kinda like better `timezone` vs `timeZone`, but I went with latter one to follow what `aws-autoscaling` did.

cc-ing @kaizencc for his input too 🙌  ... possibly related to #27105

### Reason for this change



Timezones have been supported in `AWS::ApplicationAutoScaling::ScalableTarget ScheduledAction` for a while now.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-applicationautoscaling-scalabletarget-scheduledaction.html#cfn-applicationautoscaling-scalabletarget-scheduledaction-timezone


### Description of changes

Just added the support for `timezones` in `scalableTarget.scaleOnSchedule`

### Description of how you validated changes

Added unit tests for this feature.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*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
Labels
@aws-cdk/aws-applicationautoscaling Related to AWS Application Auto Scaling effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
7 participants