diff options
7 files changed, 210 insertions, 34 deletions
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index cde4ab05..844ac737 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -3536,6 +3536,15 @@ class RebuildServer(command.ShowOne): 'future release.' ) + status = getattr(server, 'status', '').lower() + if status == 'shutoff': + success_status = ['shutoff'] + elif status in ('error', 'active'): + success_status = ['active'] + else: + msg = _("The server status is not ACTIVE, SHUTOFF or ERROR.") + raise exceptions.CommandError(msg) + try: server = server.rebuild(image, parsed_args.password, **kwargs) finally: @@ -3547,6 +3556,7 @@ class RebuildServer(command.ShowOne): compute_client.servers.get, server.id, callback=_show_progress, + success_status=success_status, ): self.app.stdout.write(_('Complete\n')) else: @@ -4721,10 +4731,10 @@ class StartServer(command.Command): for server in parsed_args.server: try: server_id = compute_client.find_server( - name=server, + server, + ignore_missing=False, details=False, all_projects=parsed_args.all_projects, - ignore_missing=False, ).id except sdk_exceptions.HttpException as exc: if exc.status_code == 403: @@ -4761,10 +4771,10 @@ class StopServer(command.Command): for server in parsed_args.server: try: server_id = compute_client.find_server( - name=server, + server, + ignore_missing=False, details=False, all_projects=parsed_args.all_projects, - ignore_missing=False, ).id except sdk_exceptions.HttpException as exc: if exc.status_code == 403: diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index 7ce1ddcb..d04fe31d 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -27,11 +27,11 @@ class ServerTests(common.ComputeTestCase): @classmethod def setUpClass(cls): - super(ServerTests, cls).setUpClass() + super().setUpClass() cls.haz_network = cls.is_service_enabled('network') def test_server_list(self): - """Test server list, set""" + """Test server list""" cmd_output = self.server_create() name1 = cmd_output['name'] cmd_output = self.server_create() @@ -1447,6 +1447,46 @@ class ServerTests(common.ComputeTestCase): raw_output = self.openstack('server volume list ' + server_name) self.assertEqual('\n', raw_output) + def test_server_stop_start(self): + """Test server stop, start""" + server_name = uuid.uuid4().hex + cmd_output = self.openstack( + 'server create ' + + '--network private ' + + '--flavor ' + + self.flavor_name + + ' ' + + '--image ' + + self.image_name + + ' ' + + '--wait ' + + server_name, + parse_output=True, + ) + + self.assertIsNotNone(cmd_output['id']) + self.assertEqual(server_name, cmd_output['name']) + self.addCleanup(self.openstack, 'server delete --wait ' + server_name) + server_id = cmd_output['id'] + + cmd_output = self.openstack( + 'server stop ' + server_name, + ) + self.assertEqual("", cmd_output) + + # This is our test that the request succeeded. If it doesn't transition + # to SHUTOFF then it didn't work. + self.wait_for_status(server_id, "SHUTOFF") + + cmd_output = self.openstack( + 'server start ' + server_name, + ) + self.assertEqual("", cmd_output) + + # As above, this is our test that the request succeeded. If it doesn't + # transition to ACTIVE then it didn't work. + self.wait_for_status(server_id, "ACTIVE") + def test_server_migration_list(self): # Verify that the command does not raise an exception when we list # migrations, including when we specify a query. diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index a4414af2..6abe7759 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -6241,6 +6241,7 @@ class TestServerRebuild(TestServer): # Fake the server to be rebuilt. The IDs of them should be the same. attrs['id'] = new_server.id + attrs['status'] = 'ACTIVE' methods = { 'rebuild': new_server, } @@ -6439,6 +6440,7 @@ class TestServerRebuild(TestServer): self.servers_mock.get, self.server.id, callback=mock.ANY, + success_status=['active'], # **kwargs ) @@ -6464,12 +6466,91 @@ class TestServerRebuild(TestServer): self.servers_mock.get, self.server.id, callback=mock.ANY, + success_status=['active'], ) self.servers_mock.get.assert_called_with(self.server.id) self.get_image_mock.assert_called_with(self.image.id) self.server.rebuild.assert_called_with(self.image, None) + @mock.patch.object(common_utils, 'wait_for_status', return_value=True) + def test_rebuild_with_wait_shutoff_status(self, mock_wait_for_status): + self.server.status = 'SHUTOFF' + arglist = [ + '--wait', + self.server.id, + ] + verifylist = [ + ('wait', True), + ('server', self.server.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # Get the command object to test. + self.cmd.take_action(parsed_args) + + # kwargs = dict(success_status=['active', 'verify_resize'],) + + mock_wait_for_status.assert_called_once_with( + self.servers_mock.get, + self.server.id, + callback=mock.ANY, + success_status=['shutoff'], + # **kwargs + ) + + self.servers_mock.get.assert_called_with(self.server.id) + self.get_image_mock.assert_called_with(self.image.id) + self.server.rebuild.assert_called_with(self.image, None) + + @mock.patch.object(common_utils, 'wait_for_status', return_value=True) + def test_rebuild_with_wait_error_status(self, mock_wait_for_status): + self.server.status = 'ERROR' + arglist = [ + '--wait', + self.server.id, + ] + verifylist = [ + ('wait', True), + ('server', self.server.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + # Get the command object to test. + self.cmd.take_action(parsed_args) + + # kwargs = dict(success_status=['active', 'verify_resize'],) + + mock_wait_for_status.assert_called_once_with( + self.servers_mock.get, + self.server.id, + callback=mock.ANY, + success_status=['active'], + # **kwargs + ) + + self.servers_mock.get.assert_called_with(self.server.id) + self.get_image_mock.assert_called_with(self.image.id) + self.server.rebuild.assert_called_with(self.image, None) + + def test_rebuild_wrong_status_fails(self): + self.server.status = 'SHELVED' + arglist = [ + self.server.id, + ] + verifylist = [ + ('server', self.server.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises( + exceptions.CommandError, self.cmd.take_action, parsed_args + ) + + self.servers_mock.get.assert_called_with(self.server.id) + self.get_image_mock.assert_called_with(self.image.id) + self.server.rebuild.assert_not_called() + def test_rebuild_with_property(self): arglist = [ self.server.id, @@ -6828,6 +6909,7 @@ class TestServerRebuildVolumeBacked(TestServer): # Fake the server to be rebuilt. The IDs of them should be the same. attrs['id'] = new_server.id + attrs['status'] = 'ACTIVE' methods = { 'rebuild': new_server, } @@ -8552,10 +8634,10 @@ class TestServerStart(TestServer): self.cmd.take_action(parsed_args) self.sdk_client.find_server.assert_called_once_with( - name=servers[0].id, + servers[0].id, + ignore_missing=False, details=False, all_projects=True, - ignore_missing=False, ) @@ -8587,10 +8669,10 @@ class TestServerStop(TestServer): self.cmd.take_action(parsed_args) self.sdk_client.find_server.assert_called_once_with( - name=servers[0].id, + servers[0].id, + ignore_missing=False, details=False, all_projects=True, - ignore_missing=False, ) diff --git a/openstackclient/tests/unit/volume/v3/test_volume.py b/openstackclient/tests/unit/volume/v3/test_volume.py index 507edc42..1b298678 100644 --- a/openstackclient/tests/unit/volume/v3/test_volume.py +++ b/openstackclient/tests/unit/volume/v3/test_volume.py @@ -16,9 +16,9 @@ import copy from unittest import mock from cinderclient import api_versions +from openstack import utils as sdk_utils from osc_lib.cli import format_columns from osc_lib import exceptions -from osc_lib import utils from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes from openstackclient.volume.v3 import volume @@ -128,18 +128,36 @@ class TestVolumeRevertToSnapshot(volume_fakes.TestVolume): def setUp(self): super().setUp() - self.volumes_mock = self.app.client_manager.volume.volumes - self.volumes_mock.reset_mock() - self.snapshots_mock = self.app.client_manager.volume.volume_snapshots - self.snapshots_mock.reset_mock() + self.app.client_manager.sdk_connection = mock.Mock() + self.app.client_manager.sdk_connection.volume = mock.Mock() + self.sdk_client = self.app.client_manager.sdk_connection.volume + self.sdk_client.reset_mock() + + patcher = mock.patch.object( + sdk_utils, 'supports_microversion', return_value=True + ) + self.addCleanup(patcher.stop) + self.supports_microversion_mock = patcher.start() + self._set_mock_microversion( + self.app.client_manager.volume.api_version.get_string() + ) + self.mock_volume = volume_fakes.create_one_volume() self.mock_snapshot = volume_fakes.create_one_snapshot( - attrs={'volume_id': self.volumes_mock.id} + attrs={'volume_id': self.mock_volume.id} ) # Get the command object to test self.cmd = volume.VolumeRevertToSnapshot(self.app, None) + def _set_mock_microversion(self, mock_v): + """Set a specific microversion for the mock supports_microversion().""" + self.supports_microversion_mock.reset_mock(return_value=True) + self.supports_microversion_mock.side_effect = ( + lambda _, v: api_versions.APIVersion(v) + <= api_versions.APIVersion(mock_v) + ) + def test_volume_revert_to_snapshot_pre_340(self): arglist = [ self.mock_snapshot.id, @@ -157,9 +175,7 @@ class TestVolumeRevertToSnapshot(volume_fakes.TestVolume): ) def test_volume_revert_to_snapshot(self): - self.app.client_manager.volume.api_version = api_versions.APIVersion( - '3.40' - ) + self._set_mock_microversion('3.40') arglist = [ self.mock_snapshot.id, ] @@ -168,14 +184,22 @@ class TestVolumeRevertToSnapshot(volume_fakes.TestVolume): ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - find_mock_result = [self.mock_snapshot, self.mock_volume] with mock.patch.object( - utils, 'find_resource', side_effect=find_mock_result - ) as find_mock: + self.sdk_client, 'find_volume', return_value=self.mock_volume + ), mock.patch.object( + self.sdk_client, 'find_snapshot', return_value=self.mock_snapshot + ): self.cmd.take_action(parsed_args) - self.volumes_mock.revert_to_snapshot.assert_called_once_with( - volume=self.mock_volume, - snapshot=self.mock_snapshot, + self.sdk_client.revert_volume_to_snapshot.assert_called_once_with( + self.mock_volume, + self.mock_snapshot, + ) + self.sdk_client.find_volume.assert_called_with( + self.mock_volume.id, + ignore_missing=False, + ) + self.sdk_client.find_snapshot.assert_called_with( + self.mock_snapshot.id, + ignore_missing=False, ) - self.assertEqual(2, find_mock.call_count) diff --git a/openstackclient/volume/v3/volume.py b/openstackclient/volume/v3/volume.py index f02053f0..0f2d3c1d 100644 --- a/openstackclient/volume/v3/volume.py +++ b/openstackclient/volume/v3/volume.py @@ -17,6 +17,7 @@ import logging from cinderclient import api_versions +from openstack import utils as sdk_utils from osc_lib.cli import format_columns from osc_lib.command import command from osc_lib import exceptions @@ -96,20 +97,22 @@ class VolumeRevertToSnapshot(command.Command): return parser def take_action(self, parsed_args): - volume_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume - if volume_client.api_version < api_versions.APIVersion('3.40'): + if not sdk_utils.supports_microversion(volume_client, '3.40'): msg = _( "--os-volume-api-version 3.40 or greater is required to " "support the 'volume revert snapshot' command" ) raise exceptions.CommandError(msg) - snapshot = utils.find_resource( - volume_client.volume_snapshots, parsed_args.snapshot + snapshot = volume_client.find_snapshot( + parsed_args.snapshot, + ignore_missing=False, ) - volume = utils.find_resource(volume_client.volumes, snapshot.volume_id) - - volume_client.volumes.revert_to_snapshot( - volume=volume, snapshot=snapshot + volume = volume_client.find_volume( + snapshot.volume_id, + ignore_missing=False, ) + + volume_client.revert_volume_to_snapshot(volume, snapshot) diff --git a/releasenotes/notes/migrate-volume-revert-to-sdk-1e399853d80ba5f8.yaml b/releasenotes/notes/migrate-volume-revert-to-sdk-1e399853d80ba5f8.yaml new file mode 100644 index 00000000..30f12e80 --- /dev/null +++ b/releasenotes/notes/migrate-volume-revert-to-sdk-1e399853d80ba5f8.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + The ``volume revert`` command has been migrated to SDK. diff --git a/releasenotes/notes/story-2010751-server-rebuild-wait-shutoff-c84cddcd3f15e9ce.yaml b/releasenotes/notes/story-2010751-server-rebuild-wait-shutoff-c84cddcd3f15e9ce.yaml new file mode 100644 index 00000000..58c67e27 --- /dev/null +++ b/releasenotes/notes/story-2010751-server-rebuild-wait-shutoff-c84cddcd3f15e9ce.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + ``openstack server rebuild`` command now fails early if the server is + not in a state supported for rebuild - either ``ACTIVE``, ``ERROR`` or + ``SHUTOFF``. + See `OpenStack Compute API reference for server rebuild action + <https://docs.openstack.org/api-ref/compute/?expanded=rebuild-server-rebuild-action-detail#rebuild-server-rebuild-action>`_. +fixes: + - | + ``openstack server rebuild --wait`` now properly works for servers in + ``SHUTOFF`` state without hanging. + [Story `2010751 <https://storyboard.openstack.org/#!/story/2010751>`_] |