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.

Sign up for GitHub

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

ecs_service_info lists all services in all clusters when no cluster attribute is given #2058

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

ecs_service_info lists all services in all clusters when no cluster attribute is given #2058

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4de8ebd
Add cluster(less) query to ecs_service_info
markotting Mar 18, 2024
eb16fcf
Added test to validate commits with no cluster
markotting Mar 18, 2024
d381118
Fixed not being able to deal with service names
markotting Mar 18, 2024
c1028cf
Fixed using the wrong service name in tests
markotting Mar 18, 2024
0d1ebee
Modified integration test. Given that we can now pass multiple cluste…
markotting Mar 18, 2024
253a097
Last fix to integration tests
markotting Mar 18, 2024
eed692d
Update plugins/modules/ecs_service_info.py
b0tting Mar 19, 2024
f63e625
Update plugins/modules/ecs_service_info.py
b0tting Mar 19, 2024
049e728
Update plugins/modules/ecs_service_info.py
b0tting Mar 19, 2024
456012c
Removed explicit error when no clusters or services exist
markotting Apr 1, 2024
2dc19a3
Merge branch 'main' of github.com:b0tting/community.aws into main
markotting Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 55 additions & 15 deletions plugins/modules/ecs_service_info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@
type: bool
cluster:
description:
- The cluster ARNS in which to list the services.
- The cluster ARNS in which to list the services. If not provided, all clusters are listed.
required: false
type: str
type: list
elements: str
service:
description:
- One or more services to get details for
- One or more services to get details for. If not provided, all services are listed.
required: false
type: list
elements: str
Expand All @@ -55,9 +56,15 @@
details: true
register: output

