Merge lp:~veebers/juju-ci-tools/ec2_client_handle_when_None into lp:juju-ci-tools

[Christopher Lee]

It's possible that the creation of the EC2Connection client fails and is None.

It's possible that the creation of the EC2Connection client fails and is None, this causes an issue further down the track when we attempt to use that client and it's only then discovered that it's None.

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

=== modified file 'substrate.py'
--- substrate.py	2017-03-29 17:44:24 +0000
+++ substrate.py	2017-04-04 05:00:37 +0000
@@ -121,7 +121,14 @@
         client = ec2.connect_to_region(
             region, aws_access_key_id=euca_environ['EC2_ACCESS_KEY'],
             aws_secret_access_key=euca_environ['EC2_SECRET_KEY'])
-        yield cls(euca_environ, region, client)
+        # There is no point constructing a AWSAccount if client is None.
+        # It can't do anything.
+        if client is None:
+            log.info(
+                'Failed to create ec2 client for region: {}.'.format(region))
+            yield None
+        else:
+            yield cls(euca_environ, region, client)
 
     def __init__(self, euca_environ, region, client):
         self.euca_environ = euca_environ

=== modified file 'tests/test_industrial_test.py'
--- tests/test_industrial_test.py	2017-02-22 18:02:19 +0000
+++ tests/test_industrial_test.py	2017-04-04 05:00:37 +0000
@@ -2149,8 +2149,9 @@
     def test_make_substrate_no_requirements(self):
         client = ModelClient(get_aws_juju_data(), '', '')
         client.env.credentials = {'credentials': {'aws': {'creds': {}}}}
-        with make_substrate_manager(client, []) as substrate:
-            self.assertIs(type(substrate), AWSAccount)
+        with patch('substrate.ec2.connect_to_region', autospec=True):
+            with make_substrate_manager(client, []) as substrate:
+                self.assertIs(type(substrate), AWSAccount)
 
     def test_make_substrate_manager_unsatisifed_requirements(self):
         client = ModelClient(get_aws_juju_data(), '', '')
@@ -2164,14 +2165,16 @@
     def test_make_substrate_satisfied_requirements(self):
         client = ModelClient(get_aws_juju_data(), '', '')
         client.env.credentials = {'credentials': {'aws': {'creds': {}}}}
-        with make_substrate_manager(
-                client, ['iter_security_groups']) as substrate:
-            self.assertIs(type(substrate), AWSAccount)
-        with make_substrate_manager(
-                client, ['iter_security_groups',
-                         'iter_instance_security_groups']
-                ) as substrate:
-            self.assertIs(type(substrate), AWSAccount)
+        with patch('substrate.ec2.connect_to_region', autospec=True):
+            with make_substrate_manager(
+                    client, ['iter_security_groups']) as substrate:
+                self.assertIs(type(substrate), AWSAccount)
+            with make_substrate_manager(
+                    client, [
+                        'iter_security_groups',
+                        'iter_instance_security_groups']
+                    ) as substrate:
+                self.assertIs(type(substrate), AWSAccount)
 
 
 class TestStageInfo(TestCase):

=== modified file 'tests/test_substrate.py'
--- tests/test_substrate.py	2017-03-29 17:53:54 +0000
+++ tests/test_substrate.py	2017-04-04 05:00:37 +0000
@@ -286,20 +286,28 @@
 class TestAWSAccount(TestCase):
 
     def test_from_boot_config(self):
-        with AWSAccount.from_boot_config(SimpleEnvironment('foo', {
-                'type': 'aws',
-                'access-key': 'skeleton',
-                'region': 'france',
-                'secret-key': 'hoover',
-                })) as aws:
-            self.assertEqual(aws.euca_environ, {
-                'AWS_ACCESS_KEY': 'skeleton',
-                'AWS_SECRET_KEY': 'hoover',
-                'EC2_ACCESS_KEY': 'skeleton',
-                'EC2_SECRET_KEY': 'hoover',
-                'EC2_URL': 'https://france.ec2.amazonaws.com',
-                })
-            self.assertEqual(aws.region, 'france')
+        with patch('substrate.ec2.connect_to_region', autospec=True):
+            with AWSAccount.from_boot_config(SimpleEnvironment('foo', {
+                    'type': 'aws',
+                    'access-key': 'skeleton',
+                    'region': 'france',
+                    'secret-key': 'hoover',
+                    })) as aws:
+                self.assertEqual(aws.euca_environ, {
+                    'AWS_ACCESS_KEY': 'skeleton',
+                    'AWS_SECRET_KEY': 'hoover',
+                    'EC2_ACCESS_KEY': 'skeleton',
+                    'EC2_SECRET_KEY': 'hoover',
+                    'EC2_URL': 'https://france.ec2.amazonaws.com',
+                    })
+                self.assertEqual(aws.region, 'france')
+
+    def test_client_construction_failure_returns_None(self):
+        with patch(
+                'substrate.ec2.connect_to_region',
+                autospec=True, return_value=None):
+            with AWSAccount.from_boot_config(get_aws_env()) as aws:
+                self.assertIsNone(aws)
 
     def test_iter_security_groups(self):
 
@@ -1403,16 +1411,17 @@
 
     def test_make_substrate_manager_aws(self):
         boot_config = get_aws_env()
-        with make_substrate_manager(boot_config) as aws:
-            self.assertIs(type(aws), AWSAccount)
-            self.assertEqual(aws.euca_environ, {
-                'AWS_ACCESS_KEY': 'skeleton-key',
-                'AWS_SECRET_KEY': 'secret-skeleton-key',
-                'EC2_ACCESS_KEY': 'skeleton-key',
-                'EC2_SECRET_KEY': 'secret-skeleton-key',
-                'EC2_URL': 'https://ca-west.ec2.amazonaws.com',
-                })
-            self.assertEqual(aws.region, 'ca-west')
+        with patch('substrate.ec2.connect_to_region', autospec=True):
+            with make_substrate_manager(boot_config) as aws:
+                self.assertIs(type(aws), AWSAccount)
+                self.assertEqual(aws.euca_environ, {
+                    'AWS_ACCESS_KEY': 'skeleton-key',
+                    'AWS_SECRET_KEY': 'secret-skeleton-key',
+                    'EC2_ACCESS_KEY': 'skeleton-key',
+                    'EC2_SECRET_KEY': 'secret-skeleton-key',
+                    'EC2_URL': 'https://ca-west.ec2.amazonaws.com',
+                    })
+                self.assertEqual(aws.region, 'ca-west')
 
     def test_make_substrate_manager_openstack(self):
         boot_config = get_os_boot_config()