diff --git a/doc/source/command-objects/volume-snapshot.rst b/doc/source/command-objects/volume-snapshot.rst index 37a5088a5..8aed1d830 100644 --- a/doc/source/command-objects/volume-snapshot.rst +++ b/doc/source/command-objects/volume-snapshot.rst @@ -133,6 +133,7 @@ Set volume snapshot properties openstack volume snapshot set [--name ] [--description ] + [--no-property] [--property [...] ] [--state ] @@ -145,6 +146,12 @@ Set volume snapshot properties New snapshot description +.. option:: --no-property + + Remove all properties from :ref:`\ ` + (specify both :option:`--no-property` and :option:`--property` to + remove the current properties before setting new properties.) + .. option:: --property Property to add or modify for this snapshot (repeat option to set multiple properties) diff --git a/openstackclient/tests/functional/volume/v1/test_snapshot.py b/openstackclient/tests/functional/volume/v1/test_snapshot.py index 1e1c6b214..89a98661f 100644 --- a/openstackclient/tests/functional/volume/v1/test_snapshot.py +++ b/openstackclient/tests/functional/volume/v1/test_snapshot.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +import json import time import uuid @@ -20,9 +21,6 @@ class VolumeSnapshotTests(common.BaseVolumeTests): """Functional tests for volume snapshot. """ VOLLY = uuid.uuid4().hex - NAME = uuid.uuid4().hex - OTHER_NAME = uuid.uuid4().hex - HEADERS = ['"Name"'] @classmethod def wait_for_status(cls, command, status, tries): @@ -30,59 +28,223 @@ class VolumeSnapshotTests(common.BaseVolumeTests): for attempt in range(tries): time.sleep(1) raw_output = cls.openstack(command + opts) - if (raw_output == status): + if (raw_output.rstrip() == status): return cls.assertOutput(status, raw_output) @classmethod def setUpClass(cls): super(VolumeSnapshotTests, cls).setUpClass() - cls.openstack('volume create --size 1 ' + cls.VOLLY) - cls.wait_for_status('volume show ' + cls.VOLLY, 'available\n', 3) - opts = cls.get_opts(['status']) - raw_output = cls.openstack('volume snapshot create --volume ' + - cls.VOLLY + ' ' + cls.NAME + opts) - cls.assertOutput('creating\n', raw_output) - cls.wait_for_status( - 'volume snapshot show ' + cls.NAME, 'available\n', 3) + # create a volume for all tests to create snapshot + cmd_output = json.loads(cls.openstack( + 'volume create -f json ' + + '--size 1 ' + + cls.VOLLY + )) + cls.wait_for_status('volume show ' + cls.VOLLY, 'available', 6) + cls.VOLUME_ID = cmd_output['id'] @classmethod def tearDownClass(cls): - # Rename test - raw_output = cls.openstack( - 'volume snapshot set --name ' + cls.OTHER_NAME + ' ' + cls.NAME) + cls.wait_for_status('volume show ' + cls.VOLLY, 'available', 6) + raw_output = cls.openstack('volume delete --force ' + cls.VOLLY) cls.assertOutput('', raw_output) - # Delete test - raw_output_snapshot = cls.openstack( - 'volume snapshot delete ' + cls.OTHER_NAME) - cls.wait_for_status('volume show ' + cls.VOLLY, 'available\n', 6) - raw_output_volume = cls.openstack('volume delete --force ' + cls.VOLLY) - cls.assertOutput('', raw_output_snapshot) - cls.assertOutput('', raw_output_volume) - def test_snapshot_list(self): - opts = self.get_opts(self.HEADERS) - raw_output = self.openstack('volume snapshot list' + opts) - self.assertIn(self.NAME, raw_output) + def test_volume_snapshot__delete(self): + """Test create, delete multiple""" + name1 = uuid.uuid4().hex + cmd_output = json.loads(self.openstack( + 'volume snapshot create -f json ' + + name1 + + ' --volume ' + self.VOLLY + )) + self.assertEqual( + name1, + cmd_output["display_name"], + ) - def test_snapshot_set_unset_properties(self): + name2 = uuid.uuid4().hex + cmd_output = json.loads(self.openstack( + 'volume snapshot create -f json ' + + name2 + + ' --volume ' + self.VOLLY + )) + self.assertEqual( + name2, + cmd_output["display_name"], + ) + + self.wait_for_status( + 'volume snapshot show ' + name1, 'available', 6) + self.wait_for_status( + 'volume snapshot show ' + name2, 'available', 6) + + del_output = self.openstack( + 'volume snapshot delete ' + name1 + ' ' + name2) + self.assertOutput('', del_output) + + def test_volume_snapshot_list(self): + """Test create, list filter""" + name1 = uuid.uuid4().hex + cmd_output = json.loads(self.openstack( + 'volume snapshot create -f json ' + + name1 + + ' --volume ' + self.VOLLY + )) + self.addCleanup(self.openstack, 'volume snapshot delete ' + name1) + self.assertEqual( + name1, + cmd_output["display_name"], + ) + self.assertEqual( + self.VOLUME_ID, + cmd_output["volume_id"], + ) + self.assertEqual( + 1, + cmd_output["size"], + ) + self.wait_for_status( + 'volume snapshot show ' + name1, 'available', 6) + + name2 = uuid.uuid4().hex + cmd_output = json.loads(self.openstack( + 'volume snapshot create -f json ' + + name2 + + ' --volume ' + self.VOLLY + )) + self.addCleanup(self.openstack, 'volume snapshot delete ' + name2) + self.assertEqual( + name2, + cmd_output["display_name"], + ) + self.assertEqual( + self.VOLUME_ID, + cmd_output["volume_id"], + ) + self.assertEqual( + 1, + cmd_output["size"], + ) + self.wait_for_status( + 'volume snapshot show ' + name2, 'available', 6) + + # Test list --long, --status + cmd_output = json.loads(self.openstack( + 'volume snapshot list -f json ' + + '--long ' + + '--status error' + )) + names = [x["Name"] for x in cmd_output] + self.assertNotIn(name1, names) + self.assertNotIn(name2, names) + + # Test list --volume + cmd_output = json.loads(self.openstack( + 'volume snapshot list -f json ' + + '--volume ' + self.VOLLY + )) + names = [x["Name"] for x in cmd_output] + self.assertIn(name1, names) + self.assertIn(name2, names) + + # Test list --name + cmd_output = json.loads(self.openstack( + 'volume snapshot list -f json ' + + '--name ' + name1 + )) + names = [x["Name"] for x in cmd_output] + self.assertIn(name1, names) + self.assertNotIn(name2, names) + + def test_snapshot_set(self): + """Test create, set, unset, show, delete volume snapshot""" + name = uuid.uuid4().hex + new_name = name + "_" + cmd_output = json.loads(self.openstack( + 'volume snapshot create -f json ' + + '--volume ' + self.VOLLY + + ' --description aaaa ' + + name + )) + self.addCleanup(self.openstack, 'volume snapshot delete ' + new_name) + self.assertEqual( + name, + cmd_output["display_name"], + ) + self.assertEqual( + 1, + cmd_output["size"], + ) + self.assertEqual( + 'aaaa', + cmd_output["display_description"], + ) + self.wait_for_status( + 'volume snapshot show ' + name, 'available', 6) + + # Test volume snapshot set raw_output = self.openstack( - 'volume snapshot set --property a=b --property c=d ' + self.NAME) - self.assertEqual("", raw_output) - opts = self.get_opts(["properties"]) - raw_output = self.openstack('volume snapshot show ' + self.NAME + opts) - self.assertEqual("a='b', c='d'\n", raw_output) + 'volume snapshot set ' + + '--name ' + new_name + + ' --description bbbb ' + + '--property Alpha=a ' + + '--property Beta=b ' + + name, + ) + self.assertOutput('', raw_output) - raw_output = self.openstack( - 'volume snapshot unset --property a ' + self.NAME) - self.assertEqual("", raw_output) - raw_output = self.openstack('volume snapshot show ' + self.NAME + opts) - self.assertEqual("c='d'\n", raw_output) + # Show snapshot set result + cmd_output = json.loads(self.openstack( + 'volume snapshot show -f json ' + + new_name + )) + self.assertEqual( + new_name, + cmd_output["display_name"], + ) + self.assertEqual( + 1, + cmd_output["size"], + ) + self.assertEqual( + 'bbbb', + cmd_output["display_description"], + ) + self.assertEqual( + "Alpha='a', Beta='b'", + cmd_output["properties"], + ) - def test_snapshot_set_description(self): + # Test volume unset raw_output = self.openstack( - 'volume snapshot set --description backup ' + self.NAME) - self.assertEqual("", raw_output) - opts = self.get_opts(["display_description", "display_name"]) - raw_output = self.openstack('volume snapshot show ' + self.NAME + opts) - self.assertEqual("backup\n" + self.NAME + "\n", raw_output) + 'volume snapshot unset ' + + '--property Alpha ' + + new_name, + ) + self.assertOutput('', raw_output) + + cmd_output = json.loads(self.openstack( + 'volume snapshot show -f json ' + + new_name + )) + self.assertEqual( + "Beta='b'", + cmd_output["properties"], + ) + + # Test volume snapshot set --no-property + raw_output = self.openstack( + 'volume snapshot set ' + + '--no-property ' + + new_name, + ) + self.assertOutput('', raw_output) + cmd_output = json.loads(self.openstack( + 'volume snapshot show -f json ' + + new_name + )) + self.assertNotIn( + "Beta='b'", + cmd_output["properties"], + ) diff --git a/openstackclient/tests/functional/volume/v2/test_snapshot.py b/openstackclient/tests/functional/volume/v2/test_snapshot.py index c83e79f87..422e5b7ce 100644 --- a/openstackclient/tests/functional/volume/v2/test_snapshot.py +++ b/openstackclient/tests/functional/volume/v2/test_snapshot.py @@ -28,7 +28,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests): for attempt in range(tries): time.sleep(1) raw_output = cls.openstack(command + opts) - if (raw_output == status): + if (raw_output.rstrip() == status): return cls.assertOutput(status, raw_output) @@ -41,12 +41,12 @@ class VolumeSnapshotTests(common.BaseVolumeTests): '--size 1 ' + cls.VOLLY )) - cls.wait_for_status('volume show ' + cls.VOLLY, 'available\n', 6) + cls.wait_for_status('volume show ' + cls.VOLLY, 'available', 6) cls.VOLUME_ID = cmd_output['id'] @classmethod def tearDownClass(cls): - cls.wait_for_status('volume show ' + cls.VOLLY, 'available\n', 6) + cls.wait_for_status('volume show ' + cls.VOLLY, 'available', 6) raw_output = cls.openstack('volume delete --force ' + cls.VOLLY) cls.assertOutput('', raw_output) @@ -75,9 +75,9 @@ class VolumeSnapshotTests(common.BaseVolumeTests): ) self.wait_for_status( - 'volume snapshot show ' + name1, 'available\n', 6) + 'volume snapshot show ' + name1, 'available', 6) self.wait_for_status( - 'volume snapshot show ' + name2, 'available\n', 6) + 'volume snapshot show ' + name2, 'available', 6) del_output = self.openstack( 'volume snapshot delete ' + name1 + ' ' + name2) @@ -105,7 +105,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests): cmd_output["size"], ) self.wait_for_status( - 'volume snapshot show ' + name1, 'available\n', 6) + 'volume snapshot show ' + name1, 'available', 6) name2 = uuid.uuid4().hex cmd_output = json.loads(self.openstack( @@ -127,7 +127,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests): cmd_output["size"], ) self.wait_for_status( - 'volume snapshot show ' + name2, 'available\n', 6) + 'volume snapshot show ' + name2, 'available', 6) raw_output = self.openstack( 'volume snapshot set ' + '--state error ' + @@ -163,7 +163,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests): self.assertIn(name1, names) self.assertNotIn(name2, names) - def test_snapshot_set(self): + def test_volume_snapshot_set(self): """Test create, set, unset, show, delete volume snapshot""" name = uuid.uuid4().hex new_name = name + "_" @@ -192,7 +192,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests): cmd_output["properties"], ) self.wait_for_status( - 'volume snapshot show ' + name, 'available\n', 6) + 'volume snapshot show ' + name, 'available', 6) # Test volume snapshot set raw_output = self.openstack( @@ -227,7 +227,7 @@ class VolumeSnapshotTests(common.BaseVolumeTests): cmd_output["properties"], ) - # Test volume unset + # Test volume snapshot unset raw_output = self.openstack( 'volume snapshot unset ' + '--property Alpha ' + @@ -243,3 +243,19 @@ class VolumeSnapshotTests(common.BaseVolumeTests): "Beta='b'", cmd_output["properties"], ) + + # Test volume snapshot set --no-property + raw_output = self.openstack( + 'volume snapshot set ' + + '--no-property ' + + new_name, + ) + self.assertOutput('', raw_output) + cmd_output = json.loads(self.openstack( + 'volume snapshot show -f json ' + + new_name + )) + self.assertNotIn( + "Beta='b'", + cmd_output["properties"], + ) diff --git a/openstackclient/tests/unit/volume/v1/test_snapshot.py b/openstackclient/tests/unit/volume/v1/test_snapshot.py index fd878f453..87a62b0a9 100644 --- a/openstackclient/tests/unit/volume/v1/test_snapshot.py +++ b/openstackclient/tests/unit/volume/v1/test_snapshot.py @@ -429,15 +429,17 @@ class TestSnapshotSet(TestSnapshot): arglist = [ "--name", "new_snapshot", "--description", "new_description", - "--property", "x=y", - "--property", "foo=foo", + "--property", "foo_1=foo_1", + "--property", "foo_2=foo_2", + "--no-property", self.snapshot.id, ] - new_property = {"x": "y", "foo": "foo"} + new_property = {"foo_1": "foo_1", "foo_2": "foo_2"} verifylist = [ ("name", "new_snapshot"), ("description", "new_description"), ("property", new_property), + ("no_property", True), ("snapshot", self.snapshot.id), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -449,8 +451,11 @@ class TestSnapshotSet(TestSnapshot): "display_description": "new_description", } self.snapshot.update.assert_called_with(**kwargs) + self.snapshots_mock.delete_metadata.assert_called_with( + self.snapshot.id, ["foo"] + ) self.snapshots_mock.set_metadata.assert_called_with( - self.snapshot.id, new_property + self.snapshot.id, {"foo_2": "foo_2", "foo_1": "foo_1"} ) self.assertIsNone(result) diff --git a/openstackclient/tests/unit/volume/v2/test_snapshot.py b/openstackclient/tests/unit/volume/v2/test_snapshot.py index 8ce356aea..f3a6ed3ce 100644 --- a/openstackclient/tests/unit/volume/v2/test_snapshot.py +++ b/openstackclient/tests/unit/volume/v2/test_snapshot.py @@ -499,7 +499,23 @@ class TestSnapshotSet(TestSnapshot): # Get the command object to mock self.cmd = volume_snapshot.SetVolumeSnapshot(self.app, None) - def test_snapshot_set(self): + def test_snapshot_set_no_option(self): + arglist = [ + self.snapshot.id, + ] + verifylist = [ + ("snapshot", self.snapshot.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + self.snapshots_mock.get.assert_called_once_with(parsed_args.snapshot) + self.assertNotCalled(self.snapshots_mock.reset_state) + self.assertNotCalled(self.snapshots_mock.update) + self.assertNotCalled(self.snapshots_mock.set_metadata) + self.assertIsNone(result) + + def test_snapshot_set_name_and_property(self): arglist = [ "--name", "new_snapshot", "--property", "x=y", @@ -526,6 +542,51 @@ class TestSnapshotSet(TestSnapshot): ) self.assertIsNone(result) + def test_snapshot_set_with_no_property(self): + arglist = [ + "--no-property", + self.snapshot.id, + ] + verifylist = [ + ("no_property", True), + ("snapshot", self.snapshot.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + self.snapshots_mock.get.assert_called_once_with(parsed_args.snapshot) + self.assertNotCalled(self.snapshots_mock.reset_state) + self.assertNotCalled(self.snapshots_mock.update) + self.assertNotCalled(self.snapshots_mock.set_metadata) + self.snapshots_mock.delete_metadata.assert_called_with( + self.snapshot.id, ["foo"] + ) + self.assertIsNone(result) + + def test_snapshot_set_with_no_property_and_property(self): + arglist = [ + "--no-property", + "--property", "foo_1=bar_1", + self.snapshot.id, + ] + verifylist = [ + ("no_property", True), + ("property", {"foo_1": "bar_1"}), + ("snapshot", self.snapshot.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + result = self.cmd.take_action(parsed_args) + self.snapshots_mock.get.assert_called_once_with(parsed_args.snapshot) + self.assertNotCalled(self.snapshots_mock.reset_state) + self.assertNotCalled(self.snapshots_mock.update) + self.snapshots_mock.delete_metadata.assert_called_with( + self.snapshot.id, ["foo"] + ) + self.snapshots_mock.set_metadata.assert_called_once_with( + self.snapshot.id, {"foo_1": "bar_1"}) + self.assertIsNone(result) + def test_snapshot_set_state_to_error(self): arglist = [ "--state", "error", diff --git a/openstackclient/volume/v1/volume_snapshot.py b/openstackclient/volume/v1/volume_snapshot.py index 45bd30c04..f22c338b8 100644 --- a/openstackclient/volume/v1/volume_snapshot.py +++ b/openstackclient/volume/v1/volume_snapshot.py @@ -239,6 +239,15 @@ class SetVolumeSnapshot(command.Command): metavar='', help=_('New snapshot description') ) + parser.add_argument( + "--no-property", + dest="no_property", + action="store_true", + help=_("Remove all properties from " + "(specify both --no-property and --property to " + "remove the current properties before setting " + "new properties.)"), + ) parser.add_argument( '--property', metavar='', @@ -254,6 +263,17 @@ class SetVolumeSnapshot(command.Command): parsed_args.snapshot) result = 0 + if parsed_args.no_property: + try: + key_list = snapshot.metadata.keys() + volume_client.volume_snapshots.delete_metadata( + snapshot.id, + list(key_list), + ) + except Exception as e: + LOG.error(_("Failed to clean snapshot properties: %s"), e) + result += 1 + if parsed_args.property: try: volume_client.volume_snapshots.set_metadata( diff --git a/openstackclient/volume/v2/volume_snapshot.py b/openstackclient/volume/v2/volume_snapshot.py index af29b777d..3c06fa5ac 100644 --- a/openstackclient/volume/v2/volume_snapshot.py +++ b/openstackclient/volume/v2/volume_snapshot.py @@ -285,6 +285,15 @@ class SetVolumeSnapshot(command.Command): metavar='', help=_('New snapshot description') ) + parser.add_argument( + "--no-property", + dest="no_property", + action="store_true", + help=_("Remove all properties from " + "(specify both --no-property and --property to " + "remove the current properties before setting " + "new properties.)"), + ) parser.add_argument( '--property', metavar='', @@ -311,6 +320,17 @@ class SetVolumeSnapshot(command.Command): parsed_args.snapshot) result = 0 + if parsed_args.no_property: + try: + key_list = snapshot.metadata.keys() + volume_client.volume_snapshots.delete_metadata( + snapshot.id, + list(key_list), + ) + except Exception as e: + LOG.error(_("Failed to clean snapshot properties: %s"), e) + result += 1 + if parsed_args.property: try: volume_client.volume_snapshots.set_metadata( diff --git a/releasenotes/notes/support-no-property-in-volume-snapshot-0af3fcb31a3cfc2b.yaml b/releasenotes/notes/support-no-property-in-volume-snapshot-0af3fcb31a3cfc2b.yaml new file mode 100644 index 000000000..6a3220b24 --- /dev/null +++ b/releasenotes/notes/support-no-property-in-volume-snapshot-0af3fcb31a3cfc2b.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Add ``--no-property`` option in ``volume snapshot set``. + Supporting ``--no-property`` option will apply user a convenient way to + clean all properties of volume snapshot in a short command. + [ Blueprint `allow-overwrite-set-options ` _]