Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

ECS bug fixes and improvements #4742

Merged
merged 9 commits into from May 31, 2023
Merged

Conversation

paladin-devops
Copy link
Contributor

@paladin-devops paladin-devops commented May 24, 2023

This PR was originally intended for adding an improvement which enables the user to set the target group protocol and protocol version for an AWS ECS deployment. However, it also adds some logging and fixes some bugs!

  1. The protocol configured for the health check was incorrectly being assigned to the protocol of the target group itself - this is now correctly being assigned to the protocol of the health check.
  2. If the user set grpc_code or http_code inside the health_check block for the AWS ECS deployment plugin, the plugin would panic - this is fixed by setting the Matcher field of the CreateTargetGroupInput struct to an empty struct of the type Matcher.
  3. When destroying a target group during a destroy operation, the check for the # of target groups was always failing, even if the correct # of target groups (1) was indeed returned. Now, if the correct # of target groups (1) is found, the release destroy op proceeds.
  4. The IAM policy created for the ODR during a runner install did not include the policy permission elasticloadbalancing:DescribeTargetHealth - this has been added!
  5. Fix the logic for destroying the ALB to skip destroying the ALB if it is not managed by Waypoint. If the ALB has a tag waypoint_managed and it is set to true, then the ALB will be deleted on when a destroy job runs.

@paladin-devops paladin-devops added plugin/ecs ecosystem Things related to waypoint interacting with external systems backport/0.11.x labels May 24, 2023
@paladin-devops paladin-devops requested a review from a team May 24, 2023 21:59
@paladin-devops paladin-devops requested a review from a team as a code owner May 24, 2023 21:59
@paladin-devops paladin-devops self-assigned this May 24, 2023
…col and version.

Prior to this commit, only the HTTP protocol was supported. Users of applications which require other protocols, or other protocol versions, such as gRPC, are now supported.

A bug was also fixed, where the user-specified health check protocol was previously being set to the target group protocol, instead of the health check's.
Upgrade AWS SDK version as well.
"target_group_protocol",
"The protocol to use for routing traffic to the targets.",
docs.Default("HTTP"),
docs.Summary("The protocol to use for routing traffic to the targets. "+
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 pulled the descriptions for these two fields from the AWS docs - I wasn't sure if that was kosher for our docs, so if it's not, happy to change!

Copy link
Contributor

@izaaklauer izaaklauer left a comment

Choose a reason for hiding this comment

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

🔥

Prior to this commit, the ALB would be deleted even if Waypoint did not create it.
Copy link
Member

@catsby catsby left a comment

Choose a reason for hiding this comment

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

Question about defaulting and hiding an error message

builtin/aws/ecs/platform.go Show resolved Hide resolved
builtin/aws/ecs/platform.go Outdated Show resolved Hide resolved
builtin/aws/ecs/platform.go Outdated Show resolved Hide resolved
Co-authored-by: Clint <catsby@users.noreply.github.com>
@catsby
Copy link
Member

catsby commented May 31, 2023

Fixes #4745

@catsby
Copy link
Member

catsby commented May 31, 2023

Hey @paladin-devops are we good to go here with this PR? I believe it fixes an existing bug in 0.11.1

@paladin-devops
Copy link
Contributor Author

@catsby I just pushed another update which should fix the failing Go test, and addressed your last open feedback. We'll be good to go after checks pass.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/0.11.x core ecosystem Things related to waypoint interacting with external systems plugin/aws plugin/ecs plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants