diff --git a/octavia/api/common/pagination.py b/octavia/api/common/pagination.py index 81eb45dacd..b95b6c6381 100644 --- a/octavia/api/common/pagination.py +++ b/octavia/api/common/pagination.py @@ -311,13 +311,26 @@ class PaginationHelper(object): self.sort_keys.append((key, self.sort_dir)) for current_sort_key, current_sort_dir in self.sort_keys: + # Translate sort_key from API standard to data model's name + current_sort_key = ( + model.__v2_wsme__.translate_key_to_data_model( + current_sort_key)) sort_dir_func = { constants.ASC: sqlalchemy.asc, constants.DESC: sqlalchemy.desc, }[current_sort_dir] try: - sort_key_attr = getattr(model, current_sort_key) + # The translated object may be a nested parameter + # such as vip.ip_address, so handle that case by + # joining with the nested table. + if '.' in current_sort_key: + parent, child = current_sort_key.split('.') + parent_obj = getattr(model, parent) + query = query.join(parent_obj) + sort_key_attr = child + else: + sort_key_attr = getattr(model, current_sort_key) except AttributeError: raise exceptions.InvalidSortKey(key=current_sort_key) query = query.order_by(sort_dir_func(sort_key_attr)) diff --git a/octavia/api/common/types.py b/octavia/api/common/types.py index caf6c99c74..fb14ee3f06 100644 --- a/octavia/api/common/types.py +++ b/octavia/api/common/types.py @@ -173,6 +173,14 @@ class BaseType(wtypes.Base, metaclass=BaseMeta): res[k] = v return res + @classmethod + def translate_key_to_data_model(cls, key): + """Translate the keys from wsme class type, to data_model.""" + if not hasattr(cls, '_type_to_model_map') or ( + key not in cls._type_to_model_map): + return key + return cls._type_to_model_map[key] + def to_dict(self, render_unsets=False): """Converts Octavia WSME type to dictionary. diff --git a/octavia/api/v2/controllers/pool.py b/octavia/api/v2/controllers/pool.py index af0f2c9396..cad21f329b 100644 --- a/octavia/api/v2/controllers/pool.py +++ b/octavia/api/v2/controllers/pool.py @@ -137,7 +137,7 @@ class PoolsController(base.BaseController): def _is_only_specified_in_request(self, request, **kwargs): request_attrs = [] check_attrs = kwargs['check_exist_attrs'] - escaped_attrs = ['from_data_model', + escaped_attrs = ['from_data_model', 'translate_key_to_data_model', 'translate_dict_keys_to_data_model', 'to_dict'] for attr in dir(request): diff --git a/octavia/tests/functional/api/v2/test_health_monitor.py b/octavia/tests/functional/api/v2/test_health_monitor.py index 97cc9d2d09..2c78260a20 100644 --- a/octavia/tests/functional/api/v2/test_health_monitor.py +++ b/octavia/tests/functional/api/v2/test_health_monitor.py @@ -455,6 +455,64 @@ class TestHealthMonitor(base.BaseAPITest): hm_id_names_asc = [(hm.get('id'), hm.get('name')) for hm in hms_asc] self.assertEqual(hm_id_names_asc, list(reversed(hm_id_names_desc))) + def test_get_all_sorted_by_max_retries(self): + pool1 = self.create_pool( + self.lb_id, + constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN, + name='pool1').get('pool') + self.set_lb_status(self.lb_id) + pool2 = self.create_pool( + self.lb_id, + constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN, + name='pool2').get('pool') + self.set_lb_status(self.lb_id) + pool3 = self.create_pool( + self.lb_id, + constants.PROTOCOL_HTTP, + constants.LB_ALGORITHM_ROUND_ROBIN, + name='pool3').get('pool') + self.set_lb_status(self.lb_id) + hm1 = self.create_health_monitor( + pool1.get('id'), constants.HEALTH_MONITOR_HTTP, + 1, 1, 1, 2, name='hm1').get(self.root_tag) + self.set_lb_status(self.lb_id) + hm2 = self.create_health_monitor( + pool2.get('id'), constants.HEALTH_MONITOR_PING, + 1, 1, 1, 1, name='hm2').get(self.root_tag) + self.set_lb_status(self.lb_id) + hm3 = self.create_health_monitor( + pool3.get('id'), constants.HEALTH_MONITOR_TCP, + 1, 1, 1, 3, name='hm3').get(self.root_tag) + self.set_lb_status(self.lb_id) + + response = self.get(self.HMS_PATH, params={'sort': 'max_retries:desc'}) + hms_desc = response.json.get(self.root_tag_list) + response = self.get(self.HMS_PATH, params={'sort': 'max_retries:asc'}) + hms_asc = response.json.get(self.root_tag_list) + + self.assertEqual(3, len(hms_desc)) + self.assertEqual(3, len(hms_asc)) + + hm_id_names_desc = [(hm.get('id'), hm.get('name')) for hm in hms_desc] + hm_id_names_asc = [(hm.get('id'), hm.get('name')) for hm in hms_asc] + self.assertEqual(hm_id_names_asc, list(reversed(hm_id_names_desc))) + + self.assertEqual(hm2[constants.MAX_RETRIES], + hms_asc[0][constants.MAX_RETRIES]) + self.assertEqual(hm1[constants.MAX_RETRIES], + hms_asc[1][constants.MAX_RETRIES]) + self.assertEqual(hm3[constants.MAX_RETRIES], + hms_asc[2][constants.MAX_RETRIES]) + + self.assertEqual(hm3[constants.MAX_RETRIES], + hms_desc[0][constants.MAX_RETRIES]) + self.assertEqual(hm1[constants.MAX_RETRIES], + hms_desc[1][constants.MAX_RETRIES]) + self.assertEqual(hm2[constants.MAX_RETRIES], + hms_desc[2][constants.MAX_RETRIES]) + def test_get_all_limited(self): pool1 = self.create_pool( self.lb_id, diff --git a/octavia/tests/functional/api/v2/test_load_balancer.py b/octavia/tests/functional/api/v2/test_load_balancer.py index dcf64c7009..57da1a049b 100644 --- a/octavia/tests/functional/api/v2/test_load_balancer.py +++ b/octavia/tests/functional/api/v2/test_load_balancer.py @@ -1310,6 +1310,41 @@ class TestLoadBalancer(base.BaseAPITest): lb_id_names_asc = [(lb.get('id'), lb.get('name')) for lb in lbs_asc] self.assertEqual(lb_id_names_asc, list(reversed(lb_id_names_desc))) + def test_get_all_sorted_by_vip_ip_address(self): + self.create_load_balancer(uuidutils.generate_uuid(), + name='lb1', + project_id=self.project_id, + vip_address='198.51.100.2') + self.create_load_balancer(uuidutils.generate_uuid(), + name='lb2', + project_id=self.project_id, + vip_address='198.51.100.1') + self.create_load_balancer(uuidutils.generate_uuid(), + name='lb3', + project_id=self.project_id, + vip_address='198.51.100.3') + response = self.get(self.LBS_PATH, + params={'sort': 'vip_address:desc'}) + lbs_desc = response.json.get(self.root_tag_list) + response = self.get(self.LBS_PATH, + params={'sort': 'vip_address:asc'}) + lbs_asc = response.json.get(self.root_tag_list) + + self.assertEqual(3, len(lbs_desc)) + self.assertEqual(3, len(lbs_asc)) + + lb_id_names_desc = [(lb.get('id'), lb.get('name')) for lb in lbs_desc] + lb_id_names_asc = [(lb.get('id'), lb.get('name')) for lb in lbs_asc] + self.assertEqual(lb_id_names_asc, list(reversed(lb_id_names_desc))) + + self.assertEqual('198.51.100.1', lbs_asc[0][constants.VIP_ADDRESS]) + self.assertEqual('198.51.100.2', lbs_asc[1][constants.VIP_ADDRESS]) + self.assertEqual('198.51.100.3', lbs_asc[2][constants.VIP_ADDRESS]) + + self.assertEqual('198.51.100.3', lbs_desc[0][constants.VIP_ADDRESS]) + self.assertEqual('198.51.100.2', lbs_desc[1][constants.VIP_ADDRESS]) + self.assertEqual('198.51.100.1', lbs_desc[2][constants.VIP_ADDRESS]) + def test_get_all_limited(self): self.create_load_balancer(uuidutils.generate_uuid(), name='lb1', diff --git a/releasenotes/notes/fix-api-sort-key-337f342d5cdce432.yaml b/releasenotes/notes/fix-api-sort-key-337f342d5cdce432.yaml new file mode 100644 index 0000000000..313b32a9dc --- /dev/null +++ b/releasenotes/notes/fix-api-sort-key-337f342d5cdce432.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed an issue where some columns could not be used for sort keys in + API list calls.