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

Discrepancies between Service Quota QuotaName and awslimitchecker quotas_name #467

Closed
xRokco opened this issue May 14, 2020 · 3 comments
Closed

Comments

@xRokco
Copy link

xRokco commented May 14, 2020

Bug Report

When reporting a bug in awslimitchecker, please provide all of the following information,
as well as any additional details that may be useful in reproducing or fixing
the issue:

Version

Python, 8.0.2

Installation Method

pip

Supporting Software Versions

➜ python --version
Python 3.7.5

Actual Output

print(limit.quota_name)
➜ NAT Gateways per AZ

Expected Output

print(limit.quota_name)
➜ NAT gateways per Availability Zone

I've noticed some naming discrepancies between what awslimitchecker names limits and what they are known as within the Service Quotas service. The above is an example of one.

Is it intended that the names awslimitchecker uses and Service Quotas uses are identical, or is that not considered important?

My use case here is writing a job which preforms limit checks and then, where possible, requests a limit increase through Service Quotas automatically. In order to request an increase you need to pass the API call the QuotaCode (L-FE5A380F in the case of "NAT gateways per Availability Zone"). To get the quota code I just get the quota name from awslimitchecker and do a list_service_quotas and match the quota names, but it fails in this case (and possibly others).

Looking into the code I can see that each limit can include an explicit quotas_name property, for example -

limits['Internet gateways'] = AwsLimit(
            'Internet gateways',
            self,
            5,
            self.warning_threshold,
            self.critical_threshold,
            limit_type='AWS::EC2::InternetGateway',
            quotas_name='Internet gateways per Region'
        )

Is this what this property is designed for? When the "canonical" name and the name AWS uses are slightly different? If so I will just open a pull request which includes a quotas_name for limits with naming discrepancies I find.

Alternatively, would it be a good idea to return the QuotaCode directly as another limit property? From my investigation it looks like these IDs are static across regions and accounts, so it might be worthwhile including and returning them.

Testing Assistance

of course

@jantman
Copy link
Owner

jantman commented May 14, 2020

@xRokco tl;dr: yes, the quotas_name argument on the AwsLimit constructor is for this case. I believe the PR is needed, and it appears that the NAT Gateways per AZ limit was not exposed by Service Quotas when I originally implemented Quotas support. Since the Quota and our internal AwsLimit don't share the same name, the Quota isn't being auto-detected and needs a manual override of the name.

If you look at the public API of the awslimitchecker.limit.AwsLimit class, you'll see that it has three properties exposing the information used for Service Quotas: quota_name, quotas_service_code, and quotas_unit.

The code behind the quota_name property is:

        if self._quotas_name is not None:
            return self._quotas_name
        return self.name

so if there's an explicit quotas_name passed in, that's used, or else the limit name is used.

So there's a lot of history here, and it's a sore point for me.

When awslimitchecker was originally written back in 2015, Trusted Advisor wasn't a thing, so all limits had hard-coded defaults that came from the published documentation. The limits were also named following that public documentation. When Trusted Advisor came out and we could finally get some limit data programmatically, some of the limits were named identically to the public docs and some weren't... so that was the first level (ta_name) of indirection. There have been a few major rewrites of the AWS docs on service limits since then, and some of the names in the docs have changed.

I implemented the Service Quotas support in late December 2019 and, at that time, all of the limits listed in our docs as having Quotas support were retrieving data from Service Quotas correctly. If there are now naming discrepancies between what awslimitchecker thinks and what Service Quotas thinks, the possibilities are that either a name changed in Service Quotas (I hope that's not the case) or that it's a Quota that wasn't present 5 months ago (which, I hope, is more likely given the third point below).

Regarding not using QuotaCode, my logic had three main points:

  1. Frankly, it's tedius. I'd need to look up the QuotaCode of every limit we support and add them all to awslimitchecker, and keep doing that as Service Quotas rolls out support for more Quotas.
  2. While the QuotaCodes seemed to be identical across a handful of accounts that I tested, I couldn't find any AWS documentation making a guarantee of this, so I was worried about it breaking in really difficult-to-reproduce ways.
  3. The way it exists right now, if a new Service Quota is introduced and it matches the name of an existing AwsLimit in awslimitchecker, it will be "detected" and used on-the-fly with no code changes. This seemed to work in the majority of cases, so I figured it would be an easy way to support new Quotas without having to release a new version of awslimitchecker every time one is added.

Looking at the Supported Limits - vpc documentation, which was last auto-generated on December 28, 2019, that does not show the NAT Gateways per AZ limit as being exposed by Quotas. This leads me to believe that NAT Gateways per AZ has been newly exposed via Service Quotas, and it's not being auto-detected by awslimitchecker because of the naming mismatch.

@xRokco
Copy link
Author

xRokco commented May 15, 2020

Thanks for the response @jantman

Your reasoning for not including the quota code makes sense. Points #1 and #2 are what was stopping me from moving forward with this, but point #3 I hadn't considered and is very relevant.

The only other potential consideration would be to return the quota_code from the list_service_quotas API call that is made in quotas.py, however list_service_quotas only returns limits managed by ServiceQuotas that have had their quota increased at some point, so would only include a fairly small subset of limits with quota codes for most accounts.

I'll try to find any more limits with naming discrepancies and open a PR to have them updated here.

@jantman
Copy link
Owner

jantman commented Sep 22, 2020

This has been fixed in 9.0.0, which is now live on PyPI and on the Docker Hub. Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants