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
(cherry picked from commit ded2303da8)
(cherry picked from commit 05982cef88)
This commit is contained in:
lkuchlan 2022-02-09 17:16:58 +02:00 committed by Goutham Pacha Ravi
parent d6e301f9dc
commit ea39ef8fc1
5 changed files with 77 additions and 13 deletions

View File

@ -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
@ -185,10 +186,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:

View File

@ -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
)
@ -138,12 +142,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)
@ -159,7 +165,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
)
@ -173,12 +179,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)
]
@ -195,7 +203,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
)
@ -208,11 +216,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)
]
@ -229,7 +239,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
)
@ -244,11 +254,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)
]
@ -266,7 +278,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
)
@ -278,6 +290,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):

View File

@ -2073,13 +2073,16 @@ class ShellTest(test_utils.TestCase):
def test_create_share(self):
# Use only required fields
self.run_command("create nfs 1")
self.assert_called("POST", "/shares", body=self.create_share_body)
expected = self.create_share_body.copy()
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):
@ -2087,26 +2090,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")

View File

@ -891,6 +891,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)
share = cs.shares.create(args.share_protocol, args.size, args.snapshot_id,
args.name, args.description,
metadata=share_metadata,

View File

@ -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.