From 8c32d2e6024d9cfa3e6ab8bff43f2a261aaee82d 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 3ee8424f7b..0c61b53363 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 ba1fe33c7e..17bc80081d 100644 --- a/octavia/api/common/types.py +++ b/octavia/api/common/types.py @@ -170,6 +170,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 edc5901918..e2565b7ea2 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 c05b55a18c..e2036bf8fe 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -398,6 +398,7 @@ TOTAL_CONNECTIONS = 'total_connections' UPDATED_AT = 'updated_at' UPDATE_DICT = 'update_dict' VIP = 'vip' +VIP_ADDRESS = 'vip_address' VIP_NETWORK = 'vip_network' VIP_SG_ID = 'vip_sg_id' VIP_SUBNET = 'vip_subnet' 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 65cbd94911..af3a228c85 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.