Merge "Fail with more meaningful error while creating client"

doc/source/api_v1.rst

@@ -22,13 +22,18 @@ credentials to `ironicclient.client.get_client()`_. By default, the
 Bare Metal Provisioning system is configured so that only administrators
 (users with 'admin' role) have access.
 
+.. note::
+    Explicit instantiation of `ironicclient.v1.client.Client`_ may cause
+    errors since it doesn't verify provided arguments, using
+    `ironicclient.client.get_client()` is prefered way to get client object.
+
 There are two different sets of credentials that can be used::
 
    * ironic endpoint and auth token
    * Identity Service (keystone) credentials
 
 Using ironic endpoint and auth token
-.....................................
+....................................
 
 An auth token and the ironic endpoint can be used to authenticate::
 

ironicclient/tests/unit/v1/test_client.py

@@ -0,0 +1,81 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import mock
+
+from ironicclient.common import filecache
+from ironicclient.common import http
+from ironicclient import exc
+from ironicclient.tests.unit import utils
+from ironicclient.v1 import client
+
+
+@mock.patch.object(http, '_construct_http_client', autospec=True)
+class ClientTest(utils.BaseTestCase):
+
+    def test_client_user_api_version(self, http_client_mock):
+        endpoint = 'http://ironic:6385'
+        token = 'safe_token'
+        os_ironic_api_version = '1.15'
+
+        client.Client(endpoint, token=token,
+                      os_ironic_api_version=os_ironic_api_version)
+
+        http_client_mock.assert_called_once_with(
+            endpoint, token=token,
+            os_ironic_api_version=os_ironic_api_version,
+            api_version_select_state='user')
+
+    @mock.patch.object(filecache, 'retrieve_data', autospec=True)
+    def test_client_cache_api_version(self, cache_mock, http_client_mock):
+        endpoint = 'http://ironic:6385'
+        token = 'safe_token'
+        os_ironic_api_version = '1.15'
+        cache_mock.return_value = os_ironic_api_version
+
+        client.Client(endpoint, token=token)
+
+        cache_mock.assert_called_once_with(host='ironic', port='6385')
+        http_client_mock.assert_called_once_with(
+            endpoint, token=token,
+            os_ironic_api_version=os_ironic_api_version,
+            api_version_select_state='cached')
+
+    @mock.patch.object(filecache, 'retrieve_data', autospec=True)
+    def test_client_default_api_version(self, cache_mock, http_client_mock):
+        endpoint = 'http://ironic:6385'
+        token = 'safe_token'
+        cache_mock.return_value = None
+
+        client.Client(endpoint, token=token)
+
+        cache_mock.assert_called_once_with(host='ironic', port='6385')
+        http_client_mock.assert_called_once_with(
+            endpoint, token=token,
+            os_ironic_api_version=client.DEFAULT_VER,
+            api_version_select_state='default')
+
+    def test_client_cache_version_no_endpoint_as_arg(self, http_client_mock):
+        self.assertRaises(exc.EndpointException,
+                          client.Client,
+                          session='fake_session',
+                          insecure=True,
+                          endpoint_override='http://ironic:6385')
+
+    def test_client_initialized_managers(self, http_client_mock):
+        cl = client.Client('http://ironic:6385', token='safe_token',
+                           os_ironic_api_version='1.15')
+
+        self.assertIsInstance(cl.node, client.node.NodeManager)
+        self.assertIsInstance(cl.port, client.port.PortManager)
+        self.assertIsInstance(cl.driver, client.driver.DriverManager)
+        self.assertIsInstance(cl.chassis, client.chassis.ChassisManager)

ironicclient/v1/client.py

@@ -16,6 +16,8 @@
 from ironicclient.common import filecache
 from ironicclient.common import http
 from ironicclient.common.http import DEFAULT_VER
+from ironicclient.common.i18n import _
+from ironicclient import exc
 from ironicclient.v1 import chassis
 from ironicclient.v1 import driver
 from ironicclient.v1 import node
@@ -32,14 +34,19 @@ class Client(object):
                             http requests. (optional)
     """
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, endpoint=None, *args, **kwargs):
         """Initialize a new client for the Ironic v1 API."""
         if kwargs.get('os_ironic_api_version'):
             kwargs['api_version_select_state'] = "user"
         else:
+            if not endpoint:
+                raise exc.EndpointException(
+                    _("Must provide 'endpoint' if os_ironic_api_version "
+                      "isn't specified"))
+
             # If the user didn't specify a version, use a cached version if
             # one has been stored
-            host, netport = http.get_server(args[0])
+            host, netport = http.get_server(endpoint)
             saved_version = filecache.retrieve_data(host=host, port=netport)
             if saved_version:
                 kwargs['api_version_select_state'] = "cached"
@@ -48,7 +55,8 @@ class Client(object):
                 kwargs['api_version_select_state'] = "default"
                 kwargs['os_ironic_api_version'] = DEFAULT_VER
 
-        self.http_client = http._construct_http_client(*args, **kwargs)
+        self.http_client = http._construct_http_client(
+            endpoint, *args, **kwargs)
 
         self.chassis = chassis.ChassisManager(self.http_client)
         self.node = node.NodeManager(self.http_client)

releasenotes/notes/index-error-no-endpoint-eb281187f80a9aa4.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - Fail with EndpointException instead of IndexError while creating
+    v1 client without ``endpoint`` argument if os_ironic_api_version
+    isn't specified.