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

VPC NAT Gateways limit is per-AZ #214

Closed
jantman opened this issue Oct 18, 2016 · 5 comments
Closed

VPC NAT Gateways limit is per-AZ #214

jantman opened this issue Oct 18, 2016 · 5 comments
Assignees
Milestone

Comments

@jantman
Copy link
Owner

jantman commented Oct 18, 2016

The VPC "NAT Gateways" limit merged in #204 and released in 0.5.1 is actually per-AZ, not per-region. That code will need to be changed to add usage for each AZ separately.

This is considered a bug, as the limit isn't being evaluated properly.

The AWS docs call this limit "NAT gateways per Availability Zone" and say, "A NAT gateway in the pending, active, or deleting state counts against your limit." (default of 5, per AZ).

I'm torn on whether I should just fix the calculation issue and release this as 0.5.2, or fix the calculation and rename the limit from "NAT gateways" to "NAT gateways per Availability Zone", which would require holding it for 0.6.0.

@jantman jantman added this to the 0.5.2 milestone Oct 18, 2016
@jantman jantman mentioned this issue Oct 18, 2016
12 tasks
@hltbra
Copy link
Contributor

hltbra commented Oct 18, 2016

If you fix the bug the way I think you will (specifying a resource_id per AZ), the get_current_limit() will return the same value, but get_current_usage() will return a list of 1 or more elements, one per AZ, correct (right now it's always 1 element)?
Would you consider this as backward incompatible? If so, I'd say go with 0.6.0 anyway (and rename it). If not, I'd go with 0.5.2.

Sorry to create this problem for you.

@hltbra
Copy link
Contributor

hltbra commented Oct 18, 2016

Similarly, I noticed that the metrics such as "Max spot instance requests per region" are implemented as a single value, instead of listing the usage per region.

>>> c = AwsLimitChecker(region='us-east-1')
>>> c.find_usage("EC2", use_ta=False)
No handlers could be found for logger "awslimitchecker.services.ec2"
>>> ec2_limits = c.get_limits("EC2", use_ta=False)
>>> ec2_limits['EC2']['Max spot instance requests per region'].get_current_usage()
[<awslimitchecker.limit.AwsLimitUsage object at 0x10f5cb390>]
>>> ec2_limits['EC2']['Max spot instance requests per region'].get_current_usage()[0].resource_id is None
True

Would you consider this a bug?

@jantman
Copy link
Owner Author

jantman commented Oct 23, 2016

@hltbra
My gut reaction is to just call this 0.6.0 and change the name.

"Max spot instance requests per region" is implemented as a single value because awslimitchecker does not have the capability to check multiple regions; each AWSLimitChecker instance connects to a single region's endpoint, so there's no case where we can ever have data for multiple regions.

@jantman jantman added the ready label Oct 23, 2016
@jantman jantman modified the milestones: 0.5.2, 0.6.0 Oct 26, 2016
@jantman jantman added in progress and removed ready labels Nov 10, 2016
jantman added a commit that referenced this issue Nov 10, 2016
@jantman jantman mentioned this issue Nov 10, 2016
@jantman jantman self-assigned this Nov 10, 2016
jantman added a commit that referenced this issue Nov 10, 2016
@jantman
Copy link
Owner Author

jantman commented Nov 10, 2016

Fix merged to develop in #224

@jantman
Copy link
Owner Author

jantman commented Nov 12, 2016

This has been released in 0.6.0 and is now live on PyPI

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

No branches or pull requests

2 participants