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

Jump to bottom

Added assignPublicIp param in network_configuration #395

Merged
merged 9 commits into from
Mar 29, 2021
Merged

Added assignPublicIp param in network_configuration #395

merged 9 commits into from
Mar 29, 2021

Conversation

sakar97
Copy link
Contributor

@sakar97 sakar97 commented Feb 6, 2021 •
edited by goneri
Loading

SUMMARY

Closes: #319
Added assignPublicIp support in network_configuration to the existing ecs_task module.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • ecs_task
ADDITIONAL INFORMATION
- name: RUN a task on Fargate with public ip assigned
  community.aws.ecs_task:
      operation: run
      count: 2
      cluster: console-sample-app-static-cluster
      task_definition: console-sample-app-static-taskdef
      task: "arn:aws:ecs:us-west-2:172139249013:task/3f8353d1-29a8-4689-bbf6-ad79937ffe8a"
      started_by: ansible_user
      launch_type: FARGATE
      network_configuration:
        assign_public_ip: yes
        subnets:
        - subnet-abcd1234
  register: task_output

@ansibullbot
Copy link

cc @Java1Guy @jillr @s-hertel @tremble @wimnat
click here for bot help

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request module module needs_triage plugins plugin (any type) needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Feb 6, 2021
@goneri
Copy link
Member

goneri commented Mar 17, 2021

Hi @sakar97,

Thank you for the PR. Before we can merge it, can you please modify https://github.com/ansible-collections/community.aws/tree/main/tests/integration/targets/ecs_cluster/tasks to test this new behaviour?

@goneri goneri mentioned this pull request Mar 17, 2021
Closed
@goneri goneri requested review from tremble and jillr and removed request for tremble March 17, 2021 12:55
@tremble
Copy link
Contributor

tremble commented Mar 17, 2021 •
edited
Loading

Note to anyone reviewing: the ecs_cluster integration tests are currently not run automatically as they're slow enough to cause problems in CI (at this time)

tremble
tremble requested changes Mar 17, 2021
View reviewed changes
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

As already requested, please add an integration test. More information about our integration test suites can be found in https://www.ansible.com/blog/getting-started-with-aws-ansible-module-development

plugins/modules/ecs_task.py Show resolved Hide resolved
plugins/modules/ecs_task.py Outdated Show resolved Hide resolved
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Mar 17, 2021
@sakar97
Copy link
Contributor Author

sakar97 commented Mar 20, 2021

Hi, I totally forgot the Integration test, I will add the required test cases for the above feature. Thanks

@ansibullbot ansibullbot added community_review integration tests/integration tests tests and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Mar 20, 2021
@sakar97 sakar97 requested a review from tremble March 20, 2021 14:47
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Mar 20, 2021
tremble
tremble requested changes Mar 20, 2021
View reviewed changes
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Minor niggle - currently the version test is a little broken.

plugins/modules/ecs_task.py Outdated Show resolved Hide resolved
plugins/modules/ecs_task.py Outdated Show resolved Hide resolved
@sakar97 sakar97 requested a review from tremble March 20, 2021 15:53
@ansibullbot ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Mar 21, 2021
tremble
tremble approved these changes Mar 21, 2021
View reviewed changes
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Tests passing locally.

@tremble tremble merged commit 152c8db into ansible-collections:main Mar 29, 2021
@sakar97 sakar97 deleted the 319 branch March 30, 2021 12:35
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@tremble
Added assignPublicIp param in network_configuration (ansible-collecti…
5300ce1 
…ons#395)

* added assign_public_ip feature
* fix sanity issues and added changelog

Co-authored-by: Mark Chappell <[email protected]>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@tremble
Added assignPublicIp param in network_configuration (ansible-collecti…
581343d 
…ons#395)

* added assign_public_ip feature
* fix sanity issues and added changelog

Co-authored-by: Mark Chappell <[email protected]>
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
@tremble @danielcotton
Added assignPublicIp param in network_configuration (ansible-collecti…
3076602 
…ons#395)

* added assign_public_ip feature
* fix sanity issues and added changelog

Co-authored-by: Mark Chappell <[email protected]>
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
@alinabuzachis
ec2_instance - Support throughtput parameter for GP3 volume types (an…
cc94310 
…sible-collections#433)

ec2_instance -  Support throughtput parameter for GP3 volume types

SUMMARY

ec2_instance - Support throughput parameter for GP3 volume types
Fixes ansible-collections#395
ISSUE TYPE


Feature Pull Request

COMPONENT NAME

ec2_instance

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Mark Chappell <None>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: Jill R <None>
Reviewed-by: None <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tremble tremble tremble approved these changes

@jillr jillr Awaiting requested review from jillr

Assignees
No one assigned
Labels
community_review feature This issue/PR relates to a feature request has_issue integration tests/integration module module plugins plugin (any type) tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ecs_task doesn't have assignPublicIp param in network_configuration
4 participants
@sakar97 @ansibullbot @goneri @tremble