feat: Added support for random sleep delay in deploy submodule #233
Conversation
|
|
||
| SLEEP_TIME=$(( $RANDOM % 10 ) + ${var.get_deployment_sleep_timer}) | ||
| echo "Sleeping for: $SLEEP_TIME Seconds" | ||
| sleep $SLEEP_TIME |
There was a problem hiding this comment.
Are you sure that aws deploy get-deployment is being throttled? Sounds strange because it is a read type of operation even if it is executed multiple times at once.
There was a problem hiding this comment.
Hi @antonbabenko,
Thanks for your review.
In a busy day when we make multiple AWS API calls we get something like:
An error occurred (ThrottlingException) when calling the GetDeployment operation (reached max retries: 4): Rate exceeded
I think the following link describes the error (even if it's a few years old): https://forums.aws.amazon.com/thread.jspa?messageID=892511
There was a problem hiding this comment.
Right. Let's drop the get_deployment_sleep_timer variable because it is not necessary, but just sleep for 5 to 10 seconds, like this:
SLEEP_TIME=$(( $RANDOM % 5 ) + 5)
Short and simpler.
There was a problem hiding this comment.
I prefer to parameterize the sleep timer, as even if you are shifting 50% of the traffic every minute, you don't need to check the progress every ~ 5 seconds.
Is it a deal breaker for you if we keep the get_deployment_sleep_timer and set the default to 5?
There was a problem hiding this comment.
It is not a deal-breaker for me but I was not familiar with the use-case you had, so trying to balance between code simplicity and required flexibility. Let's keep it as you proposed. :)
Co-authored-by: Anton Babenko <anton@antonbabenko.com>
| @@ -106,7 +109,7 @@ resource "local_file" "deploy_script" { | |||
| } | |||
|
|
|||
| resource "null_resource" "deploy" { | |||
| count = var.create && var.create_deployment ? 1 : 0 | |||
| count = var.create && var.create_deployment && var.run_deployment ? 1 : 0 | |||
There was a problem hiding this comment.
Looks good. Could you also update related examples in the examples/deploy directory and verify them?
|
|
||
| SLEEP_TIME=$(( $RANDOM % 10 ) + ${var.get_deployment_sleep_timer}) | ||
| echo "Sleeping for: $SLEEP_TIME Seconds" | ||
| sleep $SLEEP_TIME |
There was a problem hiding this comment.
Right. Let's drop the get_deployment_sleep_timer variable because it is not necessary, but just sleep for 5 to 10 seconds, like this:
SLEEP_TIME=$(( $RANDOM % 5 ) + 5)
Short and simpler.
antonbabenko
changed the title
|
Thanks, @pkleanthous ! v2.27.0 has been just released. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
Adding here 2 minor tweaks to the deploy sub-module that I found useful.
Motivation and Context
When I deploy multiple lambdas and use the
wait_deployment_completionthe AWS is throttling the CLI calls and the script fails. Here, I'm introducing theget_deployment_sleep_timervariable that it can increase the sleep time. Also the sleep time is a random number between(0 and 10) + ${get_deployment_sleep_timer}.I want to separate the infrastructure deployment from the lambda lambda function release via CodeDeploy. The motivation behind this is to capture any issues that introduced by the infrastructure changes.
An example is, you change the permissions for a resource that a lambda function use, the old version immediately louses access and you wait until the traffic switches to the new version that has all the right permissions. So the
run_deploymentnow will trigger the bash script otherwise deploy the infra, save the script and run it whenever you want.Breaking Changes
In order to run the deployment bash script now you need to set
run_deployment = trueHow Has This Been Tested?