From d85b18cb5c0d0873b919ce8d76cdd3540ba7724c Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Thu, 6 Aug 2020 15:15:52 -0700 Subject: [PATCH] Fix API sort key for complex columns The Octavia API sort function was not working correctly for some columns that have different names in the API vs the data model. This patch also handles the case where a column may be a compound object in the data model, such as vip_address->vip.ip_address. Change-Id: I0c124e80fec2ac9ad813e78df6ad25da6b8e9668 Story: 2007991 Task: 40626 (cherry picked from commit b6f6c14292329b1a977fd7c0831f917cd69cbaaf) --- octavia/api/common/pagination.py | 15 ++++- octavia/api/common/types.py | 8 +++ octavia/api/v2/controllers/pool.py | 2 +- octavia/common/constants.py | 1 + .../functional/api/v2/test_health_monitor.py | 58 +++++++++++++++++++ .../functional/api/v2/test_load_balancer.py | 35 +++++++++++ .../fix-api-sort-key-337f342d5cdce432.yaml | 5 ++ 7 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/fix-api-sort-key-337f342d5cdce432.yaml diff --git a/octavia/api/common/pagination.py b/octavia/api/common/pagination.py index 3cbd6f7943..a6ad8a181c 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 5d62578bd3..1ef9a98d58 100644 --- a/octavia/api/common/types.py +++ b/octavia/api/common/types.py @@ -155,6 +155,14 @@ class BaseType(wtypes.Base): 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 7efd1a2e1d..a940810c8c 100644 --- a/octavia/api/v2/controllers/pool.py +++ b/octavia/api/v2/controllers/pool.py @@ -136,7 +136,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/common/constants.py b/octavia/common/constants.py index da002e374c..ffb30acc7b 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -318,6 +318,7 @@ AMP_DATA = 'amp_data' AMPS_DATA = 'amps_data' NICS = 'nics' VIP = 'vip' +VIP_ADDRESS = 'vip_address' POOL = 'pool' POOL_CHILD_COUNT = 'pool_child_count' POOL_ID = 'pool_id' diff --git a/octavia/tests/functional/api/v2/test_health_monitor.py b/octavia/tests/functional/api/v2/test_health_monitor.py index b6c54c4a0e..9f15c7d4be 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 4a6e0dfbcc..d3322abd05 100644 --- a/octavia/tests/functional/api/v2/test_load_balancer.py +++ b/octavia/tests/functional/api/v2/test_load_balancer.py @@ -1250,6 +1250,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.