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

ecs_service - support setting deployment controller on a service #340

Conversation

berenddeboer
Copy link
Contributor

SUMMARY

Support setting platform version to 1.4.0 (LATEST is 1.3.0) and deployment controller.

The first allows access to new 1.4.0 features. The second change allows you to create a service that can be controlled with Code Deploy.

Example:

  - name: create a Fargate service
    community.aws.ecs_service:
      state: present
      name: "my-service"
      cluster: "my-cluster"
      platform_version: 1.4.0
      task_definition: "my-task"
      desired_count: "1"
      launch_type: FARGATE
      scheduling_strategy: REPLICA
      deployment_controller:
        type: CODE_DEPLOY
      load_balancers:
        - targetGroupArn: "arn:..."
          containerName: example
          containerPort: 80
      network_configuration:
        subnets:
          - "{{vpc_zone_a.subnet.id}}"
          - "{{vpc_zone_b.subnet.id}}"
        security_groups:
          - "sg-example"
        assign_public_ip: true

This fixes #338.

ISSUE TYPE
  • Feature Pull Request

@ansibullbot
Copy link

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) labels Jan 13, 2021
@alinabuzachis
Copy link
Contributor

Hi @berenddeboer, thank you for your contribution. The platform_version has been added in #353. Would you like to work with the other PR #353 to have both features integrated. Otherwise, the merge of the first PR will raise a conflict with the second one.

Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @berenddeboer, could you also please add a changelog fragment?

plugins/modules/ecs_service.py Show resolved Hide resolved
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.

Thanks for taking the time to raise this PR.

In addition to the changelog, I'm not sure that always adding the two parameters is the right behaviour.

placement_constraints, placement_strategy, health_check_grace_period_seconds,
network_configuration, service_registries, launch_type, scheduling_strategy):

