Merge pull request #340 from /issues/339

fixes #339 - 4.0.1 release

CHANGES.rst

@@ -1,6 +1,14 @@
 Changelog
 =========
 
+4.0.1 (2018-03-09)
+------------------
+
+This is a minor bugfix release for a few issues that users have reported recently.
+
+* Fix `Issue #337 <https://github.com/jantman/awslimitchecker/issues/337>`_ where sometimes an account even with Business-level support will not have a Trusted Advisor result for the Service Limits check, and will return a result with ``status: not_available`` or a missing ``flaggedResources`` key.
+* Fix `Issue #335 <https://github.com/jantman/awslimitchecker/issues/335>`_ where runs against the EFS service in certain unsupported regions result in either a connection timeout or an AccessDeniedException.
+
 4.0.0 (2018-02-17)
 ------------------
 

awslimitchecker/services/efs.py

@@ -39,7 +39,8 @@
 
 import abc  # noqa
 import logging
-from botocore.exceptions import EndpointConnectionError
+from botocore.exceptions import EndpointConnectionError, ClientError
+from botocore.vendored.requests.exceptions import ConnectTimeout
 
 from .base import _AwsService
 from ..limit import AwsLimit
@@ -65,7 +66,7 @@ def find_usage(self):
             lim._reset_usage()
         try:
             self._find_usage_filesystems()
