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

[WIP] add algorithm_type option for elb_target_group #282

Closed
wants to merge 7 commits into from
Closed

[WIP] add algorithm_type option for elb_target_group #282

wants to merge 7 commits into from

Conversation

markuman
Copy link
Member

SUMMARY

Add option algorithm_type for elb_target_group

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

elb_target_group

@markuman
Copy link
Member Author

d'oh

The following attributes are supported only if the load balancer is an Application Load Balancer and the target is an instance or an IP address

@tremble tremble self-requested a review October 27, 2020 19:27
@ansibullbot
Copy link

@ansibullbot ansibullbot added affects_2.10 feature This issue/PR relates to a feature request integration tests/integration module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR needs_triage plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging tests tests labels Nov 16, 2020
@markuman markuman changed the title add algorithm_type option for elb_target_group [WIP] add algorithm_type option for elb_target_group Dec 2, 2020
@ansibullbot ansibullbot added WIP Work in progress has_issue labels Jan 13, 2021
@sakar97
Copy link
Contributor

sakar97 commented Feb 2, 2021

d'oh

The following attributes are supported only if the load balancer is an Application Load Balancer and the target is an instance or an IP address

Hi @markuman, did you find any workarounds? I looked at your shippable build failed error and I noticed that it display aws_access_key in the debug, whereas aws_secret_key is displaying No_log. I am not sure if it is normal and if it's not then it should be fixed soon.
Thanks.

@tremble
Copy link
Contributor

tremble commented Mar 24, 2021

@sakar97 the access_key is approximately a username, Amazon don't treat it as a secret so we don't.

@tremble tremble removed their request for review March 24, 2021 14:13
@markuman
Copy link
Member Author

markuman commented Mar 26, 2021

d'oh

The following attributes are supported only if the load balancer is an Application Load Balancer and the target is an instance or an IP address

Hi @markuman, did you find any workarounds?

The target group itself does not know if it is attached to an ALB, ELB Classic or NLB imho.
We can exclude NLB to match the attribute thing if target type is instance or ip and the protocol is http or https.
But thant the elb_target_group can still fail if it is attached to a classic ELB.

Basically it is implemented like that: https://github.com/ansible-collections/community.aws/pull/282/files#diff-2c1c6b6915c4678f611809c13fca7baf3b17b239efd502403f050f52d2f03eafR756

That means, we must do more requests to verify where the target group is attached to.

@markuman
Copy link
Member Author

closing for the benefit of #1016

@markuman markuman closed this Mar 25, 2022
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
* Vendor ipaddress.py module util

It had been previously vendored in ansible.netcommon but dropped in 2.0
This is needed for ec2_group until we can switch to python3-only,
for ipv6 support.

Also drop ansible.netcommon dependency.

* Copy sanity ignores from netcommon for ipaddress

* We still need netcommon in tests, for ipaddr filter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 feature This issue/PR relates to a feature request has_issue integration tests/integration module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging tests tests WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants