From d0c70cc96279d2bf24f30a501b9bf572e40f8e7a Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Sun, 22 Nov 2015 10:55:46 -0500 Subject: [PATCH] Add support for generalized per-region settings Internap creates a public and a private network for each customer for each region on region activation. This means there is a per-region external network that the user may want to specify. Also, conoha has per-region auth-urls. Per-region config is still overridden by argparse or kwargs values. Change-Id: Ie2f3d2ca3ccbe7e3dd674983136b42c323544997 --- README.rst | 32 ++++++++++ os_client_config/config.py | 90 ++++++++++++++++++--------- os_client_config/tests/base.py | 17 ++++- os_client_config/tests/test_config.py | 50 ++++++++++++--- 4 files changed, 146 insertions(+), 43 deletions(-) diff --git a/README.rst b/README.rst index 9850c05ac..ced3b1821 100644 --- a/README.rst +++ b/README.rst @@ -265,6 +265,38 @@ environment variable. The above snippet will tell client programs to prefer returning an IPv4 address. +Per-region settings +------------------- + +Sometimes you have a cloud provider that has config that is common to the +cloud, but also with some things you might want to express on a per-region +basis. For instance, Internap provides a public and private network specific +to the user in each region, and putting the values of those networks into +config can make consuming programs more efficient. + +To support this, the region list can actually be a list of dicts, and any +setting that can be set at the cloud level can be overridden for that +region. + +:: + + clouds: + internap: + profile: internap + auth: + password: XXXXXXXXXXXXXXXXX + username: api-55f9a00fb2619 + project_name: inap-17037 + regions: + - name: ams01 + values: + external_network: inap-17037-WAN1654 + internal_network: inap-17037-LAN4820 + - name: nyj01 + values: + external_network: inap-17037-WAN7752 + internal_network: inap-17037-LAN6745 + Usage ----- diff --git a/os_client_config/config.py b/os_client_config/config.py index 48aa0e2d0..70989bfd7 100644 --- a/os_client_config/config.py +++ b/os_client_config/config.py @@ -15,6 +15,7 @@ # alias because we already had an option named argparse import argparse as argparse_mod +import copy import json import os import warnings @@ -121,8 +122,9 @@ def _merge_clouds(old_dict, new_dict): return ret -def _auth_update(old_dict, new_dict): +def _auth_update(old_dict, new_dict_source): """Like dict.update, except handling the nested dict called auth.""" + new_dict = copy.deepcopy(new_dict_source) for (k, v) in new_dict.items(): if k == 'auth': if k in old_dict: @@ -302,17 +304,29 @@ class OpenStackConfig(object): return self._cache_class def get_cache_arguments(self): - return self._cache_arguments.copy() + return copy.deepcopy(self._cache_arguments) def get_cache_expiration(self): - return self._cache_expiration.copy() + return copy.deepcopy(self._cache_expiration) + + def _expand_region_name(self, region_name): + return {'name': region_name, 'values': {}} + + def _expand_regions(self, regions): + ret = [] + for region in regions: + if isinstance(region, dict): + ret.append(copy.deepcopy(region)) + else: + ret.append(self._expand_region_name(region)) + return ret def _get_regions(self, cloud): if cloud not in self.cloud_config['clouds']: - return [''] + return [self._expand_region_name('')] config = self._normalize_keys(self.cloud_config['clouds'][cloud]) if 'regions' in config: - return config['regions'] + return self._expand_regions(config['regions']) elif 'region_name' in config: regions = config['region_name'].split(',') if len(regions) > 1: @@ -320,22 +334,39 @@ class OpenStackConfig(object): "Comma separated lists in region_name are deprecated." " Please use a yaml list in the regions" " parameter in {0} instead.".format(self.config_filename)) - return regions + return self._expand_regions(regions) else: # crappit. we don't have a region defined. new_cloud = dict() our_cloud = self.cloud_config['clouds'].get(cloud, dict()) self._expand_vendor_profile(cloud, new_cloud, our_cloud) if 'regions' in new_cloud and new_cloud['regions']: - return new_cloud['regions'] + return self._expand_regions(new_cloud['regions']) elif 'region_name' in new_cloud and new_cloud['region_name']: - return [new_cloud['region_name']] + return [self._expand_region_name(new_cloud['region_name'])] else: # Wow. We really tried - return [''] + return [self._expand_region_name('')] - def _get_region(self, cloud=None): - return self._get_regions(cloud)[0] + def _get_region(self, cloud=None, region_name=''): + if not cloud: + return self._expand_region_name(region_name) + + regions = self._get_regions(cloud) + if not region_name: + return regions[0] + + for region in regions: + if region['name'] == region_name: + return region + + raise exceptions.OpenStackConfigException( + 'Region {region_name} is not a valid region name for cloud' + ' {cloud}. Valid choices are {region_list}. Please note that' + ' region names are case sensitive.'.format( + region_name=region_name, + region_list=','.join([r['name'] for r in regions]), + cloud=cloud)) def get_cloud_names(self): return self.cloud_config['clouds'].keys() @@ -585,7 +616,9 @@ class OpenStackConfig(object): for cloud in self.get_cloud_names(): for region in self._get_regions(cloud): - clouds.append(self.get_one_cloud(cloud, region_name=region)) + if region: + clouds.append(self.get_one_cloud( + cloud, region_name=region['name'])) return clouds def _fix_args(self, args, argparse=None): @@ -764,30 +797,27 @@ class OpenStackConfig(object): else: cloud = self.default_cloud - if 'region_name' not in args or args['region_name'] is None: - args['region_name'] = self._get_region(cloud) - config = self._get_base_cloud_config(cloud) + # Get region specific settings + if 'region_name' not in args: + args['region_name'] = '' + region = self._get_region(cloud=cloud, region_name=args['region_name']) + args['region_name'] = region['name'] + region_args = copy.deepcopy(region['values']) + # Regions is a list that we can use to create a list of cloud/region # objects. It does not belong in the single-cloud dict - regions = config.pop('regions', None) - if regions and args['region_name'] not in regions: - raise exceptions.OpenStackConfigException( - 'Region {region_name} is not a valid region name for cloud' - ' {cloud}. Valid choices are {region_list}. Please note that' - ' region names are case sensitive.'.format( - region_name=args['region_name'], - region_list=','.join(regions), - cloud=cloud)) + config.pop('regions', None) # Can't just do update, because None values take over - for (key, val) in iter(args.items()): - if val is not None: - if key == 'auth' and config[key] is not None: - config[key] = _auth_update(config[key], val) - else: - config[key] = val + for arg_list in region_args, args: + for (key, val) in iter(arg_list.items()): + if val is not None: + if key == 'auth' and config[key] is not None: + config[key] = _auth_update(config[key], val) + else: + config[key] = val # These backwards compat values are only set via argparse. If it's # there, it's because it was passed in explicitly, and should win diff --git a/os_client_config/tests/base.py b/os_client_config/tests/base.py index 33a868d33..6d9e093d3 100644 --- a/os_client_config/tests/base.py +++ b/os_client_config/tests/base.py @@ -16,6 +16,7 @@ # under the License. +import copy import os import tempfile @@ -96,8 +97,18 @@ USER_CONF = { 'auth_url': 'http://example.com/v2', }, 'regions': [ - 'region1', - 'region2', + { + 'name': 'region1', + 'values': { + 'external_network': 'region1-network', + } + }, + { + 'name': 'region2', + 'values': { + 'external_network': 'my-network', + } + } ], }, '_test_cloud_hyphenated': { @@ -139,7 +150,7 @@ class TestCase(base.BaseTestCase): super(TestCase, self).setUp() self.useFixture(fixtures.NestedTempfile()) - conf = dict(USER_CONF) + conf = copy.deepcopy(USER_CONF) tdir = self.useFixture(fixtures.TempDir()) conf['cache']['path'] = tdir.path self.cloud_yaml = _write_yaml(conf) diff --git a/os_client_config/tests/test_config.py b/os_client_config/tests/test_config.py index c9318fcd9..a6a35ada9 100644 --- a/os_client_config/tests/test_config.py +++ b/os_client_config/tests/test_config.py @@ -226,6 +226,8 @@ class TestConfig(base.TestCase): new_config) with open(self.cloud_yaml) as fh: written_config = yaml.safe_load(fh) + # We write a cache config for testing + written_config['cache'].pop('path', None) self.assertEqual(written_config, resulting_config) @@ -239,18 +241,26 @@ class TestConfigArgparse(base.TestCase): username='user', password='password', project_name='project', - region_name='other-test-region', + region_name='region2', snack_type='cookie', ) self.options = argparse.Namespace(**self.args) + def test_get_one_cloud_bad_region_argparse(self): + c = config.OpenStackConfig(config_files=[self.cloud_yaml], + vendor_files=[self.vendor_yaml]) + + self.assertRaises( + exceptions.OpenStackConfigException, c.get_one_cloud, + cloud='_test-cloud_', argparse=self.options) + def test_get_one_cloud_argparse(self): c = config.OpenStackConfig(config_files=[self.cloud_yaml], vendor_files=[self.vendor_yaml]) - cc = c.get_one_cloud(cloud='_test-cloud_', argparse=self.options) - self._assert_cloud_details(cc) - self.assertEqual(cc.region_name, 'other-test-region') + cc = c.get_one_cloud( + cloud='_test_cloud_regions', argparse=self.options) + self.assertEqual(cc.region_name, 'region2') self.assertEqual(cc.snack_type, 'cookie') def test_get_one_cloud_just_argparse(self): @@ -259,7 +269,7 @@ class TestConfigArgparse(base.TestCase): cc = c.get_one_cloud(argparse=self.options) self.assertIsNone(cc.cloud) - self.assertEqual(cc.region_name, 'other-test-region') + self.assertEqual(cc.region_name, 'region2') self.assertEqual(cc.snack_type, 'cookie') def test_get_one_cloud_just_kwargs(self): @@ -268,7 +278,7 @@ class TestConfigArgparse(base.TestCase): cc = c.get_one_cloud(**self.args) self.assertIsNone(cc.cloud) - self.assertEqual(cc.region_name, 'other-test-region') + self.assertEqual(cc.region_name, 'region2') self.assertEqual(cc.snack_type, 'cookie') def test_get_one_cloud_dash_kwargs(self): @@ -318,10 +328,10 @@ class TestConfigArgparse(base.TestCase): def test_get_one_cloud_bad_region_no_regions(self): c = config.OpenStackConfig(config_files=[self.cloud_yaml], vendor_files=[self.vendor_yaml]) - - cc = c.get_one_cloud(cloud='_test-cloud_', region_name='bad_region') - self._assert_cloud_details(cc) - self.assertEqual(cc.region_name, 'bad_region') + self.assertRaises( + exceptions.OpenStackConfigException, + c.get_one_cloud, + cloud='_test-cloud_', region_name='bad_region') def test_get_one_cloud_no_argparse_region2(self): c = config.OpenStackConfig(config_files=[self.cloud_yaml], @@ -333,6 +343,26 @@ class TestConfigArgparse(base.TestCase): self.assertEqual(cc.region_name, 'region2') self.assertIsNone(cc.snack_type) + def test_get_one_cloud_network(self): + c = config.OpenStackConfig(config_files=[self.cloud_yaml], + vendor_files=[self.vendor_yaml]) + + cc = c.get_one_cloud( + cloud='_test_cloud_regions', region_name='region1', argparse=None) + self._assert_cloud_details(cc) + self.assertEqual(cc.region_name, 'region1') + self.assertEqual('region1-network', cc.config['external_network']) + + def test_get_one_cloud_per_region_network(self): + c = config.OpenStackConfig(config_files=[self.cloud_yaml], + vendor_files=[self.vendor_yaml]) + + cc = c.get_one_cloud( + cloud='_test_cloud_regions', region_name='region2', argparse=None) + self._assert_cloud_details(cc) + self.assertEqual(cc.region_name, 'region2') + self.assertEqual('my-network', cc.config['external_network']) + def test_fix_env_args(self): c = config.OpenStackConfig(config_files=[self.cloud_yaml], vendor_files=[self.vendor_yaml])