Merge "Deprecate volume create --project and --user options"

3 files changed, 41 insertions, 98 deletions

diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py
index 971567cb..bb6263bb 100644
--- a/openstackclient/tests/unit/volume/v2/test_volume.py
+++ b/openstackclient/tests/unit/volume/v2/test_volume.py
@@ -126,8 +126,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=None,
             volume_type=None,
-            user_id=None,
-            project_id=None,
             availability_zone=None,
             metadata=None,
             imageRef=None,
@@ -177,8 +175,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=self.new_volume.description,
             volume_type=self.new_volume.volume_type,
-            user_id=None,
-            project_id=None,
             availability_zone=self.new_volume.availability_zone,
             metadata=None,
             imageRef=None,
@@ -191,95 +187,39 @@ class TestVolumeCreate(TestVolume):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.datalist, data)
 
-    def test_volume_create_user_project_id(self):
-        # Return a project
-        self.projects_mock.get.return_value = self.project
-        # Return a user
-        self.users_mock.get.return_value = self.user
-
+    def test_volume_create_user(self):
         arglist = [
             '--size', str(self.new_volume.size),
-            '--project', self.project.id,
             '--user', self.user.id,
             self.new_volume.name,
         ]
         verifylist = [
             ('size', self.new_volume.size),
-            ('project', self.project.id),
             ('user', self.user.id),
             ('name', self.new_volume.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        # In base command class ShowOne in cliff, abstract method take_action()
-        # returns a two-part tuple with a tuple of column names and a tuple of
-        # data to be shown.
-        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,
-            user_id=self.user.id,
-            project_id=self.project.id,
-            availability_zone=None,
-            metadata=None,
-            imageRef=None,
-            source_volid=None,
-            consistencygroup_id=None,
-            multiattach=False,
-            scheduler_hints=None,
-        )
-
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
-
-    def test_volume_create_user_project_name(self):
-        # Return a project
-        self.projects_mock.get.return_value = self.project
-        # Return a user
-        self.users_mock.get.return_value = self.user
+        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+                          parsed_args)
+        self.volumes_mock.create.assert_not_called()
 