-        except EndpointConnectionError as ex:
+        except (EndpointConnectionError, ClientError, ConnectTimeout) as ex:
             logger.warning(
                 'Caught exception when trying to use EFS ('
                 'perhaps the EFS service is not available in this '

awslimitchecker/tests/services/test_efs.py

@@ -40,7 +40,8 @@
 import sys
 from awslimitchecker.services.efs import _EfsService
 from awslimitchecker.limit import AwsLimit
-from botocore.exceptions import EndpointConnectionError
+from botocore.exceptions import EndpointConnectionError, ClientError
+from botocore.vendored.requests.exceptions import ConnectTimeout
 
 # https://code.google.com/p/mock/issues/detail?id=249
 # py>=3.4 should use unittest.mock not the mock package on pypi
@@ -145,6 +146,63 @@ def test_find_usage_no_endpoint(self):
         usage = cls.limits['File systems'].get_current_usage()
         assert len(usage) == 0
 
+    def test_find_usage_access_denied(self):
+        exc = ClientError(
+            {
+                'Error': {
+                    'Code': 'AccessDeniedException',
+                    'Message': 'This account does not have permission '
+                               'to access this service',
+                }
+            },
+            'DescribeFileSystems'
+        )
+        mock_conn = Mock()
+        with patch('%s.connect' % pb) as mock_connect:
+            with patch('%s.paginate_dict' % pbm) as mock_paginate:
+                mock_paginate.side_effect = exc
+                cls = _EfsService(21, 43)
+                cls.conn = mock_conn
+                assert cls._have_usage is False
+                cls.find_usage()
+        assert cls._have_usage is True
+        assert mock_connect.mock_calls == [call()]
+        assert mock_paginate.mock_calls == [
+            call(
+                mock_conn.describe_file_systems,
+                alc_marker_path=['NextMarker'],
+                alc_data_path=['FileSystems'],
+                alc_marker_param='Marker'
+            )
+        ]
+        assert len(cls.limits) == 1
+        usage = cls.limits['File systems'].get_current_usage()
+        assert len(usage) == 0
+
+    def test_find_usage_connect_timeout(self):
+        exc = ConnectTimeout()
+        mock_conn = Mock()
+        with patch('%s.connect' % pb) as mock_connect:
+            with patch('%s.paginate_dict' % pbm) as mock_paginate:
+                mock_paginate.side_effect = exc
+                cls = _EfsService(21, 43)
+                cls.conn = mock_conn
+                assert cls._have_usage is False
+                cls.find_usage()
+        assert cls._have_usage is True
+        assert mock_connect.mock_calls == [call()]
+        assert mock_paginate.mock_calls == [
+            call(
+                mock_conn.describe_file_systems,
+                alc_marker_path=['NextMarker'],
+                alc_data_path=['FileSystems'],
+                alc_marker_param='Marker'
+            )
+        ]
+        assert len(cls.limits) == 1
+        usage = cls.limits['File systems'].get_current_usage()
+        assert len(usage) == 0
+
     def test_required_iam_permissions(self):
         cls = _EfsService(21, 43)
         assert cls.required_iam_permissions() == [

awslimitchecker/tests/test_trustedadvisor.py

@@ -692,6 +692,66 @@ def test_dont_have_ta(self):
         ]
         assert res == {}
 
+    def test_not_available(self):
+        poll_return_val = {
+            'result': {
+                'checkId': 'xxxxxxx',
+                'status': 'not_available'
+            }
+        }
+        with patch('%s._get_limit_check_id' % pb, autospec=True) as mock_id:
+            with patch('%s._get_refreshed_check_result' % pb,
+                       autospec=True) as mock_hr:
+                mock_hr.return_value = poll_return_val
+                mock_id.return_value = (
+                    'foo',
+                    [
+                        'Region',
+                        'Service',
+                        'Limit Name',
+                        'Limit Amount',
+                        'Current Usage',
+                        'Status'
+                    ]
+                )
+                res = self.cls._poll()
+        assert self.mock_conn.mock_calls == []
+        assert mock_id.mock_calls == [call(self.cls)]
+        assert mock_hr.mock_calls == [
+            call(self.cls, 'foo')
+        ]
+        assert res == {}
+
+    def test_no_flagged_resources(self):
+        poll_return_val = {
+            'result': {
+                'checkId': 'xxxxxxx',
+                'timestamp': '2015-06-15T20:27:42Z'
+            }
+        }
+        with patch('%s._get_limit_check_id' % pb, autospec=True) as mock_id:
+            with patch('%s._get_refreshed_check_result' % pb,
+                       autospec=True) as mock_hr:
+                mock_hr.return_value = poll_return_val
+                mock_id.return_value = (
+                    'foo',
+                    [
+                        'Region',
+                        'Service',
+                        'Limit Name',
+                        'Limit Amount',
+                        'Current Usage',
+                        'Status'
+                    ]
+                )
+                res = self.cls._poll()
+        assert self.mock_conn.mock_calls == []
+        assert mock_id.mock_calls == [call(self.cls)]
+        assert mock_hr.mock_calls == [
+            call(self.cls, 'foo')
+        ]
+        assert res == {}
+
 
 class TestGetRefreshedCheckResult(object):
 

awslimitchecker/trustedadvisor.py

@@ -172,6 +172,18 @@ def _poll(self):
         checks = self._get_refreshed_check_result(check_id)
         region = self.ta_region or self.conn._client_config.region_name
         res = {}
+        if checks['result'].get('status', '') == 'not_available':
+            logger.warning(
+                'Trusted Advisor returned status "not_available" for '
+                'service limit check; cannot retrieve limits from TA.'
+            )
+            return {}
+        if 'flaggedResources' not in checks['result']:
+            logger.warning(
+                'Trusted Advisor returned no results for '
+                'service limit check; cannot retrieve limits from TA.'
+            )
+            return {}
         for check in checks['result']['flaggedResources']:
             if 'region' in check and check['region'] != region:
                 continue

awslimitchecker/version.py

@@ -47,7 +47,7 @@
 except ImportError:
     logger.error("Unable to import versionfinder", exc_info=True)
 
-_VERSION_TUP = (4, 0, 0)
+_VERSION_TUP = (4, 0, 1)
 _VERSION = '.'.join([str(x) for x in _VERSION_TUP])
 _PROJECT_URL = 'https://github.com/jantman/awslimitchecker'