Fix is_default value is empty issue when create

When we use manila client to create a share type or share group
type, the is_default value is always empty, this patch is to fix
this issue.

Depends-On: Ia8bcb632591fc0b5438847b22ec59c2039ecf5f1
Change-Id: Ib264d8422fe57701f990cbbf7ea5d1e90b8b0008
Closes-bug: #1743941

manilaclient/api_versions.py

@@ -27,7 +27,7 @@ from manilaclient import utils
 
 LOG = logging.getLogger(__name__)
 
-MAX_VERSION = '2.45'
+MAX_VERSION = '2.46'
 MIN_VERSION = '2.0'
 DEPRECATED_VERSION = '1.0'
 _VERSIONED_METHOD_MAP = {}

manilaclient/tests/unit/v2/fakes.py

@@ -16,6 +16,7 @@
 from __future__ import print_function
 
 import manilaclient
+from manilaclient import api_versions
 from manilaclient.tests.unit.v2 import fake_clients as fakes
 from manilaclient.v2 import client
 
@@ -878,7 +879,10 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
                            'required_extra_specs': {'test': 'test'}}})
 
     def get_types(self, **kw):
-        return (200, {}, {
+        req_version = self.default_headers['X-Openstack-Manila-Api-Version']
+        if not isinstance(req_version, api_versions.APIVersion):
+            req_version = api_versions.APIVersion(req_version)
+        response_body = {
             'share_types': [{'id': 1,
                              'name': 'test-type-1',
                              'extra_specs': {'test1': 'test1'},
@@ -886,7 +890,14 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
                             {'id': 2,
                              'name': 'test-type-2',
                              'extra_specs': {'test1': 'test1'},
-                             'required_extra_specs': {'test': 'test'}}]})
+                             'required_extra_specs': {'test': 'test'}}]
+        }
+
+        if req_version >= api_versions.APIVersion('2.46'):
+            response_body['share_types'][0]['is_default'] = False
+            response_body['share_types'][1]['is_default'] = False
+
+        return 200, {}, response_body
 
     def get_types_1(self, **kw):
         return (200, {}, {'share_type': {
@@ -932,6 +943,7 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
             'share_type': {
                 'id': 3,
                 'name': 'test-type-3',
+                'is_default': False,
                 'description': 'test description',
                 'extra_specs': share_type['extra_specs'],
                 'required_extra_specs': required_extra_specs,
@@ -1075,6 +1087,12 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
                 },
             ],
         }
+
+        req_version = self.default_headers['X-Openstack-Manila-Api-Version']
+        if req_version >= api_versions.APIVersion('2.46'):
+            share_group_types['share_group_types'][0]['is_default'] = False
+            share_group_types['share_group_types'][1]['is_default'] = False
+
         return 200, {}, share_group_types
 
     def get_share_group_types_1(self, **kw):

manilaclient/tests/unit/v2/test_shell.py

@@ -509,15 +509,31 @@ class ShellTest(test_utils.TestCase):
         self.assert_called_anytime('GET', '/types/1234')
 
     @mock.patch.object(cliutils, 'print_list', mock.Mock())
-    def test_type_list(self):
+    @ddt.data(*itertools.product(
-        self.run_command('type-list')
+        ('type-list --columns id,is_default', 'type-list --columns id,name',
+         'type-list --columns is_default', 'type-list'),
+        {'2.45', '2.46', api_versions.MAX_VERSION}))
+    @ddt.unpack
+    def test_type_list(self, command, version):
+        self.run_command(command, version=version)
 
-        self.assert_called('GET', '/types')
+        columns_requested = ['ID', 'Name', 'visibility',
-        cliutils.print_list.assert_called_once_with(
+                             'is_default', 'required_extra_specs',
-            mock.ANY,
+                             'optional_extra_specs', 'Description']
-            ['ID', 'Name', 'visibility', 'is_default', 'required_extra_specs',
+        if 'columns' in command:
-             'optional_extra_specs', 'Description'],
+            columns_requested = command.split('--columns ')[1].split(',')
-            mock.ANY)
+
+        is_default_in_api = (api_versions.APIVersion(version) >=
+                             api_versions.APIVersion('2.46'))
+
+        if not is_default_in_api and 'is_default' in columns_requested:
+            self.assert_called('GET', '/types/default')
+            self.assert_called_anytime('GET', '/types')
+        else:
+            self.assert_called('GET', '/types')
+
+        cliutils.print_list.assert_called_with(
+            mock.ANY, columns_requested, mock.ANY)
 
     @mock.patch.object(cliutils, 'print_list', mock.Mock())
     def test_type_list_select_column(self):
@@ -529,10 +545,6 @@ class ShellTest(test_utils.TestCase):
             ['id', 'name'],
             mock.ANY)
 
