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

fix(core): increase timeout for AcmCertificateImporter #464

Merged
merged 1 commit into from
Jun 17, 2021

Conversation

ddneilson
Copy link
Contributor

Fixes: #463


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

yarn.lock Outdated
@@ -507,6 +507,13 @@
"@aws-cdk/region-info" "1.106.1"
constructs "^3.3.69"

"@aws-cdk/aws-imagebuilder@1.106.1":
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the change to add the imagebuilder example didn't include an update to the yarn.lock

// component of backoff fixed to ensure minimum total wait time on
// slow targets.
const base = Math.pow(2, attempt);
await defaultSleep(Math.random() * base * 50 + base * 150);
const backoffDelay = Math.floor(Math.random() * base * 200 + base * 1000 - 100);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've increased the delay from a base of 200ms. I figure we're waiting for the ACM service here to update the certificate; possibly also waiting for the ALB to signal that it's no longer using it as well.

So, rather than waiting (max): 0.2s, 0.35s, 0.65s, 1.250s, 2.45s, 4.85s, 9.65s, 19.25s (which won't finish due to 30s lambda timeout)

It might make more sense to wait longer, but cap out the delay (max) 1.1s, 2.1s, 4.1s, 8.1s, 16.1s, 30s, 30s, ....

@ddneilson ddneilson force-pushed the fix_issue409 branch 2 times, most recently from 3f03beb to faa3ebf Compare June 14, 2021 14:49
jusiskin
jusiskin previously approved these changes Jun 14, 2021
@jusiskin jusiskin added the contribution/core This is a PR that came from AWS. label Jun 14, 2021
jericht
jericht previously approved these changes Jun 16, 2021
@jusiskin jusiskin dismissed stale reviews from jericht and themself via 435692f June 17, 2021 14:44
Copy link
Contributor

@yashda yashda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks good to me! Thanks Daniel for the fix.

@jusiskin jusiskin merged commit 18a8098 into aws:mainline Jun 17, 2021
@ddneilson ddneilson deleted the fix_issue409 branch July 28, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AcmCertificateImporter timeout during stack delete
4 participants