From 5c9c1c77a0d534db475cc7d081283e4b0864b86d Mon Sep 17 00:00:00 2001 From: matbu Date: Wed, 15 Jun 2022 14:29:39 +0200 Subject: [PATCH] Wait for volume being available to set bootable or readonly This patch add a check to be sure that the volume created is in a available state before trying to set bootable or readonly flag. Story: 2002158 Change-Id: I8db71fd8cf5bd14eb67880f76d2e9135edeb3ed2 --- .../tests/unit/volume/v1/test_volume.py | 51 ++++++++++++++++-- .../tests/unit/volume/v2/test_volume.py | 53 +++++++++++++++++-- openstackclient/volume/v1/volume.py | 37 +++++++++++-- openstackclient/volume/v2/volume.py | 37 +++++++++++-- 4 files changed, 164 insertions(+), 14 deletions(-) diff --git a/openstackclient/tests/unit/volume/v1/test_volume.py b/openstackclient/tests/unit/volume/v1/test_volume.py index 9f16b398e..b46a608d1 100644 --- a/openstackclient/tests/unit/volume/v1/test_volume.py +++ b/openstackclient/tests/unit/volume/v1/test_volume.py @@ -430,7 +430,8 @@ class TestVolumeCreate(TestVolume): self.assertEqual(self.columns, columns) self.assertCountEqual(self.datalist, data) - def test_volume_create_with_bootable_and_readonly(self): + @mock.patch.object(utils, 'wait_for_status', return_value=True) + def test_volume_create_with_bootable_and_readonly(self, mock_wait): arglist = [ '--bootable', '--read-only', @@ -472,7 +473,8 @@ class TestVolumeCreate(TestVolume): self.volumes_mock.update_readonly_flag.assert_called_with( self.new_volume.id, True) - def test_volume_create_with_nonbootable_and_readwrite(self): + @mock.patch.object(utils, 'wait_for_status', return_value=True) + def test_volume_create_with_nonbootable_and_readwrite(self, mock_wait): arglist = [ '--non-bootable', '--read-write', @@ -515,8 +517,9 @@ class TestVolumeCreate(TestVolume): self.new_volume.id, False) @mock.patch.object(volume.LOG, 'error') + @mock.patch.object(utils, 'wait_for_status', return_value=True) def test_volume_create_with_bootable_and_readonly_fail( - self, mock_error): + self, mock_wait, mock_error): self.volumes_mock.set_bootable.side_effect = ( exceptions.CommandError()) @@ -566,6 +569,48 @@ class TestVolumeCreate(TestVolume): self.volumes_mock.update_readonly_flag.assert_called_with( self.new_volume.id, True) + @mock.patch.object(volume.LOG, 'error') + @mock.patch.object(utils, 'wait_for_status', return_value=False) + def test_volume_create_non_available_with_readonly( + self, mock_wait, mock_error): + arglist = [ + '--non-bootable', + '--read-only', + '--size', str(self.new_volume.size), + self.new_volume.display_name, + ] + verifylist = [ + ('bootable', False), + ('non_bootable', True), + ('read_only', True), + ('read_write', False), + ('size', self.new_volume.size), + ('name', self.new_volume.display_name), + ] + + parsed_args = self.check_parser( + self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.volumes_mock.create.assert_called_with( + self.new_volume.size, + None, + None, + self.new_volume.display_name, + None, + None, + None, + None, + None, + None, + None, + ) + + self.assertEqual(2, mock_error.call_count) + self.assertEqual(self.columns, columns) + self.assertCountEqual(self.datalist, data) + def test_volume_create_without_size(self): arglist = [ self.new_volume.display_name, diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index c930002f3..0419acef1 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -435,7 +435,8 @@ class TestVolumeCreate(TestVolume): self.assertEqual(self.columns, columns) self.assertCountEqual(self.datalist, data) - def test_volume_create_with_bootable_and_readonly(self): + @mock.patch.object(utils, 'wait_for_status', return_value=True) + def test_volume_create_with_bootable_and_readonly(self, mock_wait): arglist = [ '--bootable', '--read-only', @@ -478,7 +479,8 @@ class TestVolumeCreate(TestVolume): self.volumes_mock.update_readonly_flag.assert_called_with( self.new_volume.id, True) - def test_volume_create_with_nonbootable_and_readwrite(self): + @mock.patch.object(utils, 'wait_for_status', return_value=True) + def test_volume_create_with_nonbootable_and_readwrite(self, mock_wait): arglist = [ '--non-bootable', '--read-write', @@ -522,8 +524,9 @@ class TestVolumeCreate(TestVolume): self.new_volume.id, False) @mock.patch.object(volume.LOG, 'error') + @mock.patch.object(utils, 'wait_for_status', return_value=True) def test_volume_create_with_bootable_and_readonly_fail( - self, mock_error): + self, mock_wait, mock_error): self.volumes_mock.set_bootable.side_effect = ( exceptions.CommandError()) @@ -574,6 +577,50 @@ class TestVolumeCreate(TestVolume): self.volumes_mock.update_readonly_flag.assert_called_with( self.new_volume.id, True) + @mock.patch.object(volume.LOG, 'error') + @mock.patch.object(utils, 'wait_for_status', return_value=False) + def test_volume_create_non_available_with_readonly( + self, mock_wait, mock_error, + ): + arglist = [ + '--non-bootable', + '--read-only', + '--size', str(self.new_volume.size), + self.new_volume.name, + ] + verifylist = [ + ('bootable', False), + ('non_bootable', True), + ('read_only', True), + ('read_write', False), + ('size', self.new_volume.size), + ('name', self.new_volume.name), + ] + + parsed_args = self.check_parser( + self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.volumes_mock.create.assert_called_with( + size=self.new_volume.size, + snapshot_id=None, + name=self.new_volume.name, + description=None, + volume_type=None, + availability_zone=None, + metadata=None, + imageRef=None, + source_volid=None, + consistencygroup_id=None, + scheduler_hints=None, + backup_id=None, + ) + + self.assertEqual(2, mock_error.call_count) + self.assertEqual(self.columns, columns) + self.assertCountEqual(self.datalist, data) + def test_volume_create_without_size(self): arglist = [ self.new_volume.name, diff --git a/openstackclient/volume/v1/volume.py b/openstackclient/volume/v1/volume.py index dfbb0c545..198b890f6 100644 --- a/openstackclient/volume/v1/volume.py +++ b/openstackclient/volume/v1/volume.py @@ -224,15 +224,44 @@ class CreateVolume(command.ShowOne): if parsed_args.bootable or parsed_args.non_bootable: try: - volume_client.volumes.set_bootable( - volume.id, parsed_args.bootable) + if utils.wait_for_status( + volume_client.volumes.get, + volume.id, + success_status=['available'], + error_status=['error'], + sleep_time=1 + ): + volume_client.volumes.set_bootable( + volume.id, + parsed_args.bootable + ) + else: + msg = _( + "Volume status is not available for setting boot " + "state" + ) + raise exceptions.CommandError(msg) except Exception as e: LOG.error(_("Failed to set volume bootable property: %s"), e) if parsed_args.read_only or parsed_args.read_write: try: - volume_client.volumes.update_readonly_flag( + if utils.wait_for_status( + volume_client.volumes.get, volume.id, - parsed_args.read_only) + success_status=['available'], + error_status=['error'], + sleep_time=1 + ): + volume_client.volumes.update_readonly_flag( + volume.id, + parsed_args.read_only + ) + else: + msg = _( + "Volume status is not available for setting it" + "read only." + ) + raise exceptions.CommandError(msg) except Exception as e: LOG.error(_("Failed to set volume read-only access " "mode flag: %s"), e) diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py index 7905e0971..a5e5a6703 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -257,15 +257,44 @@ class CreateVolume(command.ShowOne): if parsed_args.bootable or parsed_args.non_bootable: try: - volume_client.volumes.set_bootable( - volume.id, parsed_args.bootable) + if utils.wait_for_status( + volume_client.volumes.get, + volume.id, + success_status=['available'], + error_status=['error'], + sleep_time=1 + ): + volume_client.volumes.set_bootable( + volume.id, + parsed_args.bootable + ) + else: + msg = _( + "Volume status is not available for setting boot " + "state" + ) + raise exceptions.CommandError(msg) except Exception as e: LOG.error(_("Failed to set volume bootable property: %s"), e) if parsed_args.read_only or parsed_args.read_write: try: - volume_client.volumes.update_readonly_flag( + if utils.wait_for_status( + volume_client.volumes.get, volume.id, - parsed_args.read_only) + success_status=['available'], + error_status=['error'], + sleep_time=1 + ): + volume_client.volumes.update_readonly_flag( + volume.id, + parsed_args.read_only + ) + else: + msg = _( + "Volume status is not available for setting it" + "read only." + ) + raise exceptions.CommandError(msg) except Exception as e: LOG.error(_("Failed to set volume read-only access " "mode flag: %s"), e)