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

Use aws/alb releaser for ecs #1577

Closed
izaaklauer opened this issue Jun 2, 2021 · 4 comments
Closed

Use aws/alb releaser for ecs #1577

izaaklauer opened this issue Jun 2, 2021 · 4 comments

Comments

@izaaklauer
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Currently, ECS uses it's own releaser, which has ALB manipulation logic very similar to the aws/alb plugin releaser. The alb releaser also has implemented release status, where ecs has not.

It would be nice if ECS could use the ALB releaser. Currently, trying to specify the ALB releaser on an ECS deployment yields the following error at release time:

» Releasing...
! 1 error occurred:
  	* argument cannot be satisfied: type *anypb.Any

It seems this is because the ALB releaser function wants a target group, while the ecs releaser wants a deployment.

@izaaklauer izaaklauer added the new label Jun 2, 2021
@izaaklauer izaaklauer changed the title Use aws/alb releaser for ECS Use aws/alb releaser for ecs Jun 2, 2021
@izaaklauer izaaklauer added this to the 0.4.x milestone Jun 2, 2021
@briancain briancain removed the new label Jun 9, 2021
@evanphx evanphx modified the milestones: 0.4.x, 0.5.x Jun 16, 2021
@catsby
Copy link
Contributor

catsby commented Jul 21, 2021

I looked into this and @izaaklauer and I discussed it some; the ECS deployment optionally creates a load balancer, security group, and target group and then the ECS releaser simply sets the weight to 100. The ALB releaser expects a target group to already exist and does the work (optional) of creating the security group, load balancer, et. al.

In order to get the ECS platform to not create the load balancer (and target group) users need to specify disable_alb in the use-ecs configuration... which contradicts the follow up use "alb" we would add in the release block of the configuration.

Ultimately 1 or both of these plugins would require some rework to make this work as described.

If the goal is to add a Status() method to ECS, we may want to instead look at reusing ALB's implementation via a shared utility or something.

@izaaklauer
Copy link
Contributor Author

Is there a reason that the ECS deployment plugin needs to create the load balancer? If the deployment doesn't need the ALB, then it seems like it should be the the releaser's job to create it.

@briancain briancain removed this from the 0.5.x milestone Aug 4, 2021
@briancain
Copy link
Member

Just writing a note here that fixing this could be potentially backwards incompatible when users upgrade to the version that has this fix. I removed the 0.5.x milestone and placed it in the Next Major project board for this reason.

@izaaklauer
Copy link
Contributor Author

There is a reason the ECS plugin needs to create the load balancer. As it turns out, ECS refuses to attach a target group to a service if the target group isn't behind a load balancer. It fails with this error:

The target group with targetGroupArn arn:aws:elasticloadbalancing:us-east-1:797645259670:targetgroup/example-nodejs-01FDG2SXAX5PQ9BW7/e007bed47dbebabc does not have an associated load balancer.

It also isn't possible for the ALB plugin to create the target group, as you can't modify target groups on an ECS service once it's been created: aws/containers-roadmap#712

The only option to standardize left then is to modify the ALB plugin to behave like the ECS releaser (i.e. just enable traffic on the target group), then modify the EC2 plugin to also create an ALB. I believe that will feel counterintuitive to EC2 users though.

Closing for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants