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

elb_application_lb_info - Ability to filter results (slow/failed attempts) #1767

Open
1 task done
andreiboost opened this issue Sep 26, 2023 · 6 comments
Open
1 task done
Labels
feature This issue/PR relates to a feature request

Comments

@andreiboost
Copy link

Summary

elb_application_lb_info doesn't have any option to filter/query specific ELBs beyond lookup by ARN/name. I am trying to filter by VPC ID, but this is not possible with this module. For example amazon.aws.ec2_security_group_info supports this exact case, i.e.

- name: Find all Security Groups
  amazon.aws.ec2_security_group_info:
    filters:
      vpc-id: "vpc-12345"

To achieve the same result with ALBs, I have to resort to the AWS CLI, for example:

- name: Get VPC ALBs
  check_mode: false
  changed_when: false
  ansible.builtin.command:
    cmd: >-
      aws elbv2 describe-load-balancers
      --query 'LoadBalancers[?VpcId==`vpc-12345` && Type==`application`]'
      --output json
  register: __cluster_albs_json

- name: Parse ALB output
  ansible.builtin.set_fact:
    cluster_albs: "{{ __cluster_albs_json.stdout | from_json }}"

For my purposes, this works but the output isn't the same nor as complete as elb_application_lb_info. Please consider adding a filters parameter to this module. elb_application_lb_info often fails due to throttling in accounts with many ALBs, i.e.

Failed to describe listener rules: An error occurred (Throttling) when calling the DescribeRules operation (reached max retries: 4): Rate exceeded

Issue Type

Feature Idea

Component Name

elb_application_lb_info

Additional Information

- name: Find all ALBs in VPC
  amazon.aws.elb_application_lb_info:
    filters:
      vpc-id: "vpc-12345"

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@tremble
Copy link
Contributor

tremble commented Sep 26, 2023

Hi @andreiboost,

Thanks for taking the time to submit this feature request. Unfortunately the AWS ELB APIs doesn't support performing a server-side query like this, as such it's unlikely that this would be implemented any time soon.

The workaround you listed using the "query" parameter from awscli is actually performing a JSONPath lookup on the data from all ELBs.

What should be possible is to perform the lookup in pure Ansible rather than the AWS CLI:

- name: Fetch ELB descriptions
  amazon.aws.elb_application_lb_info:
  register: elb_info

- name: Find ALBs in VPC
  set_fact:
    vpc_albs: "{{ 
        elb_info.load_balancers
        | selectattr('vpc_id', 'equalto', desired_vpc)
        | selectattr('type', 'equalto', 'application')
      }}"

(Whitespace in the filter is for legibility, it can be all on a single line)

@andreiboost
Copy link
Author

@tremble interesting, then perhaps the AWS CLI isn't timing out because it doesn't fetch as much stuff? (Listeners, Rules)

@tremble
Copy link
Contributor

tremble commented Sep 26, 2023

perhaps the AWS CLI isn't timing out because it doesn't fetch as much stuff?

Looking at the code, and the error message (Failed to describe listener rules) this is quite possible, automatic retries appear to only be half implemented in the module. Would you be able to see if #1768 at least fixes the error you're seeing (it'll still be slow)?

@andreiboost
Copy link
Author

It appears to fix that problem, but it is very slow. Takes about 90-100 seconds to fetch >100 ALBs.

Would you suggest I open another request for skipping rules and/or listeners? That should significantly speed it up when all I want is their ARNs or names.

@tremble
Copy link
Contributor

tremble commented Sep 26, 2023

Would you suggest I open another request for skipping rules and/or listeners? That should significantly speed it up when all I want is their ARNs or names.

A Pull request would be very much appreciated, but please keep in mind that the default behaviour needs to stay as it is today to avoid breaking existing playbooks.

Long term, I think supporting "filter" before we query the remaining information is probably the best option. Because there are other APIs which also don't support server side filtering (KMS springs to mind) this would be best handled as common code in module_utils that we could apply to other modules too. (An RFE to AWS wouldn't be a bad idea either)

@tremble tremble changed the title elb_application_lb_info - Ability to filter results elb_application_lb_info - Ability to filter results (slow/failed attempts) Sep 26, 2023
@andreiboost
Copy link
Author

andreiboost commented Sep 26, 2023

Understood, I'll see when I get some time to work on a PR. Adding a generic "filter" to utils would be quite a big undertaking for me as I'm not familiar with this codebase but adding options to this module to skip some things is absolutely doable. Later this week or sometime next week should be possible. Thanks for your help (and PR!).