# Basic listing example
# Basic listing example for all services in all clusters
- community.aws.ecs_service_info:
cluster: test-cluster
details: true
register: output
# Basic listing example for the list of services in two specific clusters
- community.aws.ecs_service_info:
cluster:
- test-cluster
- prod-cluster
register: output
"""

Expand Down Expand Up @@ -161,6 +168,17 @@ def list_services_with_backoff(self, **kwargs):
def describe_services_with_backoff(self, **kwargs):
return self.ecs.describe_services(**kwargs)

def list_clusters(self):
try:
paginator = self.ecs.get_paginator("list_clusters")
response = paginator.paginate().build_full_result()
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
self.module.fail_json_aws(e, msg="Couldn't list ECS clusters")
clusters = list(response["clusterArns"])
if not clusters:
self.module.fail_json_aws(e, msg="Account does not have any ECS clusters")
markuman marked this conversation as resolved.
Show resolved Hide resolved
return clusters

def list_services(self, cluster):
fn_args = dict()
if cluster and cluster is not None:
Expand Down Expand Up @@ -203,6 +221,26 @@ def extract_service_from(self, service):
e["createdAt"] = str(e["createdAt"])
return service

def get_cluster_services(self):
if self.module.params["cluster"]:
clusters = self.module.params["cluster"]
else:
clusters = self.list_clusters()

cluster_services = {}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cluster_services = {}
cluster_services = {}

Service name per cluster are unique. What will happen when more than one cluster contains the same service name?
Will the last service name overwrite the first service name?

Copy link
Author

Choose a reason for hiding this comment

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

I think there is an issue when the details flag is not added or set to false. This would return just the list of services with no attributes. Considering the scenario of 2 clusters, with the same HelloWorld service.

- community.aws.ecs_service_info:
    details: false

..would get you a list of 2 service ARNs

- community.aws.ecs_service_info:
    service: arn:aws:ecs:eu-central-1:123456789012:service/my-cluster/HelloWorld

..would get you a single ARN

But giving service names (not service ARNs) with details: false in the current ecs_service_info returns you just the existing services. So in this scenario:

- community.aws.ecs_service_info:
    details: false
    service: HelloWorld

..would in the current module fail, or force you to add a cluster and return just [HelloWorld]. But what should be the response if you have the given HelloWorld scenario? [HelloWorld] is ambiguous right? But adding cluster info or returning a cluster:service dict would be a change that changes the module response compared to the current version.

I am not an experienced contributer. Do you have a suggestion how to proceed here?

Copy link
Member

Choose a reason for hiding this comment

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

Ah maybe I also misinterpreted the return value

cluster_services[cluster] = ...

So it's already grouped by cluster.

returning a cluster:service dict would be a change that changes the module response compared to the current version.

I think this is the only solution. That would also mean that it is a breaking change and the changes will release with 8.0.0 and not in 7.2.0

Copy link
Author

@b0tting b0tting Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Okay. I need some guidance in what would be good form for the output.

So (with details: false) the current output:

services:
	 - hello_world-1
	 - hello_world-2

If I were to slide in the clusters I could add them as an extra level below services:

services:
  my-cluster: 
  	 - hello_world-1
	 - hello_world-2
  my-other-cluster: 
  	 - hello_world-1
	 - hello_world-2

Or should this be more explicit (and renaming the root item) in the lines of:

result: 
  - cluster: my-cluster
    services: 
  	 - hello_world-1
	 - hello_world-2
  - cluster: my-other-cluster
    services: 
  	 - hello_world-1
	 - hello_world-2

Copy link
Member

Choose a reason for hiding this comment

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

any opinions @tremble @alinabuzachis?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, current return value for details: false is

services:
	 - hello_world-1
	 - hello_world-2

using details: true results in

services:
	 - clusterArn: ...blue
	   serviceName: hello_world-1
           ....
	 - clusterArn: ...red
	   serviceName: hello_world-1
           ...

I guess we should keep this structure.
And once cluster name is omit, details should be automatically set to true

    - name: test
      register: out
      ecs_service_info:
        cluster: "{{ omit }}"
        details: true

This won't break any backwards compatiblity and won't return any ambiguous results in case a service name exist in more than one cluster.

What do you think @b0tting

for cluster in clusters:
service_arns = self.list_services(cluster)["services"]
if self.module.params["service"]:
service_names = [service.split("/")[-1] for service in service_arns]
services_found = []
for service in self.module.params["service"]:
if service in service_names or service in service_arns:
services_found.append(service)
cluster_services[cluster] = services_found
else:
cluster_services[cluster] = service_arns
return cluster_services


def chunks(l, n):
"""Yield successive n-sized chunks from l."""
Expand All @@ -215,7 +253,7 @@ def main():
argument_spec = dict(
details=dict(type="bool", default=False),
events=dict(type="bool", default=True),
cluster=dict(),
cluster=dict(type="list", elements="str"),
service=dict(type="list", elements="str", aliases=["name"]),
)

Expand All @@ -224,18 +262,20 @@ def main():
show_details = module.params.get("details")

task_mgr = EcsServiceManager(module)
cluster_services = task_mgr.get_cluster_services()

if show_details:
if module.params["service"]:
services = module.params["service"]
else:
services = task_mgr.list_services(module.params["cluster"])["services"]
ecs_info = dict(services=[], services_not_running=[])
for chunk in chunks(services, 10):
running_services, services_not_running = task_mgr.describe_services(module.params["cluster"], chunk)
ecs_info["services"].extend(running_services)
ecs_info["services_not_running"].extend(services_not_running)
for cluster, services in cluster_services.items():
for chunk in chunks(services, 10):
running_services, services_not_running = task_mgr.describe_services(cluster, chunk)
ecs_info["services"].extend(running_services)
ecs_info["services_not_running"].extend(services_not_running)
else:
ecs_info = task_mgr.list_services(module.params["cluster"])
service_arns = []
for service_list in cluster_services.values():
service_arns.extend(service_list)
ecs_info = dict(services=service_arns)

module.exit_json(changed=False, **ecs_info)

Expand Down
24 changes: 18 additions & 6 deletions tests/integration/targets/ecs_cluster/tasks/20_ecs_service.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,7 @@

- name: obtain facts for existing service in the cluster
ecs_service_info:
cluster: "{{ ecs_cluster_name }}"
service: "{{ ecs_service_name }}"
service: "{{ ecs_service_name }}2"
details: true
events: false
register: ecs_service_info
Expand All @@ -423,7 +422,7 @@
- name: assert that non-existent service is missing
assert:
that:
- "ecs_service_info.services_not_running[0].reason == 'MISSING'"
- "ecs_service_info.services|length == 0"

- name: obtain specific ECS service facts
ecs_service_info:
Expand All @@ -437,6 +436,19 @@
that:
- "'networkConfiguration' in ecs_service_info.services[0]"

- name: obtain facts for all clusters
ecs_service_info:
details: true
events: false
register: ecs_service_info

- name: assert that service info is available
assert:
that:
- "ecs_service_info.services|length == 3"
- "ecs_service_info.services_not_running|length == 0"
- "ecs_cluster_name in ecs_service_info.services[0].clusterArn"

- name: attempt to get facts from missing task definition
ecs_taskdefinition_info:
task_definition: "{{ ecs_task_name }}-vpc:{{ ecs_task_definition.taskdefinition.revision + 1}}"
Expand Down Expand Up @@ -547,17 +559,17 @@

>> "rolloutStateReason": "ECS deployment ecs-svc/5156684577543126023 in progress.",
constraints and placement strategies are only changeable if the rollout state is "COMPLETED"

a) ecs_service has currently no waiter function. so this is a DIY waiter
b) the state reached never "COMPLETED" because something if wrong with the ECS EC2 Instances
or the network setup. The EC2 instance never arrived as an active instance in the cluster.

>> no container instance met all of its requirements. Reason: No Container Instances were found in your cluster.
>> For more information, see the Troubleshooting section of the Amazon ECS Developer Guide.
>> ec2_instance networking does not work correctly, no instance available for the cluster

Because all of this, all following tasks, that test the change of a constraint or placement stragegy are
using `force_new_deployment: true`. That ignores a) and b).
using `force_new_deployment: true`. That ignores a) and b).
ignore_errors: true
ecs_service_info:
name: "{{ ecs_service_name }}-constraint"
Expand Down
Loading