From cfd62e4bbd44795cd3fafa623d43afc43b2311ce Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Mon, 14 Mar 2016 18:39:57 -0400 Subject: [PATCH] Add wait_for_server API call. We need to be able to wait for a server to reach active status. This was the intended usage of get_active_server(), but it doesn't do any waiting, which is a bummer. Change-Id: I5be0b1cf6f1b910b6d861d81fc1083e4793b9e5f --- .../wait_for_server-8dc8446b7c673d36.yaml | 3 + shade/openstackcloud.py | 75 ++++++----- shade/tests/unit/test_create_server.py | 126 ++++++++++-------- 3 files changed, 119 insertions(+), 85 deletions(-) create mode 100644 releasenotes/notes/wait_for_server-8dc8446b7c673d36.yaml diff --git a/releasenotes/notes/wait_for_server-8dc8446b7c673d36.yaml b/releasenotes/notes/wait_for_server-8dc8446b7c673d36.yaml new file mode 100644 index 000000000..58bc54c5c --- /dev/null +++ b/releasenotes/notes/wait_for_server-8dc8446b7c673d36.yaml @@ -0,0 +1,3 @@ +--- +features: + - New wait_for_server() API call to wait for a server to reach ACTIVE status. diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index 769a2c028..55dba1d9f 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -3474,7 +3474,6 @@ class OpenStackCloud(object): with _utils.shade_exceptions("Error in creating instance"): server = self.manager.submitTask(_tasks.ServerCreate( name=name, flavor=flavor, **kwargs)) - server_id = server.id admin_pass = server.get('adminPass') or kwargs.get('admin_pass') if not wait: # This is a direct get task call to skip the list_servers @@ -3489,41 +3488,55 @@ class OpenStackCloud(object): if server.status == 'ERROR': raise OpenStackCloudException( "Error in creating the server.") + if wait: - timeout_message = "Timeout waiting for the server to come up." - start_time = time.time() - # There is no point in iterating faster than the list_servers cache - for count in _utils._iterate_timeout( - timeout, - timeout_message, - wait=self._SERVER_AGE): - try: - # Use the get_server call so that the list_servers - # cache can be leveraged - server = self.get_server(server_id) - except Exception: - continue - if not server: - continue - - # We have more work to do, but the details of that are - # hidden from the user. So, calculate remaining timeout - # and pass it down into the IP stack. - remaining_timeout = timeout - int(time.time() - start_time) - if remaining_timeout <= 0: - raise OpenStackCloudTimeout(timeout_message) - - server = self.get_active_server( - server=server, reuse=reuse_ips, - auto_ip=auto_ip, ips=ips, ip_pool=ip_pool, - wait=wait, timeout=remaining_timeout) - if server: - server.adminPass = admin_pass - return server + server = self.wait_for_server( + server, auto_ip=auto_ip, ips=ips, ip_pool=ip_pool, + reuse=reuse_ips, timeout=timeout + ) server.adminPass = admin_pass return server + def wait_for_server( + self, server, auto_ip=True, ips=None, ip_pool=None, + reuse=True, timeout=180): + """ + Wait for a server to reach ACTIVE status. + """ + server_id = server['id'] + timeout_message = "Timeout waiting for the server to come up." + start_time = time.time() + + # There is no point in iterating faster than the list_servers cache + for count in _utils._iterate_timeout( + timeout, + timeout_message, + wait=self._SERVER_AGE): + try: + # Use the get_server call so that the list_servers + # cache can be leveraged + server = self.get_server(server_id) + except Exception: + continue + if not server: + continue + + # We have more work to do, but the details of that are + # hidden from the user. So, calculate remaining timeout + # and pass it down into the IP stack. + remaining_timeout = timeout - int(time.time() - start_time) + if remaining_timeout <= 0: + raise OpenStackCloudTimeout(timeout_message) + + server = self.get_active_server( + server=server, reuse=reuse, + auto_ip=auto_ip, ips=ips, ip_pool=ip_pool, + wait=True, timeout=remaining_timeout) + + if server is not None and server['status'] == 'ACTIVE': + return server + def get_active_server( self, server, auto_ip=True, ips=None, ip_pool=None, reuse=True, wait=False, timeout=180): diff --git a/shade/tests/unit/test_create_server.py b/shade/tests/unit/test_create_server.py index 5ff0b1d7e..66e415425 100644 --- a/shade/tests/unit/test_create_server.py +++ b/shade/tests/unit/test_create_server.py @@ -20,6 +20,7 @@ Tests for the `create_server` command. """ from mock import patch, Mock +import mock import os_client_config from shade import _utils from shade import meta @@ -164,67 +165,84 @@ class TestCreateServer(base.TestCase): name='server-name', image='image=id', flavor='flavor-id', admin_pass='ooBootheiX0edoh')) - def test_create_server_with_admin_pass_wait(self): + @patch.object(OpenStackCloud, "wait_for_server") + @patch.object(OpenStackCloud, "nova_client") + def test_create_server_with_admin_pass_wait(self, mock_nova, mock_wait): """ Test that a server with an admin_pass passed returns the password """ - with patch("shade.OpenStackCloud"): - build_server = fakes.FakeServer( - '1234', '', 'BUILD', addresses=dict(public='1.1.1.1'), - adminPass='ooBootheiX0edoh') - next_server = fakes.FakeServer( - '1234', '', 'BUILD', addresses=dict(public='1.1.1.1')) - fake_server = fakes.FakeServer( - '1234', '', 'ACTIVE', addresses=dict(public='1.1.1.1')) - ret_fake_server = fakes.FakeServer( - '1234', '', 'ACTIVE', addresses=dict(public='1.1.1.1'), - adminPass='ooBootheiX0edoh') - config = { - "servers.create.return_value": build_server, - "servers.get.return_value": next_server, - "servers.list.side_effect": [ - [next_server], [fake_server]] - } - OpenStackCloud.nova_client = Mock(**config) - with patch.object(OpenStackCloud, "add_ips_to_server", - return_value=fake_server): - self.assertEqual( - _utils.normalize_server( - meta.obj_to_dict(ret_fake_server), - cloud_name=self.client.name, - region_name=self.client.region_name), - _utils.normalize_server( - meta.obj_to_dict( - self.client.create_server( - 'server-name', 'image-id', 'flavor-id', - wait=True, admin_pass='ooBootheiX0edoh')), - cloud_name=self.client.name, - region_name=self.client.region_name) - ) + fake_server = fakes.FakeServer('1234', '', 'BUILD') + fake_server_with_pass = fakes.FakeServer('1234', '', 'BUILD', + adminPass='ooBootheiX0edoh') - def test_create_server_wait(self): + mock_nova.servers.create.return_value = fake_server + mock_nova.servers.get.return_value = fake_server + # The wait returns non-password server + mock_wait.return_value = _utils.normalize_server( + meta.obj_to_dict(fake_server), None, None) + + server = self.client.create_server( + name='server-name', image='image-id', + flavor='flavor-id', admin_pass='ooBootheiX0edoh', wait=True) + + # Assert that we did wait + self.assertTrue(mock_wait.called) + + # Even with the wait, we should still get back a passworded server + self.assertEqual( + server, + _utils.normalize_server(meta.obj_to_dict(fake_server_with_pass), + None, None) + ) + + @patch.object(OpenStackCloud, "get_active_server") + @patch.object(OpenStackCloud, "get_server") + def test_wait_for_server(self, mock_get_server, mock_get_active_server): """ - Test that create_server with a wait returns the server instance when + Test that waiting for a server returns the server instance when its status changes to "ACTIVE". """ - with patch("shade.OpenStackCloud"): - build_server = fakes.FakeServer( - '1234', '', 'ACTIVE', addresses=dict(public='1.1.1.1')) - fake_server = fakes.FakeServer( - '1234', '', 'ACTIVE', addresses=dict(public='1.1.1.1')) - config = { - "servers.create.return_value": build_server, - "servers.get.return_value": build_server, - "servers.list.side_effect": [ - [build_server], [fake_server]] - } - OpenStackCloud.nova_client = Mock(**config) - with patch.object(OpenStackCloud, "add_ips_to_server", - return_value=fake_server): - self.assertEqual( - self.client.create_server( - 'server-name', 'image-id', 'flavor-id', wait=True), - fake_server) + building_server = {'id': 'fake_server_id', 'status': 'BUILDING'} + active_server = {'id': 'fake_server_id', 'status': 'ACTIVE'} + + mock_get_server.side_effect = iter([building_server, active_server]) + mock_get_active_server.side_effect = iter([ + building_server, active_server]) + + server = self.client.wait_for_server(building_server) + + self.assertEqual(2, mock_get_server.call_count) + mock_get_server.assert_has_calls([ + mock.call(building_server['id']), + mock.call(active_server['id']), + ]) + + self.assertEqual(2, mock_get_active_server.call_count) + mock_get_active_server.assert_has_calls([ + mock.call(server=building_server, reuse=True, auto_ip=True, + ips=None, ip_pool=None, wait=True, timeout=mock.ANY), + mock.call(server=active_server, reuse=True, auto_ip=True, + ips=None, ip_pool=None, wait=True, timeout=mock.ANY), + ]) + + self.assertEqual('ACTIVE', server['status']) + + @patch.object(OpenStackCloud, 'wait_for_server') + @patch.object(OpenStackCloud, 'nova_client') + def test_create_server_wait(self, mock_nova, mock_wait): + """ + Test that create_server with a wait actually does the wait. + """ + fake_server = {'id': 'fake_server_id', 'status': 'BUILDING'} + mock_nova.servers.create.return_value = fake_server + + self.client.create_server( + 'server-name', 'image-id', 'flavor-id', wait=True), + + mock_wait.assert_called_once_with( + fake_server, auto_ip=True, ips=None, + ip_pool=None, reuse=True, timeout=180 + ) @patch('time.sleep') def test_create_server_no_addresses(self, mock_sleep):