@gravesm gravesm added feature This issue/PR relates to a feature request and removed needs_triage labels Sep 26, 2023
softwarefactory-project-zuul bot pushed a commit that referenced this issue Sep 27, 2023
… retries enabled (#1768)

elb_application_lb_info - ensure queries for additional ALB data have retries enabled

SUMMARY
As per #1767 the queries to pull extra info about ALBs are hitting rate limits when there's a lot of ALBs.  Unfortunately the initial list operation has limited server-side filtering capabilities (and we don't have consistent client side filtering implemented at this time).
Ensure that all of the extra queries have retries with jittered backoff enabled.
Additionally, drops a redundant describe_load_balancers call to retrieve the ip_address_type information.  (added by ansible-collections/community.aws#499)
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
elb_application_lb_info
ADDITIONAL INFORMATION
I don't consider this a full fix for #1767 so I'm not using the "fixes".

Reviewed-by: Alina Buzachis
patchback bot pushed a commit that referenced this issue Sep 27, 2023
… retries enabled (#1768)

elb_application_lb_info - ensure queries for additional ALB data have retries enabled

SUMMARY
As per #1767 the queries to pull extra info about ALBs are hitting rate limits when there's a lot of ALBs.  Unfortunately the initial list operation has limited server-side filtering capabilities (and we don't have consistent client side filtering implemented at this time).
Ensure that all of the extra queries have retries with jittered backoff enabled.
Additionally, drops a redundant describe_load_balancers call to retrieve the ip_address_type information.  (added by ansible-collections/community.aws#499)
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
elb_application_lb_info
ADDITIONAL INFORMATION
I don't consider this a full fix for #1767 so I'm not using the "fixes".

Reviewed-by: Alina Buzachis
(cherry picked from commit d1c5f05)
softwarefactory-project-zuul bot pushed a commit that referenced this issue Sep 28, 2023
… retries enabled (#1768) (#1775)

[PR #1768/d1c5f053 backport][stable-6] elb_application_lb_info - ensure queries for additional ALB data have retries enabled

This is a backport of PR #1768 as merged into main (d1c5f05).
SUMMARY
As per #1767 the queries to pull extra info about ALBs are hitting rate limits when there's a lot of ALBs.  Unfortunately the initial list operation has limited server-side filtering capabilities (and we don't have consistent client side filtering implemented at this time).
Ensure that all of the extra queries have retries with jittered backoff enabled.
Additionally, drops a redundant describe_load_balancers call to retrieve the ip_address_type information.  (added by ansible-collections/community.aws#499)
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
elb_application_lb_info
ADDITIONAL INFORMATION
I don't consider this a full fix for #1767 so I'm not using the "fixes".

Reviewed-by: Mark Chappell
softwarefactory-project-zuul bot pushed a commit that referenced this issue Sep 29, 2023
…1778)

elb_application_lb_info - Add parameters to skip fetching some data

Add include_attributes, include_listeners and include_listener_rules.
SUMMARY
Related to #1767.
This PR adds parameters to the module which disable fetching certain data.
They all default to true for backwards compatibility. In my tests disabling all 3 speeds it up by around 10x when fetching a lot (100+) ALBs.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
elb_application_lb_info
ADDITIONAL INFORMATION
Please let me know if the coupling of include_listeners and include_listener_rules isn't desired. I can add checks to require include_listeners be true if include_listener_rules as an alternative (or something else?).

Reviewed-by: Mark Chappell
alinabuzachis pushed a commit to alinabuzachis/amazon.aws that referenced this issue Oct 2, 2023
… retries enabled (ansible-collections#1768)

elb_application_lb_info - ensure queries for additional ALB data have retries enabled

SUMMARY
As per ansible-collections#1767 the queries to pull extra info about ALBs are hitting rate limits when there's a lot of ALBs.  Unfortunately the initial list operation has limited server-side filtering capabilities (and we don't have consistent client side filtering implemented at this time).
Ensure that all of the extra queries have retries with jittered backoff enabled.
Additionally, drops a redundant describe_load_balancers call to retrieve the ip_address_type information.  (added by ansible-collections/community.aws#499)
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
elb_application_lb_info
ADDITIONAL INFORMATION
I don't consider this a full fix for ansible-collections#1767 so I'm not using the "fixes".

Reviewed-by: Alina Buzachis
alinabuzachis pushed a commit to alinabuzachis/amazon.aws that referenced this issue Oct 2, 2023
…nsible-collections#1778)

elb_application_lb_info - Add parameters to skip fetching some data

Add include_attributes, include_listeners and include_listener_rules.
SUMMARY
Related to ansible-collections#1767.
This PR adds parameters to the module which disable fetching certain data.
They all default to true for backwards compatibility. In my tests disabling all 3 speeds it up by around 10x when fetching a lot (100+) ALBs.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
elb_application_lb_info
ADDITIONAL INFORMATION
Please let me know if the coupling of include_listeners and include_listener_rules isn't desired. I can add checks to require include_listeners be true if include_listener_rules as an alternative (or something else?).

Reviewed-by: Mark Chappell
abikouo pushed a commit to abikouo/amazon.aws that referenced this issue Oct 24, 2023
…ions#1767)

elb_target_group: fix lost property AvailabilityZone

SUMMARY
Closes ansible-collections#1736
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
elb_target_group

Reviewed-by: Mark Chappell
Reviewed-by: Alina Buzachis
Reviewed-by: Markus Bergholz <git@osuv.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request
Projects
None yet
Development

No branches or pull requests

3 participants