feat: add time zone support for pool schedules #4063
Conversation
Fixes philips-labs#4056 Replaces the `aws_cloudwatch_event_rule` in the `pool` module (which only support UTC) with `aws_scheduler_schedule`, which supports arbitrary time zones via the `schedule_expression_timezone` attribute. The AWS EventBridge Scheduler is a drop in replacement for scheduler based AWS EventBridge Rules ([see AWS blog post](https://aws.amazon.com/blogs/compute/introducing-amazon-eventbridge-scheduler/)), including the expression syntax and pricing. The timezone can be set via the `schedule_expression_timezone` property via the main module or `multi-runners` module.
janslow
changed the title
janslow
force-pushed
the
janslow/aws-scheduler-pool
branch
from
869e302
to
e086e3a
Compare
|
Note, I've tried to run the GitHub Action to update the auto-generated Terraform Docs (as per the contribution guide), but it's failing because it requires credentials that I don't have access to. In terms of testing, I couldn't find any existing automated tests for the pool, although I have tested it on my existing install (both with and without the new property) |
Terraform docs will updated on either release or main branch. Feel free to create an issue to address the shortcoming in the release notes
We only have automated tests for the lambda's, not for the full module. We doe typically a manual test dependending on the change with one of the examples. Would be great if we have an full auto test test. Any idea would be welcome how to run auto tests for IaC on an OS repo in a secure way. |
The process of updating the Terraform module documentation is now run on the `main` branch ([example](philips-labs#3919)), so it no longer needs to be run on forked repositories (it also requires credentials for the GitHub App). As per @npalm's [comment](philips-labs#4063 (comment)), I'm removing this reference, to prevent any confusion for new contributors.
👍 I've created a PR to remove the reference from the contribution guide, as it's no longer relevant when creating a PR. |
Unfortunately, I'm not aware of any magic solution. For internal apps, I've used Terratest to spin up ephemeral environments that are realistic, then run integration tests against them. I think that'd be tricky for this project though, especially given the fact it's a public project. One possibility could be to use Terraform's native test features to plan the example modules (using mock providers, so no AWS access is required), then running assertions against it. For example, asserting that all of the |
Thanks for the feedback! That was also my take and the reason why there no intergration tests (yet). Internally for the runners we deploy to a staging and ran validation workflows as verification. All automated. |
|
Tested with ephemeral example, works as expected. |
The process of updating the Terraform module documentation is now run on the `main` branch ([example](#3919)), so it no longer needs to be run on forked repositories (it also requires credentials for the GitHub App). As per @npalm's [comment](#4063 (comment)), I'm removing this reference, to prevent any confusion for new contributors.
Co-authored-by: Niek Palm <npalm@users.noreply.github.com>
|
Thanks for the review, I've accepted your suggestion |
|
Thanks for adding the feature! Feel free to update the other time triggers as well! |
🤖 I have created a release *beep* *boop* --- ## [5.15.0](v5.14.1...v5.15.0) (2024-08-16) ### Features * add time zone support for pool schedules ([#4063](#4063)) ([b8f9eb4](b8f9eb4)) @janslow * scale up for long waiting jobs (job retry) ([#4064](#4064)) ([6120571](6120571)) ### Bug Fixes * **lambda:** bump axios from 1.7.2 to 1.7.4 in /lambdas ([#4071](#4071)) ([2f32195](2f32195)) * **lambda:** bump the aws group in /lambdas with 5 updates ([#4057](#4057)) ([5ecdbad](5ecdbad)) * **lambda:** bump the aws-powertools group in /lambdas with 3 updates ([#4058](#4058)) ([f9533f3](f9533f3)) * **lambda:** Prevent scale-up lambda from starting runner for user repo if org level runners is enabled ([#3909](#3909)) ([98b1560](98b1560)) @PerGon --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: forest-releaser[bot] <80285352+forest-releaser[bot]@users.noreply.github.com>
## Problme In #4063 PR the EventBridge Schedule is introduces. For schedule groups a name prefix is used. But since the name for the name prefix is already unieque the name can be used instead of prefix to avoid too long prefix names (30 chars plus).
Fixes #4056
The timezone can be set via the
schedule_expression_timezoneproperty via the main module ormulti-runnersmodule.Replaces the
aws_cloudwatch_event_rulein thepoolmodule (which only support UTC) withaws_scheduler_schedule, which supports arbitrary time zones via theschedule_expression_timezoneattribute.In this situation AWS EventBridge Scheduler is a drop in replacement for schedule-based AWS EventBridge Rules (see AWS blog post), including the expression syntax and pricing. The main (internal) difference is that AWS Scheduler requires the use of an IAM Role, instead of Lambda permissions.