Add CLI error notification in case there is no share type

Currently, when there is no default share type and try to create
a manila share without specifying a share type, the creation
request is sent and no CLI error notification is received.
This patch prevent sending the create request and provide
early feedback to CLI users.

Closes-Bug: #1960422

Change-Id: I66a8bcebe35e744f9796e3db44d6cedf2ada983f

manilaclient/osc/v2/share.py

@@ -23,6 +23,7 @@ from osc_lib import utils as oscutils
 
 from manilaclient import api_versions
 from manilaclient.common._i18n import _
+from manilaclient.common.apiclient import exceptions as apiclient_exceptions
 from manilaclient.common.apiclient import utils as apiutils
 from manilaclient.common import cliutils
 from manilaclient.osc import utils
@@ -193,10 +194,17 @@ class CreateShare(command.ShowOne):
         # TODO(s0ru): the table shows 'Field', 'Value'
         share_client = self.app.client_manager.share
 
-        share_type = None
         if parsed_args.share_type:
             share_type = apiutils.find_resource(share_client.share_types,
                                                 parsed_args.share_type).id
+        else:
+            try:
+                share_type = apiutils.find_resource(
+                    share_client.share_types, 'default').id
+            except apiclient_exceptions.CommandError:
+                msg = ("There is no default share type available. You must "
+                       "pick a valid share type to create a share.")
+                raise exceptions.CommandError(msg)
 
         share_network = None
         if parsed_args.share_network:

manilaclient/tests/unit/osc/v2/test_share.py

@@ -82,6 +82,8 @@ class TestShareCreate(TestShare):
         self.share_snapshot = (
             manila_fakes.FakeShareSnapshot.create_one_snapshot())
         self.snapshots_mock.get.return_value = self.share_snapshot
+        self.share_type = manila_fakes.FakeShareType.create_one_sharetype()
+        self.share_types_mock.get.return_value = self.share_type
 
         # Get the command object to test
         self.cmd = osc_shares.CreateShare(self.app, None)