params = dict(
cluster=cluster_name,
serviceName=service_name,
platformVersion=platform_version,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to cause an exception when platform_version isn't set (it'll default to None, which Boto3 doesn't like.)

taskDefinition=task_definition,
loadBalancers=load_balancers,
clientToken=client_token,
role=role,
deploymentController=deployment_controller,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it's possible to change existing services but blindly setting this here would add new behaviour wiping out any manual configuration of a deployment controller (people sometimes work around modules not having features by using the AWS cli to tweak things.)

plugins/modules/ecs_service.py Show resolved Hide resolved
plugins/modules/ecs_service.py Outdated Show resolved Hide resolved
@berenddeboer
Copy link
Contributor Author

Hi @berenddeboer, thank you for your contribution. The platform_version has been added in #353. Would you like to work with the other PR #353 to have both features integrated. Otherwise, the merge of the first PR will raise a conflict with the second one.

It seems #353 is good to go, so if that gets merged verse, I can resolve any conflicts.

@ansibullbot
Copy link

@berenddeboer this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html labels Apr 4, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
@tremble tremble force-pushed the support-new-ecs-fargate-functionality branch from 7074613 to 7f0b51d Compare July 3, 2022 15:08
@github-actions
Copy link

github-actions bot commented Jul 3, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor labels Jul 3, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

✔️ ansible-galaxy-importer SUCCESS in 4m 01s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 07s
ansible-test-sanity-docker-devel FAILURE in 11m 23s (non-voting)
ansible-test-sanity-docker-milestone FAILURE in 9m 05s
ansible-test-sanity-docker-stable-2.12 FAILURE in 10m 09s
ansible-test-sanity-docker-stable-2.13 FAILURE in 10m 42s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 49s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 49s
✔️ ansible-test-splitter SUCCESS in 2m 37s
✔️ integration-community.aws-1 SUCCESS in 5m 22s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

✔️ ansible-galaxy-importer SUCCESS in 4m 09s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 40s
ansible-test-sanity-docker-devel FAILURE in 11m 27s (non-voting)
ansible-test-sanity-docker-milestone FAILURE in 9m 26s
ansible-test-sanity-docker-stable-2.12 FAILURE in 9m 21s
ansible-test-sanity-docker-stable-2.13 FAILURE in 10m 28s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 35s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 5m 58s
✔️ ansible-test-splitter SUCCESS in 2m 35s
✔️ integration-community.aws-1 SUCCESS in 5m 46s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@tremble tremble changed the title specify platform version and deployment controller on a service ecs_service - support setting deployment controller on a service Jul 3, 2022
@tremble tremble requested a review from jillr July 3, 2022 18:38
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.

I've trimmed this back to just the deployment controller. For the sake of clearing things out I think we skip the integration test requirement.

Unsupported ecs_service tests pass

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 06s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 15s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 26s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 41s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 35s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 51s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 00s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 10s
✔️ ansible-test-splitter SUCCESS in 2m 29s
✔️ integration-community.aws-1 SUCCESS in 5m 56s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@markuman markuman added the backport-4 PR should be backported to the stable-4 branch label Jul 4, 2022
@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Jul 4, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 3m 58s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 59s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 30s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 11m 01s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 23s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 9m 08s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 7m 46s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 12s
✔️ ansible-test-splitter SUCCESS in 2m 55s
✔️ integration-community.aws-1 SUCCESS in 5m 48s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit c9b1b02 into ansible-collections:main Jul 4, 2022
@patchback
Copy link

patchback bot commented Jul 4, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/c9b1b02eb78c62eeccb3a96a597ece8e7a3d6038/pr-340

Backported as #1303

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jul 4, 2022
ecs_service - support setting deployment controller on a service

SUMMARY
Support setting platform version to 1.4.0 (LATEST is 1.3.0) and deployment controller.
The first allows access to new 1.4.0 features. The second change allows you to create a service that can be controlled with Code Deploy.
Example:
  - name: create a Fargate service
    community.aws.ecs_service:
      state: present
      name: "my-service"
      cluster: "my-cluster"
      platform_version: 1.4.0
      task_definition: "my-task"
      desired_count: "1"
      launch_type: FARGATE
      scheduling_strategy: REPLICA
      deployment_controller:
        type: CODE_DEPLOY
      load_balancers:
        - targetGroupArn: "arn:..."
          containerName: example
          containerPort: 80
      network_configuration:
        subnets:
          - "{{vpc_zone_a.subnet.id}}"
          - "{{vpc_zone_b.subnet.id}}"
        security_groups:
          - "sg-example"
        assign_public_ip: true

This fixes #338.
ISSUE TYPE

Feature Pull Request

Reviewed-by: Jill R <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: Markus Bergholz <[email protected]>
(cherry picked from commit c9b1b02)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jul 4, 2022
… (#1303)

[PR #340/c9b1b02e backport][stable-4] ecs_service - support setting deployment controller on a service

This is a backport of PR #340 as merged into main (c9b1b02).
SUMMARY
Support setting platform version to 1.4.0 (LATEST is 1.3.0) and deployment controller.
The first allows access to new 1.4.0 features. The second change allows you to create a service that can be controlled with Code Deploy.
Example:
  - name: create a Fargate service
    community.aws.ecs_service:
      state: present
      name: "my-service"
      cluster: "my-cluster"
      platform_version: 1.4.0
      task_definition: "my-task"
      desired_count: "1"
      launch_type: FARGATE
      scheduling_strategy: REPLICA
      deployment_controller:
        type: CODE_DEPLOY
      load_balancers:
        - targetGroupArn: "arn:..."
          containerName: example
          containerPort: 80
      network_configuration:
        subnets:
          - "{{vpc_zone_a.subnet.id}}"
          - "{{vpc_zone_b.subnet.id}}"
        security_groups:
          - "sg-example"
        assign_public_ip: true

This fixes #338.
ISSUE TYPE

Feature Pull Request

Reviewed-by: Mark Chappell <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4 PR should be backported to the stable-4 branch community_review feature This issue/PR relates to a feature request mergeit Merge the PR (SoftwareFactory) module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

community.aws.ecs_service should support deploymentController parameter
6 participants