From 224d375ef4120998dc51fbf55f1778d1ccf118a0 Mon Sep 17 00:00:00 2001 From: Matt Riedemann <mriedem@us.ibm.com> Date: Fri, 29 May 2015 11:55:09 -0700 Subject: [PATCH] Add --wait to server delete This allows the server delete command to wait for the server to be deleted (obviously). The wait method is the same model that Tempest uses, i.e. wait for a 404 on server GET (successful deletion), fail if the server went to ERROR status, or fail if a timeout is reached. The default timeout of 300 seconds is also what Tempest uses. Closes-Bug: #1460112 Change-Id: I0e66c400903e82832944d1cad61e7eb30177c3e8 --- doc/source/command-objects/server.rst | 6 ++- openstackclient/common/utils.py | 46 ++++++++++++++++++ openstackclient/compute/v2/server.py | 17 +++++++ openstackclient/tests/common/test_utils.py | 39 +++++++++++++++ .../tests/compute/v2/test_server.py | 47 +++++++++++++++++++ 5 files changed, 154 insertions(+), 1 deletion(-) diff --git a/doc/source/command-objects/server.rst b/doc/source/command-objects/server.rst index f9ea590828..10ab306502 100644 --- a/doc/source/command-objects/server.rst +++ b/doc/source/command-objects/server.rst @@ -157,7 +157,11 @@ Delete server(s) .. code:: bash os server delete - <server> [<server> ...] + <server> [<server> ...] [--wait] + +.. option:: --wait + + Wait for delete to complete .. describe:: <server> diff --git a/openstackclient/common/utils.py b/openstackclient/common/utils.py index 4139770cd6..aad0519c1b 100644 --- a/openstackclient/common/utils.py +++ b/openstackclient/common/utils.py @@ -283,6 +283,52 @@ def wait_for_status(status_f, return retval +def wait_for_delete(manager, + res_id, + status_field='status', + sleep_time=5, + timeout=300, + callback=None): + """Wait for resource deletion + + :param res_id: the resource id to watch + :param status_field: the status attribute in the returned resource object, + this is used to check for error states while the resource is being + deleted + :param sleep_time: wait this long between checks (seconds) + :param timeout: check until this long (seconds) + :param callback: called per sleep cycle, useful to display progress; this + function is passed a progress value during each iteration of the wait + loop + :rtype: True on success, False if the resource has gone to error state or + the timeout has been reached + """ + total_time = 0 + while total_time < timeout: + try: + # might not be a bad idea to re-use find_resource here if it was + # a bit more friendly in the exceptions it raised so we could just + # handle a NotFound exception here without parsing the message + res = manager.get(res_id) + except Exception as ex: + if type(ex).__name__ == 'NotFound': + return True + raise + + status = getattr(res, status_field, '').lower() + if status == 'error': + return False + + if callback: + progress = getattr(res, 'progress', None) or 0 + callback(progress) + time.sleep(sleep_time) + total_time += sleep_time + + # if we got this far we've timed out + return False + + def get_effective_log_level(): """Returns the lowest logging level considered by logging handlers diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 41c1b904f6..5007b072ac 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -572,6 +572,11 @@ class DeleteServer(command.Command): nargs="+", help=_('Server(s) to delete (name or ID)'), ) + parser.add_argument( + '--wait', + action='store_true', + help=_('Wait for delete to complete'), + ) return parser def take_action(self, parsed_args): @@ -581,6 +586,18 @@ class DeleteServer(command.Command): server_obj = utils.find_resource( compute_client.servers, server) compute_client.servers.delete(server_obj.id) + if parsed_args.wait: + if utils.wait_for_delete( + compute_client.servers, + server_obj.id, + callback=_show_progress, + ): + sys.stdout.write('\n') + else: + self.log.error(_('Error deleting server: %s'), + server_obj.id) + sys.stdout.write(_('\nError deleting server')) + raise SystemExit return diff --git a/openstackclient/tests/common/test_utils.py b/openstackclient/tests/common/test_utils.py index cda0b1351d..d9f5b7a56e 100644 --- a/openstackclient/tests/common/test_utils.py +++ b/openstackclient/tests/common/test_utils.py @@ -13,6 +13,9 @@ # under the License. # +import time +import uuid + import mock from openstackclient.common import exceptions @@ -120,6 +123,42 @@ class TestUtils(test_utils.TestCase): utils.sort_items, items, sort_str) + @mock.patch.object(time, 'sleep') + def test_wait_for_delete_ok(self, mock_sleep): + # Tests the normal flow that the resource is deleted with a 404 coming + # back on the 2nd iteration of the wait loop. + resource = mock.MagicMock(status='ACTIVE', progress=None) + mock_get = mock.Mock(side_effect=[resource, + exceptions.NotFound(404)]) + manager = mock.MagicMock(get=mock_get) + res_id = str(uuid.uuid4()) + callback = mock.Mock() + self.assertTrue(utils.wait_for_delete(manager, res_id, + callback=callback)) + mock_sleep.assert_called_once_with(5) + callback.assert_called_once_with(0) + + @mock.patch.object(time, 'sleep') + def test_wait_for_delete_timeout(self, mock_sleep): + # Tests that we fail if the resource is not deleted before the timeout. + resource = mock.MagicMock(status='ACTIVE') + mock_get = mock.Mock(return_value=resource) + manager = mock.MagicMock(get=mock_get) + res_id = str(uuid.uuid4()) + self.assertFalse(utils.wait_for_delete(manager, res_id, sleep_time=1, + timeout=1)) + mock_sleep.assert_called_once_with(1) + + @mock.patch.object(time, 'sleep') + def test_wait_for_delete_error(self, mock_sleep): + # Tests that we fail if the resource goes to error state while waiting. + resource = mock.MagicMock(status='ERROR') + mock_get = mock.Mock(return_value=resource) + manager = mock.MagicMock(get=mock_get) + res_id = str(uuid.uuid4()) + self.assertFalse(utils.wait_for_delete(manager, res_id)) + self.assertFalse(mock_sleep.called) + class NoUniqueMatch(Exception): pass diff --git a/openstackclient/tests/compute/v2/test_server.py b/openstackclient/tests/compute/v2/test_server.py index baf53742e5..a8a1936d82 100644 --- a/openstackclient/tests/compute/v2/test_server.py +++ b/openstackclient/tests/compute/v2/test_server.py @@ -16,6 +16,7 @@ import copy import mock +from openstackclient.common import utils as common_utils from openstackclient.compute.v2 import server from openstackclient.tests.compute.v2 import fakes as compute_fakes from openstackclient.tests import fakes @@ -319,6 +320,52 @@ class TestServerDelete(TestServer): compute_fakes.server_id, ) + @mock.patch.object(common_utils, 'wait_for_delete', return_value=True) + def test_server_delete_wait_ok(self, mock_wait_for_delete): + arglist = [ + compute_fakes.server_id, '--wait' + ] + verifylist = [ + ('servers', [compute_fakes.server_id]), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # DisplayCommandBase.take_action() returns two tuples + self.cmd.take_action(parsed_args) + + self.servers_mock.delete.assert_called_with( + compute_fakes.server_id, + ) + + mock_wait_for_delete.assert_called_once_with( + self.servers_mock, + compute_fakes.server_id, + callback=server._show_progress + ) + + @mock.patch.object(common_utils, 'wait_for_delete', return_value=False) + def test_server_delete_wait_fails(self, mock_wait_for_delete): + arglist = [ + compute_fakes.server_id, '--wait' + ] + verifylist = [ + ('servers', [compute_fakes.server_id]), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # DisplayCommandBase.take_action() returns two tuples + self.assertRaises(SystemExit, self.cmd.take_action, parsed_args) + + self.servers_mock.delete.assert_called_with( + compute_fakes.server_id, + ) + + mock_wait_for_delete.assert_called_once_with( + self.servers_mock, + compute_fakes.server_id, + callback=server._show_progress + ) + class TestServerImageCreate(TestServer):