@@ -95,10 +97,12 @@ class TestShareCreate(TestShare):
         arglist = [
             self.new_share.share_proto,
             str(self.new_share.size),
+            '--share-type', self.share_type.id,
         ]
         verifylist = [
             ('share_proto', self.new_share.share_proto),
-            ('size', self.new_share.size)
+            ('size', self.new_share.size),
+            ('share_type', self.share_type.id)
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -114,7 +118,7 @@ class TestShareCreate(TestShare):
             share_group_id=None,
             share_network=None,
             share_proto=self.new_share.share_proto,
-            share_type=None,
+            share_type=self.share_type.id,
             size=self.new_share.size,
             snapshot_id=None,
             scheduler_hints={}
@@ -139,12 +143,14 @@ class TestShareCreate(TestShare):
         arglist = [
             self.new_share.share_proto,
             str(self.new_share.size),
+            '--share-type', self.share_type.id,
             '--property', 'Manila=zorilla',
             '--property', 'Zorilla=manila'
         ]
         verifylist = [
             ('share_proto', self.new_share.share_proto),
             ('size', self.new_share.size),
+            ('share_type', self.share_type.id),
             ('property', {'Manila': 'zorilla', 'Zorilla': 'manila'}),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -160,7 +166,7 @@ class TestShareCreate(TestShare):
             share_group_id=None,
             share_network=None,
             share_proto=self.new_share.share_proto,
-            share_type=None,
+            share_type=self.share_type.id,
             size=self.new_share.size,
             snapshot_id=None,
             scheduler_hints={}
@@ -181,12 +187,14 @@ class TestShareCreate(TestShare):
         arglist = [
             self.new_share.share_proto,
             str(self.new_share.size),
+            '--share-type', self.share_type.id,
             '--scheduler-hint', ('same_host=%s' % share1_name),
             '--scheduler-hint', ('different_host=%s' % share2_name),
         ]
         verifylist = [
             ('share_proto', self.new_share.share_proto),
             ('size', self.new_share.size),
+            ('share_type', self.share_type.id),
             ('scheduler_hint',
                 {'same_host': share1_name, 'different_host': share2_name}),
         ]
@@ -203,7 +211,7 @@ class TestShareCreate(TestShare):
             share_group_id=None,
             share_network=None,
             share_proto=self.new_share.share_proto,
-            share_type=None,
+            share_type=self.share_type.id,
             size=self.new_share.size,
             snapshot_id=None,
             scheduler_hints={'same_host': shares[0].id,
@@ -219,12 +227,14 @@ class TestShareCreate(TestShare):
         arglist = [
             self.new_share.share_proto,
             str(self.new_share.size),
+            '--share-type', self.share_type.id,
             '--snapshot-id', self.share_snapshot.id
 
         ]
         verifylist = [
             ('share_proto', self.new_share.share_proto),
             ('size', self.new_share.size),
+            ('share_type', self.share_type.id),
             ('snapshot_id', self.share_snapshot.id)
         ]
 
@@ -241,7 +251,7 @@ class TestShareCreate(TestShare):
             share_group_id=None,
             share_network=None,
             share_proto=self.new_share.share_proto,
-            share_type=None,
+            share_type=self.share_type.id,
             size=self.new_share.size,
             snapshot_id=self.share_snapshot.id,
             scheduler_hints={}
@@ -255,11 +265,13 @@ class TestShareCreate(TestShare):
         arglist = [
             self.new_share.share_proto,
             str(self.new_share.size),
+            '--share-type', self.share_type.id,
             '--wait'
         ]
         verifylist = [
             ('share_proto', self.new_share.share_proto),
             ('size', self.new_share.size),
+            ('share_type', self.share_type.id),
             ('wait', True)
         ]
 
@@ -276,7 +288,7 @@ class TestShareCreate(TestShare):
             share_group_id=None,
             share_network=None,
             share_proto=self.new_share.share_proto,
-            share_type=None,
+            share_type=self.share_type.id,
             size=self.new_share.size,
             snapshot_id=None,
             scheduler_hints={}
@@ -292,11 +304,13 @@ class TestShareCreate(TestShare):
         arglist = [
             self.new_share.share_proto,
             str(self.new_share.size),
+            '--share-type', self.share_type.id,
             '--wait'
         ]
         verifylist = [
             ('share_proto', self.new_share.share_proto),
             ('size', self.new_share.size),
+            ('share_type', self.share_type.id),
             ('wait', True)
         ]
 
@@ -314,7 +328,7 @@ class TestShareCreate(TestShare):
                 share_group_id=None,
                 share_network=None,
                 share_proto=self.new_share.share_proto,
-                share_type=None,
+                share_type=self.share_type.id,
                 size=self.new_share.size,
                 snapshot_id=None,
                 scheduler_hints={}
@@ -327,6 +341,24 @@ class TestShareCreate(TestShare):
             self.assertCountEqual(self.columns, columns)
             self.assertCountEqual(self.datalist, data)
 
+    def test_create_share_with_no_existing_share_type(self):
+        arglist = [
+            self.new_share.share_proto,
+            str(self.new_share.size),
+        ]
+        verifylist = [
+            ('share_proto', self.new_share.share_proto),
+            ('size', self.new_share.size),
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        self.share_types_mock.get.side_effect = osc_exceptions.CommandError()
+
+        self.assertRaises(
+            osc_exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+
 
 class TestShareDelete(TestShare):
 

manilaclient/tests/unit/v2/test_shell.py

@@ -2092,13 +2092,16 @@ class ShellTest(test_utils.TestCase):
 
     def test_create_share(self):
         # Use only required fields
-        self.run_command("create nfs 1")
+        expected = self.create_share_body.copy()
-        self.assert_called("POST", "/shares", body=self.create_share_body)
+        expected['share']['share_type'] = 'test_type'
+        self.run_command("create nfs 1 --share-type test_type")
+        self.assert_called("POST", "/shares", body=expected)
 
     def test_create_public_share(self):
         expected = self.create_share_body.copy()
         expected['share']['is_public'] = True
-        self.run_command("create --public nfs 1")
+        expected['share']['share_type'] = 'test_type'
+        self.run_command("create --public nfs 1 --share-type test_type")
         self.assert_called("POST", "/shares", body=expected)
 
     def test_create_with_share_network(self):
@@ -2106,26 +2109,35 @@ class ShellTest(test_utils.TestCase):
         sn = "fake-share-network"
         with mock.patch.object(shell_v2, "_find_share_network",
                                mock.Mock(return_value=sn)):
-            self.run_command("create nfs 1 --share-network %s" % sn)
+            self.run_command("create nfs 1 --share-type test_type "
+                             "--share-network %s" % sn)
             expected = self.create_share_body.copy()
             expected['share']['share_network_id'] = sn
+            expected['share']['share_type'] = 'test_type'
             self.assert_called("POST", "/shares", body=expected)
             shell_v2._find_share_network.assert_called_once_with(mock.ANY, sn)
 
     def test_create_with_metadata(self):
         # Except required fields added metadata
-        self.run_command("create nfs 1 --metadata key1=value1 key2=value2")
+        self.run_command("create nfs 1 --metadata key1=value1 key2=value2 "
+                         "--share-type test_type")
         expected = self.create_share_body.copy()
         expected['share']['metadata'] = {"key1": "value1", "key2": "value2"}
+        expected['share']['share_type'] = 'test_type'
         self.assert_called("POST", "/shares", body=expected)
 
     def test_create_with_wait(self):
-        self.run_command("create nfs 1 --wait")
+        self.run_command("create nfs 1 --wait --share-type test_type")
         expected = self.create_share_body.copy()
+        expected['share']['share_type'] = 'test_type'
         self.assert_called_anytime(
             "POST", "/shares", body=expected, clear_callstack=False)
         self.assert_called("GET", "/shares/1234")
 
+    def test_create_share_with_no_existing_share_type(self):
+        self.assertRaises(
+            exceptions.CommandError, self.run_command, "create nfs 1")
+
     def test_allow_access_cert(self):
         self.run_command("access-allow 1234 cert client.example.com")
 

manilaclient/v2/shell.py

@@ -972,6 +972,14 @@ def do_create(cs, args):
         raise exceptions.CommandError(
             "Share name cannot be with the value 'None'")
 
+    if not args.share_type:
+        try:
+            _find_share_type(cs, "default")
+        except exceptions.CommandError:
+            msg = ("There is no default share type available. You must pick "
+                   "a valid share type to create a share.")
+            raise exceptions.CommandError(msg)
+
     scheduler_hints = {}
     if args.scheduler_hints:
         scheduler_hints = _extract_key_value_options(args, 'scheduler_hints')

releasenotes/notes/bug-1960422-fix-no-default-share-type-d6191ea0aa1e57fa.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    `Launchpad bug 1960422 <https://bugs.launchpad.net/python-manilaclient/+bug/1960422>`_
+    has been fixed by prevent sending the share creation request and provide
+    early feedback to CLI users.