From bda72e4adcb33d5a7e6d47fcc46626ebfab1f2f0 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Fri, 2 Sep 2016 09:58:11 -0500 Subject: [PATCH] Use list_servers for polling rather than get_server_by_id Using get_server_by_id bypasses the list_server poll/cache. There is one legit place where we should do this, but in the other places it causes us to poll with great frequency unnecessarily. The TaskManager test changes are just to get test discover to stop finding then skipping the fixtures in the file. Change-Id: I7f1e360f53d7344dfc996eb4c4813184720f8da0 --- shade/openstackcloud.py | 12 ++++++--- shade/tests/unit/test_floating_ip_neutron.py | 6 ++--- shade/tests/unit/test_rebuild_server.py | 14 +++++------ shade/tests/unit/test_task_manager.py | 26 ++++++++++---------- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index b91089140..597e1bddf 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -3900,8 +3900,9 @@ class OpenStackCloud(object): server_id = server['id'] for _ in _utils._iterate_timeout( timeout, - "Timeout waiting for the floating IP to be attached."): - server = self.get_server_by_id(server_id) + "Timeout waiting for the floating IP to be attached.", + wait=self._SERVER_AGE): + server = self.get_server(server_id) ext_ip = meta.get_server_ip(server, ext_tag='floating') if ext_ip == floating_ip['floating_ip_address']: return server @@ -4652,11 +4653,14 @@ class OpenStackCloud(object): for count in _utils._iterate_timeout( timeout, "Timeout waiting for server {0} to " - "rebuild.".format(server_id)): + "rebuild.".format(server_id), + wait=self._SERVER_AGE): try: - server = self.get_server_by_id(server_id) + server = self.get_server(server_id) except Exception: continue + if not server: + continue if server['status'] == 'ACTIVE': server.adminPass = admin_pass diff --git a/shade/tests/unit/test_floating_ip_neutron.py b/shade/tests/unit/test_floating_ip_neutron.py index 262fadbca..34d1083bb 100644 --- a/shade/tests/unit/test_floating_ip_neutron.py +++ b/shade/tests/unit/test_floating_ip_neutron.py @@ -532,16 +532,16 @@ class TestFloatingIP(base.TestCase): ) @patch.object(OpenStackCloud, 'delete_floating_ip') - @patch.object(OpenStackCloud, 'get_server_by_id') + @patch.object(OpenStackCloud, 'get_server') @patch.object(OpenStackCloud, 'create_floating_ip') @patch.object(OpenStackCloud, 'has_service') def test_add_ip_refresh_timeout( self, mock_has_service, mock_create_floating_ip, - mock_get_server_by_id, mock_delete_floating_ip): + mock_get_server, mock_delete_floating_ip): mock_has_service.return_value = True mock_create_floating_ip.return_value = self.floating_ip - mock_get_server_by_id.return_value = self.fake_server + mock_get_server.return_value = self.fake_server self.assertRaises( exc.OpenStackCloudTimeout, diff --git a/shade/tests/unit/test_rebuild_server.py b/shade/tests/unit/test_rebuild_server.py index 5cb81eab4..509394b99 100644 --- a/shade/tests/unit/test_rebuild_server.py +++ b/shade/tests/unit/test_rebuild_server.py @@ -56,13 +56,13 @@ class TestRebuildServer(base.TestCase): with patch("shade.OpenStackCloud"): config = { "servers.rebuild.return_value": rebuild_server, - "servers.get.return_value": error_server, + "servers.list.return_value": [error_server], "floating_ips.list.return_value": [fake_floating_ip] } OpenStackCloud.nova_client = Mock(**config) self.assertRaises( OpenStackCloudException, - self.cloud.rebuild_server, "a", "b", wait=True) + self.cloud.rebuild_server, "1234", "b", wait=True) def test_rebuild_server_timeout(self): """ @@ -73,7 +73,7 @@ class TestRebuildServer(base.TestCase): with patch("shade.OpenStackCloud"): config = { "servers.rebuild.return_value": rebuild_server, - "servers.get.return_value": rebuild_server, + "servers.list.return_value": [rebuild_server], } OpenStackCloud.nova_client = Mock(**config) self.assertRaises( @@ -125,7 +125,7 @@ class TestRebuildServer(base.TestCase): '5678') config = { "servers.rebuild.return_value": rebuild_server, - "servers.get.return_value": active_server, + "servers.list.return_value": [active_server], "floating_ips.list.return_value": [fake_floating_ip] } OpenStackCloud.nova_client = Mock(**config) @@ -135,7 +135,7 @@ class TestRebuildServer(base.TestCase): meta.obj_to_dict(ret_active_server), cloud_name='cloud-name', region_name='RegionOne'), self.cloud.rebuild_server( - "a", "b", wait=True, admin_pass='ooBootheiX0edoh')) + "1234", "b", wait=True, admin_pass='ooBootheiX0edoh')) def test_rebuild_server_wait(self): """ @@ -150,7 +150,7 @@ class TestRebuildServer(base.TestCase): '5678') config = { "servers.rebuild.return_value": rebuild_server, - "servers.get.return_value": active_server, + "servers.list.return_value": [active_server], "floating_ips.list.return_value": [fake_floating_ip] } OpenStackCloud.nova_client = Mock(**config) @@ -159,4 +159,4 @@ class TestRebuildServer(base.TestCase): _utils.normalize_server( meta.obj_to_dict(active_server), cloud_name='cloud-name', region_name='RegionOne'), - self.cloud.rebuild_server("a", "b", wait=True)) + self.cloud.rebuild_server("1234", "b", wait=True)) diff --git a/shade/tests/unit/test_task_manager.py b/shade/tests/unit/test_task_manager.py index bedd115a8..46531c8e0 100644 --- a/shade/tests/unit/test_task_manager.py +++ b/shade/tests/unit/test_task_manager.py @@ -21,37 +21,37 @@ class TestException(Exception): pass -class TestTask(task_manager.Task): +class TaskTest(task_manager.Task): def main(self, client): raise TestException("This is a test exception") -class TestTaskGenerator(task_manager.Task): +class TaskTestGenerator(task_manager.Task): def main(self, client): yield 1 -class TestTaskInt(task_manager.Task): +class TaskTestInt(task_manager.Task): def main(self, client): return int(1) -class TestTaskFloat(task_manager.Task): +class TaskTestFloat(task_manager.Task): def main(self, client): return float(2.0) -class TestTaskStr(task_manager.Task): +class TaskTestStr(task_manager.Task): def main(self, client): return "test" -class TestTaskBool(task_manager.Task): +class TaskTestBool(task_manager.Task): def main(self, client): return True -class TestTaskSet(task_manager.Task): +class TaskTestSet(task_manager.Task): def main(self, client): return set([1, 2]) @@ -69,24 +69,24 @@ class TestTaskManager(base.TestCase): Specifically, we test if we get the same behaviour with all the configured interpreters (e.g. py27, p34, pypy, ...) """ - self.assertRaises(TestException, self.manager.submit_task, TestTask()) + self.assertRaises(TestException, self.manager.submit_task, TaskTest()) def test_dont_munchify_int(self): - ret = self.manager.submit_task(TestTaskInt()) + ret = self.manager.submit_task(TaskTestInt()) self.assertIsInstance(ret, int) def test_dont_munchify_float(self): - ret = self.manager.submit_task(TestTaskFloat()) + ret = self.manager.submit_task(TaskTestFloat()) self.assertIsInstance(ret, float) def test_dont_munchify_str(self): - ret = self.manager.submit_task(TestTaskStr()) + ret = self.manager.submit_task(TaskTestStr()) self.assertIsInstance(ret, str) def test_dont_munchify_bool(self): - ret = self.manager.submit_task(TestTaskBool()) + ret = self.manager.submit_task(TaskTestBool()) self.assertIsInstance(ret, bool) def test_dont_munchify_set(self): - ret = self.manager.submit_task(TestTaskSet()) + ret = self.manager.submit_task(TaskTestSet()) self.assertIsInstance(ret, set)