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 cluster type #7

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@fabiorphp

fabiorphp commented Nov 28, 2018

Was defined a cluster_type property for creating an specific ecs cluster and added variable for creating task definitions.

@fabiorphp fabiorphp requested a review from mtulio Nov 28, 2018

@mtulio

Great improvement, a few changes.

README.md Outdated
@@ -63,6 +79,7 @@ Including an example of how to use your role (for instance, with variables passe
- us-east-1a
subnets:
- subnet-abcd1234
custer_type: ec2

This comment has been minimized.

@mtulio

mtulio Nov 30, 2018

Contributor

cluster_type

This comment has been minimized.

@fabiorphp
@@ -5,13 +5,16 @@
- set_fact:
cluster_userdata: "{{ lookup('template', 'lcg_userdata.j2') }}"
when: item_c.cluster_type == "ec2"

This comment has been minimized.

@mtulio

mtulio Nov 30, 2018

Contributor

Too many conditional (when) here, IMHO you could use one block here. What do you think?

This comment has been minimized.

@fabiorphp
Create cluster based on cluster_type
This new property will create a specific cluster for AWS ECS.

@fabiorphp fabiorphp force-pushed the feat/fargate branch from 48f3629 to f449af0 Nov 30, 2018

- name: Create Auto Scaling Group

This comment has been minimized.

@mtulio

mtulio Nov 30, 2018

Contributor

@fabiorphp I did not find the retro compability here, we have another projects that uses this role with default behavior (using cluster=ec2 - but it is not defined, beacuse is new option), if them update then run, I could see an broken execution, right?

What do you think? I believe that we can leva the default behavior in ec2 and declare fargate.

This comment has been minimized.

@fabiorphp

This comment has been minimized.

@fabiorphp
@@ -28,7 +28,23 @@ ecs_clusters: # the list of ECS cluster conf
- us-east-1a
subnets: # List of subnets
- subnet-abcd1234
cluster_type: ec2 # Cluster type: ec2 or fargate

This comment has been minimized.

@mtulio

mtulio Nov 30, 2018

Contributor

It could be clear if some comments here, like: "To create an fargate cluster you just run...."

This comment has been minimized.

@fabiorphp

@fabiorphp fabiorphp force-pushed the feat/fargate branch from f449af0 to 7e41c7a Dec 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment