quota: Allow 'quota set' to function without volume service

Unlike cinderclient, SDK attempts to connect to a service as soon as you
create a client. A keystoneauth1.exceptions.catalog.EndpointNotFound
exception can be raised if this service does not exist in the service
catalog. Avoid this for the quota and limits commands by first checking
if the service is enabled.

In the process, we rework the 'is_volume_endpoint_enabled' helper we are
using to check for the existence of the service to *not* require a
volume client, since this was causing a chicken and egg issue for us
(and was also pretty much unnecessary).

Change-Id: I56e68f00ea221d689eb7f668e9e5ffa7d1a20184
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Closes-bug: #2076229

openstackclient/common/clientmanager.py

@@ -116,7 +116,6 @@ class ClientManager(clientmanager.ClientManager):
 
     def is_network_endpoint_enabled(self):
         """Check if the network endpoint is enabled"""
-
         # NOTE(dtroyer): is_service_available() can also return None if
         #                there is no Service Catalog, callers here are
         #                not expecting that so fold None into True to
@@ -125,34 +124,16 @@ class ClientManager(clientmanager.ClientManager):
 
     def is_compute_endpoint_enabled(self):
         """Check if Compute endpoint is enabled"""
-
         return self.is_service_available('compute') is not False
 
-    def is_volume_endpoint_enabled(self, volume_client):
+    # TODO(stephenfin): Drop volume_client argument in OSC 8.0 or later.
+    def is_volume_endpoint_enabled(self, volume_client=None):
         """Check if volume endpoint is enabled"""
-        # NOTE(jcross): Cinder did some interesting things with their service
+        return (
-        #               name so we need to figure out which version to look
+            self.is_service_available('volume') is not False
-        #               for when calling is_service_available()
+            or self.is_service_available('volumev3') is not False
-        endpoint_data = volume_client.get_endpoint_data()
+            or self.is_service_available('volumev2') is not False
-        # Not sure how endpoint data stores the api version for v2 API,
+        )
-        # for v3 it is a tuple (3, 0)
-        if endpoint_data.api_version and isinstance(
-            endpoint_data.api_version, tuple
-        ):
-            volume_version = endpoint_data.api_version[0]
-        else:
-            # Setting volume_version as 2 here if it doesn't satisfy the
-            # conditions for version 3
-            volume_version = 2
-        if (
-            self.is_service_available("volumev%s" % volume_version)
-            is not False
-        ):
-            return True
-        elif self.is_service_available('volume') is not False:
-            return True
-        else:
-            return False
 
 
 # Plugin Support

openstackclient/common/limits.py

@@ -101,9 +101,6 @@ class ShowLimits(command.Lister):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.sdk_connection.compute
-        volume_client = self.app.client_manager.sdk_connection.volume
-
         project_id = None
         if parsed_args.project is not None:
             identity_client = self.app.client_manager.identity
@@ -125,11 +122,13 @@ class ShowLimits(command.Lister):
         volume_limits = None
 
         if self.app.client_manager.is_compute_endpoint_enabled():
+            compute_client = self.app.client_manager.sdk_connection.compute
             compute_limits = compute_client.get_limits(
                 reserved=parsed_args.is_reserved, tenant_id=project_id
             )
 
-        if self.app.client_manager.is_volume_endpoint_enabled(volume_client):
+        if self.app.client_manager.is_volume_endpoint_enabled():
+            volume_client = self.app.client_manager.sdk_connection.volume
             volume_limits = volume_client.get_limits(
                 project_id=project_id,
             )

openstackclient/common/quota.py

@@ -565,10 +565,13 @@ class SetQuota(common.NetDetectionMixin, command.Command):
             msg = _('--force cannot be used with --class or --default')
             raise exceptions.CommandError(msg)
 
-        compute_client = self.app.client_manager.sdk_connection.compute
-        volume_client = self.app.client_manager.sdk_connection.volume
-
         compute_kwargs = {}
+        volume_kwargs = {}
+        network_kwargs = {}
+
+        if self.app.client_manager.is_compute_endpoint_enabled():
+            compute_client = self.app.client_manager.sdk_connection.compute
+
             for k, v in COMPUTE_QUOTAS.items():
                 value = getattr(parsed_args, k, None)
                 if value is not None:
@@ -577,21 +580,27 @@ class SetQuota(common.NetDetectionMixin, command.Command):
             if compute_kwargs and parsed_args.force is True:
                 compute_kwargs['force'] = parsed_args.force
 
-        volume_kwargs = {}
+        if self.app.client_manager.is_volume_endpoint_enabled():
+            volume_client = self.app.client_manager.sdk_connection.volume
+
             for k, v in VOLUME_QUOTAS.items():
                 value = getattr(parsed_args, k, None)
                 if value is not None:
-                if parsed_args.volume_type and k in IMPACT_VOLUME_TYPE_QUOTAS:
+                    if (
+                        parsed_args.volume_type
+                        and k in IMPACT_VOLUME_TYPE_QUOTAS
+                    ):
                         k = k + '_%s' % parsed_args.volume_type
                     volume_kwargs[k] = value
 
-        network_kwargs = {}
         if self.app.client_manager.is_network_endpoint_enabled():
+            network_client = self.app.client_manager.network
+
             for k, v in NETWORK_QUOTAS.items():
                 value = getattr(parsed_args, k, None)
                 if value is not None:
                     network_kwargs[k] = value
-        else:
+        elif self.app.client_manager.is_compute_endpoint_enabled():
             for k, v in NOVA_NETWORK_QUOTAS.items():
                 value = getattr(parsed_args, k, None)
                 if value is not None:
@@ -632,11 +641,7 @@ class SetQuota(common.NetDetectionMixin, command.Command):
             compute_client.update_quota_set(project, **compute_kwargs)
         if volume_kwargs:
             volume_client.update_quota_set(project, **volume_kwargs)
-        if (
+        if network_kwargs:
-            network_kwargs
-            and self.app.client_manager.is_network_endpoint_enabled()
-        ):
-            network_client = self.app.client_manager.network
             network_client.update_quota(project, **network_kwargs)
 
 

openstackclient/tests/unit/fakes.py

@@ -154,7 +154,7 @@ class FakeClientManager:
     def is_compute_endpoint_enabled(self):
         return self.compute_endpoint_enabled
 
-    def is_volume_endpoint_enabled(self, client):
+    def is_volume_endpoint_enabled(self, client=None):
         return self.volume_endpoint_enabled