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

ECSRun add network mode argument #4704

Closed
bennnym opened this issue Jun 27, 2021 · 5 comments
Closed

ECSRun add network mode argument #4704

bennnym opened this issue Jun 27, 2021 · 5 comments
Labels
enhancement An improvement of an existing feature status:stale This may not be relevant anymore

Comments

@bennnym
Copy link

bennnym commented Jun 27, 2021

Current behavior

ECSRun doesn't appear to have the ability to run an ecs job on your existing ec2 instance in your cluster (that your prefect ecs agent is running on).

There is a slack message on the prefect threads that go into it too.

"When using ECS with launch_type EC2, the default network mode used for the Task Definition is awsvpc. As I am using the default VPC whose subnets have the auto assign public IP for IPv6 is disabled, the containers cannot communicate with prefect API because they only have the private network interface.

This can be solved if bridge network mode is used. However, as long as I can see in the documentation, there is no way to modify that, just the task_definition which forces to define the whole TaskDefinition manually. "

Proposed behavior

Ability to change ECSRun to Flows on your backed ec2 instances with an additional argument in the object.

@bennnym bennnym added the enhancement An improvement of an existing feature label Jun 27, 2021
@kvnkho kvnkho self-assigned this Jun 28, 2021
@kvnkho
Copy link
Contributor

kvnkho commented Jun 29, 2021

I have spent some time scoping this out. Just some clarifications on the issue. There are two steps from the ECS Client when running ECS tasks. The first one is the register_task_defnition call to register. The second one is the run_task call to actually run the task. ECSRun provides a run_task_kwargs argument, but this goes into run_task, meaning that arguments intended for register_task_defnition can't be passed this way.

AWS ECS takes networkMode as an argument for the register_task_definition method call. This can take the following values: bridge|host|awsvpc|none. Because this is unavailable to pass iin the ECSRun, the current implementation of the ECSAgent relies on this networkMode being passed through the task-defnition. Users have to rely on creating the task-defnition from scratch.

The deprecated FargateAgent actually has good support for changing networkMode. It does this by taking in kwargs and then pass them to register_task_definition first, and the remaining ones to run_task.

Proposed solutions:

  1. Expose networkMode in ECSRun

We can take in the networkMode parameter and pass this explicitly to the register_task_definition call.

  1. Expose a register_task_definition_kwargs in ECSRun

We could create a dictionary parameter and networkMode would be passed through that. This would then be combined with the task-definition and altogether passed to the register_task_definition. I have a feeling we moved away from this so I will dig in a bit deeper as to the downsides of this.

Honestly leaning towards solution 1 right now unless someone else has input or I find that there are other arguments we want to expose on the ECSRun side.

References:

register_task_definition

run_task

Current Workaround:

The easiest way to specify networkMode right now is:

task_definition = yaml.safe_load(
      """
      networkMode: bridge
      cpu: 1024
      memory: 2048
      containerDefinitions:
      - name: flow
      """
  )
ECSRun( ..., task_definition=task_definition)

@bennnym
Copy link
Author

bennnym commented Jun 30, 2021

I think #1 is a good solution. Great work @kvnkho

@Sanil2108
Copy link

Is there any update on this issue?

@zanieb
Copy link
Contributor

zanieb commented Nov 18, 2021

It's on our backlog to investigate further. Since there is a viable workaround (providing a task definition), it is not a high priority.

@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. To keep this issue open remove stale label or comment.

@github-actions github-actions bot added the status:stale This may not be relevant anymore label Jan 31, 2023
@bennnym bennnym closed this as not planned Won't fix, can't repro, duplicate, stale Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature status:stale This may not be relevant anymore
Projects
None yet
Development

No branches or pull requests

4 participants