+    def test_volume_create_project(self):
         arglist = [
             '--size', str(self.new_volume.size),
-            '--project', self.project.name,
-            '--user', self.user.name,
+            '--project', self.project.id,
             self.new_volume.name,
         ]
         verifylist = [
             ('size', self.new_volume.size),
-            ('project', self.project.name),
-            ('user', self.user.name),
+            ('project', self.project.id),
             ('name', self.new_volume.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        # In base command class ShowOne in cliff, abstract method take_action()
-        # returns a two-part tuple with a tuple of column names and a tuple of
-        # data to be shown.
-        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,
-            user_id=self.user.id,
-            project_id=self.project.id,
-            availability_zone=None,
-            metadata=None,
-            imageRef=None,
-            source_volid=None,
-            consistencygroup_id=None,
-            multiattach=False,
-            scheduler_hints=None,
-        )
-
-        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.datalist, data)
+        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+                          parsed_args)
+        self.volumes_mock.create.assert_not_called()
 
     def test_volume_create_properties(self):
         arglist = [
@@ -306,8 +246,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=None,
             volume_type=None,
-            user_id=None,
-            project_id=None,
             availability_zone=None,
             metadata={'Alpha': 'a', 'Beta': 'b'},
             imageRef=None,
@@ -347,8 +285,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=None,
             volume_type=None,
-            user_id=None,
-            project_id=None,
             availability_zone=None,
             metadata=None,
             imageRef=image.id,
@@ -388,8 +324,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=None,
             volume_type=None,
-            user_id=None,
-            project_id=None,
             availability_zone=None,
             metadata=None,
             imageRef=image.id,
@@ -428,8 +362,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=None,
             volume_type=None,
-            user_id=None,
-            project_id=None,
             availability_zone=None,
             metadata=None,
             imageRef=None,
@@ -469,8 +401,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=None,
             volume_type=None,
-            user_id=None,
-            project_id=None,
             availability_zone=None,
             metadata=None,
             imageRef=None,
@@ -514,8 +444,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=None,
             volume_type=None,
-            user_id=None,
-            project_id=None,
             availability_zone=None,
             metadata=None,
             imageRef=None,
@@ -568,8 +496,6 @@ class TestVolumeCreate(TestVolume):
             name=self.new_volume.name,
             description=None,
             volume_type=None,
-            user_id=None,
-            project_id=None,
             availability_zone=None,
             metadata=None,
             imageRef=None,
diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py
index fc648cef..8ab61d2a 100644
--- a/openstackclient/volume/v2/volume.py
+++ b/openstackclient/volume/v2/volume.py
@@ -96,12 +96,12 @@ class CreateVolume(command.ShowOne):
         parser.add_argument(
             '--user',
             metavar='<user>',
-            help=_('Specify an alternate user (name or ID)'),
+            help=argparse.SUPPRESS,
         )
         parser.add_argument(
             '--project',
             metavar='<project>',
-            help=_('Specify an alternate project (name or ID)'),
+            help=argparse.SUPPRESS,
         )
         parser.add_argument(
             "--availability-zone",
@@ -159,7 +159,6 @@ class CreateVolume(command.ShowOne):
 
     def take_action(self, parsed_args):
         _check_size_arg(parsed_args)
-        identity_client = self.app.client_manager.identity
         volume_client = self.app.client_manager.volume
         image_client = self.app.client_manager.image
 
@@ -197,17 +196,22 @@ class CreateVolume(command.ShowOne):
             # snapshot size.
             size = max(size or 0, snapshot_obj.size)
 
-        project = None
+        # NOTE(abishop): Cinder's volumes.create() has 'project_id' and
+        # 'user_id' args, but they're not wired up to anything. The only way
+        # to specify an alternate project or user for the volume is to use
+        # the identity overrides (e.g. "--os-project-id").
+        #
+        # Now, if the project or user arg is specified then the command is
+        # rejected. Otherwise, Cinder would actually create a volume, but
+        # without the specified property.
         if parsed_args.project:
-            project = utils.find_resource(
-                identity_client.projects,
-                parsed_args.project).id
-
-        user = None
+            raise exceptions.CommandError(
+                _("ERROR: --project is deprecated, please use"
+                  " --os-project-name or --os-project-id instead."))
         if parsed_args.user:
-            user = utils.find_resource(
-                identity_client.users,
-                parsed_args.user).id
+            raise exceptions.CommandError(
+                _("ERROR: --user is deprecated, please use"
+                  " --os-username instead."))
 
         volume = volume_client.volumes.create(
             size=size,
@@ -215,8 +219,6 @@ class CreateVolume(command.ShowOne):
             name=parsed_args.name,
             description=parsed_args.description,
             volume_type=parsed_args.type,
-            user_id=user,
-            project_id=project,
             availability_zone=parsed_args.availability_zone,
             metadata=parsed_args.property,
             imageRef=image,
diff --git a/releasenotes/notes/bug-1777153-750e6044aa28d5d8.yaml b/releasenotes/notes/bug-1777153-750e6044aa28d5d8.yaml
new file mode 100644
index 00000000..a2a9b51a
--- /dev/null
+++ b/releasenotes/notes/bug-1777153-750e6044aa28d5d8.yaml
@@ -0,0 +1,15 @@
+---
+deprecations:
+  - |
+    The ``--project`` and ``--user`` options for the ``volume create``
+    command have been deprecated. They are deprecated because Cinder's
+    volume create API ignores the corresponding API inputs.
+fixes:
+  - |
+    Fix ``volume create`` by removing two broken options. The ``--project``
+    and ``--user`` options were intended to specify an alternate project
+    and/or user for the volume, but the Volume service's API does not
+    support this behavior. This caused the volume to be created, but
+    without the expected project/user values. However, an alternate
+    project and/or user may be specified using identity overrides (e.g.
+    --os-username, --os-project-id).