-    def test_type_list_default_volume_type(self):
-        self.run_command('type-list')
-        self.assert_called_anytime('GET', '/types/default')
-
     def test_type_list_all(self):
         self.run_command('type-list --all')
         self.assert_called_anytime('GET', '/types?is_public=all')
@@ -2436,14 +2448,30 @@ class ShellTest(test_utils.TestCase):
             exceptions.CommandError,
             self.run_command, 'share-group-snapshot-delete fake-sg-snapshot')
 
-    def test_share_group_type_list(self):
+    @ddt.data(*itertools.product(
+        ('--columns id,is_default', '--columns id,name',
+         '--columns is_default', ''),
+        {'2.45', '2.46', api_versions.MAX_VERSION}))
+    @ddt.unpack
+    def test_share_group_type_list(self, command_args, version):
         self.mock_object(shell_v2, '_print_share_group_type_list')
+        command = 'share-group-type-list ' + command_args
+        columns_requested = command_args.split('--columns ')[-1] or None
+        is_default_in_api = (api_versions.APIVersion(version) >=
+                             api_versions.APIVersion('2.46'))
 
-        self.run_command('share-group-type-list')
+        self.run_command(command, version=version)
+
+        if (not is_default_in_api and
+                (not columns_requested or 'is_default' in columns_requested)):
+            self.assert_called('GET', '/share-group-types/default')
+            self.assert_called_anytime('GET', '/share-group-types')
+        else:
+            self.assert_called('GET', '/share-group-types')
 
-        self.assert_called('GET', '/share-group-types')
         shell_v2._print_share_group_type_list.assert_called_once_with(
-            mock.ANY, default_share_group_type=mock.ANY, columns=mock.ANY)
+            mock.ANY, default_share_group_type=mock.ANY,
+            columns=columns_requested)
 
     def test_share_group_type_list_select_column(self):
         self.mock_object(shell_v2, '_print_share_group_type_list')
@@ -2454,11 +2482,6 @@ class ShellTest(test_utils.TestCase):
         shell_v2._print_share_group_type_list.assert_called_once_with(
             mock.ANY, default_share_group_type=mock.ANY, columns='id,name')
 
-    def test_share_group_type_list_default_share_type(self):
-        self.run_command('share-group-type-list')
-
-        self.assert_called_anytime('GET', '/share-group-types/default')
-
     def test_share_group_type_list_all(self):
         self.run_command('share-group-type-list --all')
 

manilaclient/v2/shell.py

@@ -153,7 +153,13 @@ def _find_share_instance(cs, instance):
 
 def _print_type_show(stype, default_share_type=None):
 
