-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Update go-discover to support ECS discovery #13782
Conversation
Hey @fdr2 Thanks for making this, we're in a bit of crunch-time right now gearing up for the next consul release, so i'd expect us to get to this after the release in early august. |
Sounds great @Amier3 thanks for the feedback. I'm hoping to investigate the performance concerns raised regarding efs, currently working out load tests with k6 and datadog in a similar fashion to the ec2 load test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some suggestions for the docs -- mostly edits for consistency with the style guide.
Co-authored-by: trujillo-adam <[email protected]>
improve readability
add datadog api key improve makefile
Thank you! These are excellent improvements. I've incorporated them all. |
Hi @fdr2, this is great work! Thank you. We'd prefer to put the load test code for ECS into our ECS-focused repository: https://github.com/hashicorp/terraform-aws-consul-ecs/ The path forward I'd suggest is:
I know this is a lot to ask, so thanks for your patience 🙂. Let me know if you have any questions. |
Co-authored-by: trujillo-adam <[email protected]>
improve readability
add datadog api key improve makefile
…pdates/go-discover # Conflicts: # go.sum
Hi @fdr2, thanks for your patience on this! Can you please add a changelog entry that indicates the support for ECS discovery? There are instructions here for including a changelog entry We were in code freeze at the time, so I wasn't able to include this in Consul 1.14.0, but this can go out in the next patch release(s) of 1.12, 1.13, and 1.14 after this is merged. |
Co-authored-by: trujillo-adam <[email protected]>
Co-authored-by: trujillo-adam <[email protected]>
Co-authored-by: trujillo-adam <[email protected]>
Co-authored-by: Frank DiRocco <[email protected]> Co-authored-by: trujillo-adam <[email protected]>
Co-authored-by: Frank DiRocco <[email protected]> Co-authored-by: trujillo-adam <[email protected]>
Co-authored-by: Frank DiRocco <[email protected]> Co-authored-by: trujillo-adam <[email protected]>
Co-authored-by: trujillo-adam <[email protected]>
Description
Go-Discover now supports ECS container instance discovery. This change updates the go-discover dependency and corresponding documentation outlining new options.
Testing & Reproduction steps
test/load-ecs has been added with load testing capabilities for 25 vus at 10 minutes. see test/load-ecs/README.md for more info.test/load-ecs has been moved to terraform-aws-consul-ecs PR:139Links
Issue: GH-6802
Dependency: PR-197
PR Checklist