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

Fargate support #23

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

davidworkman9
Copy link

Fixes fargate support from the ecs-nginx-proxy project.

New to: go, amazon ecs, and fargate so this is definitely a hack, but it works for me! Feedback would be appreciated.

@PradeepJaiswar
Copy link

Can this be merged anytime soon? Really want to try quick.

@codesuki
Copy link
Owner

codesuki commented Jan 3, 2018

I am on holiday right now, but I'll try to have a look.

Thanks for the PR! Much appreciated.

@codesuki
Copy link
Owner

codesuki commented Jan 9, 2018

I finally had a look at the code.
First thing I want to confirm is about the port. You chose 80 as the default which works for you. Which port does your container listen on?
What I want to know is if the new task networking that fargate seems to use automatically uses the port you use inside your container.
So please let me know which port your container runs, if it's also port 80 or another one. And in case you change your container (app) port if it still works with port 80.

@codesuki
Copy link
Owner

codesuki commented Jan 9, 2018

I found the answer.
An error occurred (ClientException) when calling the RegisterTaskDefinition operation: When networkMode=awsvpc, the host ports and container ports in port mappings must match.

So could you rewrite the code a little.
If the TaskDefinition has NetworkMode set to awsvpc (you have to confirm if that is the string, just my best guess), then use the private IP as you do now and set the container port as the port.

If the networking is bridge then do the same as current master branch, using the port mappings.

If you feel super motivated you could also add support for host networking by just using the instance IP + container port. (A mix of the other two)

@codesuki
Copy link
Owner

Is this PR still alive? :)

@davidworkman9
Copy link
Author

Sorry, I've been busy. Will try to look at the suggestions this weekend.

@anbotero
Copy link

We quickly declared virtualPort in line 118, just to accept different port we assign from variables, but we would really like to see @codesuki suggestions into the code 🤞

@codesuki
Copy link
Owner

@anbotero Hi there. Recently I am exclusively using Google Cloud so I have no chance to test things on AWS.
If you want you can take over this PR, fix it up and I will merge it.

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

Successfully merging this pull request may close these issues.

Fargate Support
4 participants