diff options
| -rw-r--r-- | ceilometerclient/client.py | 87 | ||||
| -rw-r--r-- | ceilometerclient/common/base.py | 6 | ||||
| -rw-r--r-- | ceilometerclient/shell.py | 15 | ||||
| -rw-r--r-- | ceilometerclient/tests/v2/test_alarms.py | 20 | ||||
| -rw-r--r-- | ceilometerclient/tests/v2/test_samples.py | 2 | ||||
| -rw-r--r-- | ceilometerclient/tests/v2/test_shell.py | 38 | ||||
| -rw-r--r-- | ceilometerclient/v2/alarms.py | 6 | ||||
| -rw-r--r-- | ceilometerclient/v2/samples.py | 2 | ||||
| -rw-r--r-- | ceilometerclient/v2/shell.py | 58 | ||||
| -rw-r--r-- | doc/source/conf.py | 5 | ||||
| -rw-r--r-- | requirements.txt | 3 | ||||
| -rw-r--r-- | test-requirements.txt | 3 |
12 files changed, 161 insertions, 84 deletions
diff --git a/ceilometerclient/client.py b/ceilometerclient/client.py index e4568a4..73772be 100644 --- a/ceilometerclient/client.py +++ b/ceilometerclient/client.py @@ -13,7 +13,9 @@ from keystoneclient.auth.identity import v2 as v2_auth from keystoneclient.auth.identity import v3 as v3_auth from keystoneclient import discover +from keystoneclient import exceptions as ks_exc from keystoneclient import session +import six.moves.urllib.parse as urlparse from ceilometerclient.common import utils from ceilometerclient import exc @@ -21,11 +23,39 @@ from ceilometerclient.openstack.common.apiclient import auth from ceilometerclient.openstack.common.apiclient import exceptions +def _discover_auth_versions(session, auth_url): + # discover the API versions the server is supporting based on the + # given URL + v2_auth_url = None + v3_auth_url = None + try: + ks_discover = discover.Discover(session=session, auth_url=auth_url) + v2_auth_url = ks_discover.url_for('2.0') + v3_auth_url = ks_discover.url_for('3.0') + except ks_exc.DiscoveryFailure: + raise + except exceptions.ClientException: + # Identity service may not support discovery. In that case, + # try to determine version from auth_url + url_parts = urlparse.urlparse(auth_url) + (scheme, netloc, path, params, query, fragment) = url_parts + path = path.lower() + if path.startswith('/v3'): + v3_auth_url = auth_url + elif path.startswith('/v2'): + v2_auth_url = auth_url + else: + raise exc.CommandError('Unable to determine the Keystone ' + 'version to authenticate with ' + 'using the given auth_url.') + return (v2_auth_url, v3_auth_url) + + def _get_keystone_session(**kwargs): # TODO(fabgia): the heavy lifting here should be really done by Keystone. # Unfortunately Keystone does not support a richer method to perform # discovery and return a single viable URL. A bug against Keystone has - # been filed: https://bugs.launchpad.net/pyhton-keystoneclient/+bug/1330677 + # been filed: https://bugs.launchpad.net/python-keystoneclient/+bug/1330677 # first create a Keystone session cacert = kwargs.pop('cacert', None) @@ -48,17 +78,7 @@ def _get_keystone_session(**kwargs): # create the keystone client session ks_session = session.Session(verify=verify, cert=cert) - - try: - # discover the supported keystone versions using the auth endpoint url - ks_discover = discover.Discover(session=ks_session, auth_url=auth_url) - # Determine which authentication plugin to use. - v2_auth_url = ks_discover.url_for('2.0') - v3_auth_url = ks_discover.url_for('3.0') - except Exception: - raise exc.CommandError('Unable to determine the Keystone version ' - 'to authenticate with using the given ' - 'auth_url: %s' % auth_url) + v2_auth_url, v3_auth_url = _discover_auth_versions(ks_session, auth_url) username = kwargs.pop('username', None) user_id = kwargs.pop('user_id', None) @@ -68,46 +88,25 @@ def _get_keystone_session(**kwargs): project_domain_id = kwargs.pop('project_domain_id', None) auth = None - if v3_auth_url and v2_auth_url: - # the auth_url does not have the versions specified - # e.g. http://no.where:5000 - # Keystone will return both v2 and v3 as viable options - # but we need to decide based on the arguments passed - # what version is callable - if (user_domain_name or user_domain_id or project_domain_name or - project_domain_id): - # domain is supported only in v3 - auth = v3_auth.Password( - v3_auth_url, - username=username, - user_id=user_id, - user_domain_name=user_domain_name, - user_domain_id=user_domain_id, - project_domain_name=project_domain_name, - project_domain_id=project_domain_id, - **kwargs) - else: - # no domain, then use v2 - auth = v2_auth.Password( - v2_auth_url, - username, - kwargs.pop('password', None), - tenant_id=project_id, - tenant_name=project_name) - elif v3_auth_url: + use_domain = (user_domain_id or user_domain_name or + project_domain_id or project_domain_name) + use_v3 = v3_auth_url and (use_domain or (not v2_auth_url)) + use_v2 = v2_auth_url and not use_domain + + if use_v3: # the auth_url as v3 specified # e.g. http://no.where:5000/v3 # Keystone will return only v3 as viable option auth = v3_auth.Password( v3_auth_url, username=username, + password=kwargs.pop('password', None), user_id=user_id, user_domain_name=user_domain_name, user_domain_id=user_domain_id, project_domain_name=project_domain_name, - project_domain_id=project_domain_id, - **kwargs) - elif v2_auth_url: + project_domain_id=project_domain_id) + elif use_v2: # the auth_url as v2 specified # e.g. http://no.where:5000/v2.0 # Keystone will return only v2 as viable option @@ -183,7 +182,7 @@ class AuthPlugin(auth.BaseAuthPlugin): token = lambda: ks_session.get_token() endpoint = self.opts.get('endpoint') or \ _get_endpoint(ks_session, **ks_kwargs) - self.opts['token'] = token() + self.opts['token'] = token self.opts['endpoint'] = endpoint def token_and_endpoint(self, endpoint_type, service_type): @@ -222,7 +221,7 @@ def Client(version, *args, **kwargs): def get_client(version, **kwargs): - """Get an authtenticated client, based on the credentials + """Get an authenticated client, based on the credentials in the keyword args. :param api_version: the API version to use ('1' or '2') diff --git a/ceilometerclient/common/base.py b/ceilometerclient/common/base.py index 7d4be24..4129176 100644 --- a/ceilometerclient/common/base.py +++ b/ceilometerclient/common/base.py @@ -19,6 +19,7 @@ Base utilities to build API operation managers and objects on top of. import copy +from ceilometerclient import exc from ceilometerclient.openstack.common.apiclient import base # Python 2.4 compat @@ -55,7 +56,10 @@ class Manager(object): def _list(self, url, response_key=None, obj_class=None, body=None, expect_single=False): - body = self.api.get(url).json() + resp = self.api.get(url) + if not resp.content: + raise exc.HTTPNotFound + body = resp.json() if obj_class is None: obj_class = self.resource_class diff --git a/ceilometerclient/shell.py b/ceilometerclient/shell.py index 138dd62..d16dc51 100644 --- a/ceilometerclient/shell.py +++ b/ceilometerclient/shell.py @@ -256,17 +256,10 @@ class CeilometerShell(object): class HelpFormatter(argparse.HelpFormatter): - INDENT_BEFORE_ARGUMENTS = 6 - MAX_WIDTH_ARGUMENTS = 32 - - def add_arguments(self, actions): - for action in filter(lambda x: not x.option_strings, actions): - for choice in action.choices: - length = len(choice) + self.INDENT_BEFORE_ARGUMENTS - if(length > self._max_help_position and - length <= self.MAX_WIDTH_ARGUMENTS): - self._max_help_position = length - super(HelpFormatter, self).add_arguments(actions) + def __init__(self, prog, indent_increment=2, max_help_position=32, + width=None): + super(HelpFormatter, self).__init__(prog, indent_increment, + max_help_position, width) def start_section(self, heading): # Title-case the headings diff --git a/ceilometerclient/tests/v2/test_alarms.py b/ceilometerclient/tests/v2/test_alarms.py index c3aa6d5..7f0d8bf 100644 --- a/ceilometerclient/tests/v2/test_alarms.py +++ b/ceilometerclient/tests/v2/test_alarms.py @@ -21,6 +21,7 @@ import six from six.moves import xrange # noqa import testtools +from ceilometerclient import exc from ceilometerclient.openstack.common.apiclient import client from ceilometerclient.openstack.common.apiclient import fake_client from ceilometerclient.v2 import alarms @@ -207,6 +208,17 @@ fixtures = { None, ), }, + '/v2/alarms/unk-alarm-id': + { + 'GET': ( + {}, + None, + ), + 'PUT': ( + {}, + None, + ), + }, '/v2/alarms/alarm-id/state': { 'PUT': ( @@ -380,6 +392,14 @@ class AlarmManagerTest(testtools.TestCase): self.http_client.assert_called(*expect_get_2, pos=1) self.assertEqual('alarm', state) + def test_update_missing(self): + alarm = None + try: + alarm = self.mgr.update(alarm_id='unk-alarm-id', **UPDATE_ALARM) + except exc.CommandError: + pass + self.assertEqual(alarm, None) + def test_delete_from_alarm_class(self): alarm = self.mgr.get(alarm_id='alarm-id') self.assertIsNotNone(alarm) diff --git a/ceilometerclient/tests/v2/test_samples.py b/ceilometerclient/tests/v2/test_samples.py index 5804c7e..dfbdf39 100644 --- a/ceilometerclient/tests/v2/test_samples.py +++ b/ceilometerclient/tests/v2/test_samples.py @@ -103,7 +103,7 @@ class SampleManagerTest(utils.BaseTestCase): expect = [ 'POST', '/v2/meters/instance' ] - self.http_client.assert_called(*expect, body=CREATE_SAMPLE) + self.http_client.assert_called(*expect, body=[CREATE_SAMPLE]) self.assertIsNotNone(sample) def test_limit(self): diff --git a/ceilometerclient/tests/v2/test_shell.py b/ceilometerclient/tests/v2/test_shell.py index 7da088a..632684f 100644 --- a/ceilometerclient/tests/v2/test_shell.py +++ b/ceilometerclient/tests/v2/test_shell.py @@ -260,8 +260,7 @@ class ShellAlarmCommandTest(utils.BaseTestCase): self._test_alarm_threshold_action_args('create', argv) def test_alarm_threshold_update_args(self): - argv = ['alarm-threshold-update', - '--alarm_id', 'x'] + self.THRESHOLD_ALARM_CLI_ARGS + argv = ['alarm-threshold-update', 'x'] + self.THRESHOLD_ALARM_CLI_ARGS self._test_alarm_threshold_action_args('update', argv) @mock.patch('sys.stdout', new=six.StringIO()) @@ -804,14 +803,18 @@ class ShellStatisticsTest(utils.BaseTestCase): class ShellEmptyIdTest(utils.BaseTestCase): """Test empty field which will cause calling incorrect rest uri.""" - def _test_entity_action_with_empty_values(self, entity, *args): + def _test_entity_action_with_empty_values(self, entity, + *args, **kwargs): + positional = kwargs.pop('positional', False) for value in ('', ' ', ' ', '\t'): - self._test_entity_action_with_empty_value(entity, value, *args) + self._test_entity_action_with_empty_value(entity, value, + positional, *args) - def _test_entity_action_with_empty_value(self, entity, value, *args): + def _test_entity_action_with_empty_value(self, entity, value, + positional, *args): + new_args = [value] if positional else ['--%s' % entity, value] + argv = list(args) + new_args shell = base_shell.CeilometerShell() - argv = list(args) + ['--%s' % entity, value] - with mock.patch('ceilometerclient.exc.CommandError') as e: e.return_value = exc.BaseException() self.assertRaises(exc.BaseException, shell.parse_args, argv) @@ -820,7 +823,8 @@ class ShellEmptyIdTest(utils.BaseTestCase): def _test_alarm_action_with_empty_ids(self, method, *args): args = [method] + list(args) - self._test_entity_action_with_empty_values('alarm_id', *args) + self._test_entity_action_with_empty_values('alarm_id', + positional=True, *args) def test_alarm_show_with_empty_id(self): self._test_alarm_action_with_empty_ids('alarm-show') @@ -879,3 +883,21 @@ class ShellEmptyIdTest(utils.BaseTestCase): def test_trait_list_with_empty_trait_name(self): args = ['trait-list', '--event_type', 'x'] self._test_entity_action_with_empty_values('trait_name', *args) + + +class ShellObsoletedArgsTest(utils.BaseTestCase): + """Test arguments that have been obsoleted.""" + + def _test_entity_obsoleted(self, entity, value, positional, *args): + new_args = [value] if positional else ['--%s' % entity, value] + argv = list(args) + new_args + shell = base_shell.CeilometerShell() + with mock.patch('sys.stdout', new_callable=six.StringIO) as stdout: + shell.parse_args(argv) + self.assertIn('obsolete', stdout.getvalue()) + + def test_obsolete_alarm_id(self): + for method in ['alarm-show', 'alarm-update', 'alarm-threshold-update', + 'alarm-combination-update', 'alarm-delete', + 'alarm-state-get', 'alarm-history']: + self._test_entity_obsoleted('alarm_id', 'abcde', False, method) diff --git a/ceilometerclient/v2/alarms.py b/ceilometerclient/v2/alarms.py index 341cbb7..694b5c0 100644 --- a/ceilometerclient/v2/alarms.py +++ b/ceilometerclient/v2/alarms.py @@ -84,6 +84,7 @@ class AlarmManager(base.Manager): return self._list(self._path(alarm_id), expect_single=True)[0] except IndexError: return None + except exc.HTTPNotFound: # When we try to get deleted alarm HTTPNotFound occurs # or when alarm doesn't exists this exception don't must @@ -156,7 +157,10 @@ class AlarmManager(base.Manager): def update(self, alarm_id, **kwargs): self._compat_legacy_alarm_kwargs(kwargs) - updated = self.get(alarm_id).to_dict() + alarm = self.get(alarm_id) + if alarm is None: + raise exc.CommandError('Alarm not found: %s' % alarm_id) + updated = alarm.to_dict() updated['time_constraints'] = self._merge_time_constraints( updated.get('time_constraints', []), kwargs) kwargs = dict((k, v) for k, v in kwargs.items() diff --git a/ceilometerclient/v2/samples.py b/ceilometerclient/v2/samples.py index b0ff3b5..8081efc 100644 --- a/ceilometerclient/v2/samples.py +++ b/ceilometerclient/v2/samples.py @@ -47,6 +47,6 @@ class SampleManager(base.Manager): new = dict((key, value) for (key, value) in kwargs.items() if key in CREATION_ATTRIBUTES) url = self._path(counter_name=kwargs['counter_name']) - body = self.api.post(url, json=new).json() + body = self.api.post(url, json=[new]).json() if body: return [Sample(self, b) for b in body] diff --git a/ceilometerclient/v2/shell.py b/ceilometerclient/v2/shell.py index 12f9cbf..8ee1149 100644 --- a/ceilometerclient/v2/shell.py +++ b/ceilometerclient/v2/shell.py @@ -51,11 +51,21 @@ SIMPLE_OPERATORS = ["=", "!=", "<", "<=", '>', '>='] class NotEmptyAction(argparse.Action): def __call__(self, parser, namespace, values, option_string=None): + values = values or getattr(namespace, self.dest) if not values or values.isspace(): raise exc.CommandError('%s should not be empty' % self.dest) setattr(namespace, self.dest, values) +def obsoleted_by(new_dest): + class ObsoletedByAction(argparse.Action): + def __call__(self, parser, namespace, values, option_string=None): + old_dest = option_string or self.dest + print('%s is obsolete! See help for more details.' % old_dest) + setattr(namespace, new_dest, values) + return ObsoletedByAction + + @utils.arg('-q', '--query', metavar='<QUERY>', help='key[op]data_type::value; list. data_type is optional, ' 'but if supplied must be string, integer, float, or boolean.') @@ -333,7 +343,10 @@ def _display_alarm(alarm): utils.print_dict(data, wrap=72) -@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True, +@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', + action=obsoleted_by('alarm_id'), help=argparse.SUPPRESS, + dest='alarm_id_deprecated') +@utils.arg('alarm_id', metavar='<ALARM_ID>', nargs='?', action=NotEmptyAction, help='ID of the alarm to show.') def do_alarm_show(cc, args={}): '''Show an alarm.''' @@ -490,7 +503,10 @@ def do_alarm_combination_create(cc, args={}): _display_alarm(alarm) -@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True, +@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', + action=obsoleted_by('alarm_id'), help=argparse.SUPPRESS, + dest='alarm_id_deprecated') +@utils.arg('alarm_id', metavar='<ALARM_ID>', nargs='?', action=NotEmptyAction, help='ID of the alarm to update.') @common_alarm_arguments() @utils.arg('--remove-time-constraint', action='append', @@ -531,7 +547,10 @@ def do_alarm_update(cc, args={}): _display_alarm(alarm) -@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True, +@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', + action=obsoleted_by('alarm_id'), help=argparse.SUPPRESS, + dest='alarm_id_deprecated') +@utils.arg('alarm_id', metavar='<ALARM_ID>', nargs='?', action=NotEmptyAction, help='ID of the alarm to update.') @common_alarm_arguments() @utils.arg('--remove-time-constraint', action='append', @@ -583,7 +602,10 @@ def do_alarm_threshold_update(cc, args={}): _display_alarm(alarm) -@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True, +@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', + action=obsoleted_by('alarm_id'), help=argparse.SUPPRESS, + dest='alarm_id_deprecated') +@utils.arg('alarm_id', metavar='<ALARM_ID>', nargs='?', action=NotEmptyAction, help='ID of the alarm to update.') @common_alarm_arguments() @utils.arg('--remove-time-constraint', action='append', @@ -615,7 +637,10 @@ def do_alarm_combination_update(cc, args={}): _display_alarm(alarm) -@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True, +@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', + action=obsoleted_by('alarm_id'), help=argparse.SUPPRESS, + dest='alarm_id_deprecated') +@utils.arg('alarm_id', metavar='<ALARM_ID>', nargs='?', action=NotEmptyAction, help='ID of the alarm to delete.') def do_alarm_delete(cc, args={}): '''Delete an alarm.''' @@ -625,7 +650,10 @@ def do_alarm_delete(cc, args={}): raise exc.CommandError('Alarm not found: %s' % args.alarm_id) -@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True, +@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', + action=obsoleted_by('alarm_id'), help=argparse.SUPPRESS, + dest='alarm_id_deprecated') +@utils.arg('alarm_id', metavar='<ALARM_ID>', nargs='?', action=NotEmptyAction, help='ID of the alarm state to set.') @utils.arg('--state', metavar='<STATE>', required=True, help='State of the alarm, one of: ' + str(ALARM_STATES) + @@ -639,7 +667,10 @@ def do_alarm_state_set(cc, args={}): utils.print_dict({'state': state}, wrap=72) -@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True, +@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', + action=obsoleted_by('alarm_id'), help=argparse.SUPPRESS, + dest='alarm_id_deprecated') +@utils.arg('alarm_id', metavar='<ALARM_ID>', nargs='?', action=NotEmptyAction, help='ID of the alarm state to show.') def do_alarm_state_get(cc, args={}): '''Get the state of an alarm.''' @@ -650,8 +681,10 @@ def do_alarm_state_get(cc, args={}): utils.print_dict({'state': state}, wrap=72) -@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', required=True, - action=NotEmptyAction, +@utils.arg('-a', '--alarm_id', metavar='<ALARM_ID>', + action=obsoleted_by('alarm_id'), help=argparse.SUPPRESS, + dest='alarm_id_deprecated') +@utils.arg('alarm_id', metavar='<ALARM_ID>', nargs='?', action=NotEmptyAction, help='ID of the alarm for which history is shown.') @utils.arg('-q', '--query', metavar='<QUERY>', help='key[op]data_type::value; list. data_type is optional, ' @@ -688,7 +721,7 @@ def do_resource_list(cc, args={}): sortby=1) -@utils.arg('-r', '--resource_id', metavar='<RESOURCE_ID>', required=True, +@utils.arg('resource_id', metavar='<RESOURCE_ID>', action=NotEmptyAction, help='ID of the resource to show.') def do_resource_show(cc, args={}): '''Show the resource.''' @@ -719,9 +752,8 @@ def do_event_list(cc, args={}): )}) -@utils.arg('-m', '--message_id', metavar='<message_id>', - help='The ID of the event. Should be a UUID.', - required=True, action=NotEmptyAction) +@utils.arg('message_id', metavar='<message_id>', action=NotEmptyAction, + help='The ID of the event. Should be a UUID.') def do_event_show(cc, args={}): '''Show a particular event.''' event = cc.events.get(args.message_id) diff --git a/doc/source/conf.py b/doc/source/conf.py index 7124622..2313013 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -26,7 +26,7 @@ gen_ref("v2", "Version 2 API Reference", # Add any Sphinx extension module names here, as strings. They can be # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom ones. -extensions = ['sphinx.ext.autodoc', 'sphinx.ext.intersphinx', 'oslosphinx' ] +extensions = ['sphinx.ext.autodoc', 'oslosphinx' ] # autodoc generation is a bit aggressive and a nuisance when doing heavy # text edit cycles. @@ -76,6 +76,3 @@ latex_documents = [ 'manual' ), ] - -# Example configuration for intersphinx: refer to the Python standard library. -intersphinx_mapping = {'http://docs.python.org/': None} diff --git a/requirements.txt b/requirements.txt index cb199b5..ffc4744 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,6 @@ +# The order of packages is significant, because pip processes them in the order +# of appearance. Changing the order has an impact on the overall integration +# process, which may cause wedges in the gate later. pbr>=0.6,!=0.7,<1.0 argparse iso8601>=0.1.9 diff --git a/test-requirements.txt b/test-requirements.txt index 41a8186..1bdbb36 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,3 +1,6 @@ +# The order of packages is significant, because pip processes them in the order +# of appearance. Changing the order has an impact on the overall integration +# process, which may cause wedges in the gate later. # Hacking already pins down pep8, pyflakes and flake8 hacking>=0.9.1,<0.10 coverage>=3.6 |