Keep is_public usage backwards compatible3.6.1

5a1513244caf7acbd41e181419bc8b62bf4bcaba added the ability to
filter by AZ, but it changed the existing behavior if is_public
was not specified. This adds handling to make sure we are still
consistent with the previous behavior.

Co-Authored-by: Alan Bishop <abishop@redhat.com>
Change-Id: I5000aab092c1b434c8dc17bbe4b2d3d632f528c3
Closes-bug: #1778055

3 files changed, 36 insertions, 12 deletions

diff --git a/cinderclient/tests/unit/fakes.py b/cinderclient/tests/unit/fakes.py
index 2b7d190..61d1904 100644
--- a/cinderclient/tests/unit/fakes.py
+++ b/cinderclient/tests/unit/fakes.py
@@ -51,17 +51,23 @@ class FakeClient(object):
             result = False
         return result
 
+    def assert_in_call(self, url_part):
+        """Assert a call contained a part in its URL."""
+        assert self.client.callstack, "Expected call but no calls were made"
+
+        called = self.client.callstack[-1][1]
+        assert url_part in called, 'Expected %s in call but found %s' % (
+            url_part, called)
+
     def assert_called(self, method, url, body=None,
                       partial_body=None, pos=-1, **kwargs):
-        """
-        Assert than an API method was just called.
-        """
+        """Assert than an API method was just called."""
         expected = (method, url)
-        called = self.client.callstack[pos][0:2]
-
         assert self.client.callstack, ("Expected %s %s but no calls "
                                        "were made." % expected)
 
+        called = self.client.callstack[pos][0:2]
+
         assert expected == called, 'Expected %s %s; got %s %s' % (
             expected + called)
 
diff --git a/cinderclient/tests/unit/v3/test_shell.py b/cinderclient/tests/unit/v3/test_shell.py
index 761994c..43fb097 100644
--- a/cinderclient/tests/unit/v3/test_shell.py
+++ b/cinderclient/tests/unit/v3/test_shell.py
@@ -96,6 +96,9 @@ class ShellTest(utils.TestCase):
         return self.shell.cs.assert_called(method, url, body,
                                            partial_body, **kwargs)
 
+    def assert_call_contained(self, url_part):
+        self.shell.cs.assert_in_call(url_part)
+
     @ddt.data({'resource': None, 'query_url': None},
               {'resource': 'volume', 'query_url': '?resource=volume'},
               {'resource': 'group', 'query_url': '?resource=group'})
@@ -253,10 +256,16 @@ class ShellTest(utils.TestCase):
     def test_type_list_with_filters(self):
         self.run_command('--os-volume-api-version 3.52 type-list '
                          '--filters extra_specs={key:value}')
-        self.assert_called(
-            'GET', '/types?%s' % parse.urlencode(
+        self.assert_called('GET', mock.ANY)
+        self.assert_call_contained(
+            parse.urlencode(
                 {'extra_specs':
                     {six.text_type('key'): six.text_type('value')}}))
+        self.assert_call_contained(parse.urlencode({'is_public': None}))
+
+    def test_type_list_no_filters(self):
+        self.run_command('--os-volume-api-version 3.52 type-list')
+        self.assert_called('GET', '/types?is_public=None')
 
     @ddt.data("3.10", "3.11")
     def test_list_with_group_id_after_3_10(self, version):
diff --git a/cinderclient/v3/volume_types.py b/cinderclient/v3/volume_types.py
index 5030b5e..4d24756 100644
--- a/cinderclient/v3/volume_types.py
+++ b/cinderclient/v3/volume_types.py
@@ -85,14 +85,23 @@ class VolumeTypeManager(base.ManagerWithFind):
     def list(self, search_opts=None, is_public=None):
         """Lists all volume types.
 
-        :rtype: list of :class:`VolumeType`.
+        :param search_opts: Optional search filters.
+        :param is_public: Whether to only get public types.
+        :return: List of :class:`VolumeType`.
         """
         if not search_opts:
             search_opts = dict()
-        if is_public:
-            search_opts.update({"is_public": is_public})
-        query_string = "?%s" % parse.urlencode(
-            search_opts) if search_opts else ''
+
+        # Remove 'all_tenants' option added by ManagerWithFind.findall(),
+        # as it is not a valid search option for volume_types.
+        search_opts.pop('all_tenants', None)
+
+        # Need to keep backwards compatibility with is_public usage. If it
+        # isn't included then cinder will assume you want is_public=True, which
+        # negatively affects the results.
+        search_opts['is_public'] = is_public
+
+        query_string = "?%s" % parse.urlencode(search_opts)
         return self._list("/types%s" % query_string, "volume_types")
 
     def get(self, volume_type):