-    is_default = 'YES' if stype == default_share_type else 'NO'
+    if hasattr(stype, 'is_default'):
+        is_default = 'YES' if stype.is_default else 'NO'
+    elif default_share_type:
+        is_default = 'YES' if stype.id == default_share_type.id else 'NO'
+    else:
+        is_default = 'NO'
+
     stype_dict = {
         'id': stype.id,
         'name': stype.name,
@@ -3680,8 +3686,11 @@ def _print_share_type_list(stypes, default_share_type=None, columns=None,
                            description=False):
 
     def _is_default(share_type):
-        if share_type == default_share_type:
+        if hasattr(share_type, 'is_default'):
-            return 'YES'
+            return 'YES' if share_type.is_default else '-'
+        elif default_share_type:
+            default = default_share_type.id
+            return 'YES' if share_type.id == default else '-'
         else:
             return '-'
 
@@ -3715,10 +3724,9 @@ def _print_share_type_list(stypes, default_share_type=None, columns=None,
 def _print_share_type(stype, default_share_type=None, show_des=False):
 
     def _is_default(share_type):
-        if share_type == default_share_type:
+        if hasattr(share_type, 'is_default'):
-            return 'YES'
+            return 'YES' if share_type.is_default else '-'
-        else:
+        return '-'
-            return '-'
 
     stype_dict = {
         'ID': stype.id,
@@ -3790,13 +3798,14 @@ def do_type_list(cs, args):
             'extra_specs': extra_specs
         }
 
-    try:
-        default = cs.share_types.get()
-    except exceptions.NotFound:
-        default = None
-
     share_types = cs.share_types.list(show_all=show_all,
                                       search_opts=search_opts)
+    default = None
+    if share_types and not hasattr(share_types[0], 'is_default'):
+        if ((args.columns and 'is_default' in args.columns) or
+                args.columns is None):
+            default = cs.share_types.get()
+
     show_des = cs.api_version.matches(
         api_versions.APIVersion("2.41"), api_versions.APIVersion())
     _print_share_type_list(share_types, default_share_type=default,
@@ -3810,10 +3819,10 @@ def do_type_list(cs, args):
 def do_type_show(cs, args):
     """Show share type details."""
     share_type = cs.share_types.show(args.share_type)
-    try:
+
+    default = None
+    if (share_type and not hasattr(share_type, 'is_default')):
         default = cs.share_types.get()
-    except exceptions.NotFound:
-        default = None
     _print_type_show(share_type, default_share_type=default)
 
 
@@ -4064,9 +4073,12 @@ def do_type_access_remove(cs, args):
 def _print_share_group_type_list(share_group_types,
                                  default_share_group_type=None, columns=None):
 
-    def _is_default(share_group_types):
+    def _is_default(share_group_type):
-        if share_group_types == default_share_group_type:
+        if hasattr(share_group_type, 'is_default'):
-            return 'YES'
+            return 'YES' if share_group_type.is_default else '-'
+        elif default_share_group_type:
+            default = default_share_group_type.id
+            return 'YES' if share_group_type.id == default else '-'
         else:
             return '-'
 
@@ -4094,16 +4106,15 @@ def _print_share_group_type_list(share_group_types,
 def _print_share_group_type(share_group_type, default_share_type=None):
 
     def _is_default(share_group_type):
-        if share_group_type == default_share_type:
+        if hasattr(share_group_type, 'is_default'):
-            return 'YES'
+            return 'YES' if share_group_type.is_default else '-'
-        else:
+        return '-'
-            return '-'
 
     share_group_type_dict = {
         'ID': share_group_type.id,
         'Name': share_group_type.name,
         'Visibility': _is_share_type_public(share_group_type),
-        'is_default': _is_default(share_group_type),
+        'is_default': _is_default
     }
     cliutils.print_dict(share_group_type_dict)
 
@@ -4131,12 +4142,14 @@ def _find_share_group_type(cs, sg_type):
 def do_share_group_type_list(cs, args):
     """Print a list of available 'share group types'."""
 
-    try:
-        default = cs.share_group_types.get()
-    except exceptions.NotFound:
-        default = None
-
     sg_types = cs.share_group_types.list(show_all=args.all)
+
+    default = None
+    if sg_types and not hasattr(sg_types[0], 'is_default'):
+        if ((args.columns and 'is_default' in args.columns) or
+                args.columns is None):
+            default = cs.share_group_types.get()
+
     _print_share_group_type_list(
         sg_types, default_share_group_type=default, columns=args.columns)
 

releasenotes/notes/fix-is-default-empty-557844001e0401e2.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - The share type and share group type shell commands retrieve
+    the 'is_default' value from the manila API where supported.
+    This fix also addresses the blank 'is_default' field when
+    creating share types and share group types.