Use openstacksdk for accessing Swift

This change replaces the swiftclient dependency with openstacksdk.
The ultimate goal is to use only openstacksdk for all cross-service
interactions.

The configuration option max_retires has been broken for several
releases since commit 7e8cdc0d0f,
this change deprecates it.

Change-Id: I635519f88e90f4267ff7a9cb063a697ff39e4710

ironic_inspector/common/swift.py

@@ -15,9 +15,9 @@
 
 import json
 
+import openstack
+from openstack import exceptions as os_exc
 from oslo_config import cfg
-from swiftclient import client as swift_client
-from swiftclient import exceptions as swift_exceptions
 
 from ironic_inspector.common.i18n import _
 from ironic_inspector.common import keystone
@@ -53,19 +53,9 @@ class SwiftAPI(object):
             if not SWIFT_SESSION:
                 SWIFT_SESSION = keystone.get_session('swift')
 
-            adapter = keystone.get_adapter('swift', session=SWIFT_SESSION)
+            self.connection = openstack.connection.Connection(
-        except Exception as exc:
+                session=SWIFT_SESSION,
-            raise utils.Error(_("Could not create an adapter to connect to "
+                oslo_conf=CONF).object_store
-                                "the object storage service: %s") % exc)
-
-        # TODO(pas-ha) reverse-construct SSL-related session options here
-        params = {
-            'os_options': {
-                'object_storage_url': adapter.get_endpoint()}}
-
-        try:
-            self.connection = swift_client.Connection(session=SWIFT_SESSION,
-                                                      **params)
         except Exception as exc:
             raise utils.Error(_("Could not connect to the object storage "
                                 "service: %s") % exc)
@@ -82,8 +72,8 @@ class SwiftAPI(object):
         :raises: utils.Error, if any operation with Swift fails.
         """
         try:
-            self.connection.put_container(container)
+            self.connection.create_container(container)
-        except swift_exceptions.ClientException as e:
+        except os_exc.SDKException as e:
             err_msg = (_('Swift failed to create container %(container)s. '
                          'Error was: %(error)s') %
                        {'container': container, 'error': e})
@@ -94,11 +84,9 @@ class SwiftAPI(object):
             headers['X-Delete-After'] = CONF.swift.delete_after
 
         try:
-            obj_uuid = self.connection.put_object(container,
+            obj_uuid = self.connection.create_object(
-                                                  object,
+                container, object, data=data, headers=headers)
-                                                  data,
+        except os_exc.SDKException as e:
-                                                  headers=headers)
-        except swift_exceptions.ClientException as e:
             err_msg = (_('Swift failed to create object %(object)s in '
                          'container %(container)s. Error was: %(error)s') %
                        {'object': object, 'container': container, 'error': e})
@@ -115,8 +103,8 @@ class SwiftAPI(object):
         :raises: utils.Error, if the Swift operation fails.
         """
         try:
-            headers, obj = self.connection.get_object(container, object)
+            obj = self.connection.download_object(object, container=container)
-        except swift_exceptions.ClientException as e:
+        except os_exc.SDKException as e:
             err_msg = (_('Swift failed to get object %(object)s in '
                          'container %(container)s. Error was: %(error)s') %
                        {'object': object, 'container': container, 'error': e})

ironic_inspector/conf/opts.py

@@ -31,7 +31,6 @@ def set_config_defaults():
                                          'requests=WARNING',
                                          'urllib3.connectionpool=WARNING',
                                          'keystonemiddleware=WARNING',
-                                         'swiftclient=WARNING',
                                          'keystoneauth=WARNING',
                                          'ironicclient=WARNING'])
     set_cors_middleware_defaults()

ironic_inspector/conf/swift.py

@@ -23,9 +23,8 @@ SERVICE_TYPE = 'object-store'
 
 _OPTS = [
     cfg.IntOpt('max_retries',
-               default=2,
+               help=_('This option is deprecated and has no effect.'),
-               help=_('Maximum number of times to retry a Swift request, '
+               deprecated_for_removal=True),
-                      'before failing.')),
     cfg.IntOpt('delete_after',
                default=0,
                help=_('Number of seconds that the Swift object will last '

ironic_inspector/test/unit/test_swift.py

@@ -15,10 +15,9 @@
 # Mostly copied from ironic/tests/test_swift.py
 
 from keystoneauth1 import exceptions as ks_exc
-from keystoneauth1 import loading as kloading
 import mock
-from swiftclient import client as swift_client
+import openstack
-from swiftclient import exceptions as swift_exception
+from openstack import exceptions as os_exc
 
 from ironic_inspector.common import keystone
 from ironic_inspector.common import swift
@@ -26,128 +25,99 @@ from ironic_inspector.test import base as test_base
 from ironic_inspector import utils
 
 
-class BaseTest(test_base.NodeTest):
+@mock.patch.object(keystone, 'get_session', autospec=True)
-    def setUp(self):
+@mock.patch.object(openstack.connection, 'Connection', autospec=True)
-        super(BaseTest, self).setUp()
+class SwiftTestCase(test_base.NodeTest):
-        self.all_macs = self.macs + ['DE:AD:BE:EF:DE:AD']
-        self.pxe_mac = self.macs[1]
-        self.data = {
-            'ipmi_address': self.bmc_address,
-            'cpus': 2,
-            'cpu_arch': 'x86_64',
-            'memory_mb': 1024,
-            'local_gb': 20,
-            'interfaces': {
-                'em1': {'mac': self.macs[0], 'ip': '1.2.0.1'},
-                'em2': {'mac': self.macs[1], 'ip': '1.2.0.2'},
-                'em3': {'mac': self.all_macs[2]},
-            },
-            'boot_interface': '01-' + self.pxe_mac.replace(':', '-'),
-        }
-
-
-@mock.patch.object(keystone, 'get_adapter', autospec=True)
-@mock.patch.object(keystone, 'register_auth_opts')
-@mock.patch.object(keystone, 'get_session')
-@mock.patch.object(swift_client, 'Connection', autospec=True)
-class SwiftTestCase(BaseTest):
 
     def setUp(self):
         super(SwiftTestCase, self).setUp()
         swift.reset_swift_session()
-        self.swift_exception = swift_exception.ClientException('', '')
-        self.cfg.config(group='swift', max_retries=2)
-        # NOTE(aarefiev) register keystoneauth dynamic options
-        adapter_opts = kloading.get_adapter_conf_options(
-            include_deprecated=False)
-        self.cfg.register_opts(adapter_opts, 'swift')
         self.addCleanup(swift.reset_swift_session)
 
-    def test___init__(self, connection_mock, load_mock,
+    def test___init__(self, connection_mock, load_mock):
-                      opts_mock, adapter_mock):
-        fake_endpoint = "http://localhost:6000"
-        adapter_mock.return_value.get_endpoint.return_value = fake_endpoint
         swift.SwiftAPI()
         connection_mock.assert_called_once_with(
             session=load_mock.return_value,
-            os_options={'object_storage_url': fake_endpoint})
+            oslo_conf=swift.CONF)
 
-    def test___init__keystone_failure(self, connection_mock, load_mock,
+    def test___init__keystone_failure(self, connection_mock, load_mock):
-                                      opts_mock, adapter_mock):
+        load_mock.side_effect = ks_exc.MissingRequiredOptions([])
-        adapter_mock.side_effect = ks_exc.MissingRequiredOptions([])
+        self.assertRaisesRegex(utils.Error, 'Could not connect',
-        self.assertRaisesRegex(utils.Error, 'Could not create an adapter',
                                swift.SwiftAPI)
         self.assertFalse(connection_mock.called)
 
-    def test___init__swift_failure(self, connection_mock, load_mock,
+    def test___init__sdk_failure(self, connection_mock, load_mock):
-                                   opts_mock, adapter_mock):
-        fake_endpoint = "http://localhost:6000"
-        adapter_mock.return_value.get_endpoint.return_value = fake_endpoint
         connection_mock.side_effect = RuntimeError()
         self.assertRaisesRegex(utils.Error, 'Could not connect',
                                swift.SwiftAPI)
         connection_mock.assert_called_once_with(
             session=load_mock.return_value,
-            os_options={'object_storage_url': fake_endpoint})
+            oslo_conf=swift.CONF)
 
-    def test_create_object(self, connection_mock, load_mock,
+    def test_create_object(self, connection_mock, load_mock):
-                           opts_mock, adapter_mock):
         swiftapi = swift.SwiftAPI()
-        connection_obj_mock = connection_mock.return_value
+        swift_mock = connection_mock.return_value.object_store
-
+        swift_mock.create_object.return_value = 'object-uuid'
-        connection_obj_mock.put_object.return_value = 'object-uuid'
 
         object_uuid = swiftapi.create_object('object', 'some-string-data')
 
-        connection_obj_mock.put_container.assert_called_once_with('ironic-'
+        swift_mock.create_container.assert_called_once_with('ironic-inspector')
-                                                                  'inspector')
+        swift_mock.create_object.assert_called_once_with(
-        connection_obj_mock.put_object.assert_called_once_with(
+            'ironic-inspector', 'object',
-            'ironic-inspector', 'object', 'some-string-data', headers=None)
+            data='some-string-data', headers=None)
+        self.assertEqual('object-uuid', object_uuid)
+
+    def test_create_object_with_delete_after(self, connection_mock, load_mock):
+        swift.CONF.set_override('delete_after', 60, group='swift')
+
+        swiftapi = swift.SwiftAPI()
+        swift_mock = connection_mock.return_value.object_store
+        swift_mock.create_object.return_value = 'object-uuid'
+
+        object_uuid = swiftapi.create_object('object', 'some-string-data')
+
+        swift_mock.create_container.assert_called_once_with('ironic-inspector')
+        swift_mock.create_object.assert_called_once_with(
+            'ironic-inspector', 'object',
+            data='some-string-data', headers={'X-Delete-After': 60})
         self.assertEqual('object-uuid', object_uuid)
 
     def test_create_object_create_container_fails(
-            self, connection_mock, load_mock, opts_mock, adapter_mock):
+            self, connection_mock, load_mock):
         swiftapi = swift.SwiftAPI()
-        connection_obj_mock = connection_mock.return_value
+        swift_mock = connection_mock.return_value.object_store
-        connection_obj_mock.put_container.side_effect = self.swift_exception
+        swift_mock.create_container.side_effect = os_exc.SDKException
         self.assertRaises(utils.Error, swiftapi.create_object, 'object',
                           'some-string-data')
-        connection_obj_mock.put_container.assert_called_once_with('ironic-'
+        swift_mock.create_container.assert_called_once_with('ironic-inspector')
-                                                                  'inspector')
+        self.assertFalse(swift_mock.create_object.called)
-        self.assertFalse(connection_obj_mock.put_object.called)
 
-    def test_create_object_put_object_fails(self, connection_mock, load_mock,
+    def test_create_object_put_object_fails(self, connection_mock, load_mock):
-                                            opts_mock, adapter_mock):
         swiftapi = swift.SwiftAPI()
-        connection_obj_mock = connection_mock.return_value
+        swift_mock = connection_mock.return_value.object_store
-        connection_obj_mock.put_object.side_effect = self.swift_exception
+        swift_mock.create_object.side_effect = os_exc.SDKException
         self.assertRaises(utils.Error, swiftapi.create_object, 'object',
                           'some-string-data')
-        connection_obj_mock.put_container.assert_called_once_with('ironic-'
+        swift_mock.create_container.assert_called_once_with('ironic-inspector')
-                                                                  'inspector')
+        swift_mock.create_object.assert_called_once_with(
-        connection_obj_mock.put_object.assert_called_once_with(
+            'ironic-inspector', 'object',
-            'ironic-inspector', 'object', 'some-string-data', headers=None)
+            data='some-string-data', headers=None)
 
-    def test_get_object(self, connection_mock, load_mock,
+    def test_get_object(self, connection_mock, load_mock):
-                        opts_mock, adapter_mock):
         swiftapi = swift.SwiftAPI()
-        connection_obj_mock = connection_mock.return_value
+        swift_mock = connection_mock.return_value.object_store
-
-        expected_obj = self.data
-        connection_obj_mock.get_object.return_value = ('headers', expected_obj)
 
         swift_obj = swiftapi.get_object('object')
 
-        connection_obj_mock.get_object.assert_called_once_with(
+        swift_mock.download_object.assert_called_once_with(
-            'ironic-inspector', 'object')
+            'object', container='ironic-inspector')
-        self.assertEqual(expected_obj, swift_obj)
+        self.assertIs(swift_mock.download_object.return_value, swift_obj)
 
-    def test_get_object_fails(self, connection_mock, load_mock,
+    def test_get_object_fails(self, connection_mock, load_mock):
-                              opts_mock, adapter_mock):
         swiftapi = swift.SwiftAPI()
-        connection_obj_mock = connection_mock.return_value
+        swift_mock = connection_mock.return_value.object_store
-        connection_obj_mock.get_object.side_effect = self.swift_exception
+        swift_mock.download_object.side_effect = os_exc.SDKException
         self.assertRaises(utils.Error, swiftapi.get_object,
                           'object')
-        connection_obj_mock.get_object.assert_called_once_with(
+        swift_mock.download_object.assert_called_once_with(
-            'ironic-inspector', 'object')
+            'object', container='ironic-inspector')

lower-constraints.txt

@@ -56,7 +56,7 @@ munch==2.2.0
 netaddr==0.7.18
 netifaces==0.10.6
 openstackdocstheme==1.18.1
-openstacksdk==0.12.0
+openstacksdk==0.30.0
 os-api-ref==1.4.0
 os-client-config==1.29.0
 os-service-types==1.2.0
@@ -97,7 +97,6 @@ python-ironicclient==2.3.0
 python-keystoneclient==3.15.0
 python-mimeparse==1.6.0
 python-subunit==1.2.0
-python-swiftclient==3.2.0
 pytz==2013.6
 PyYAML==3.12
 reno==2.5.0

releasenotes/notes/swift-max-retries-dfaecb74bd3aba9a.yaml

@@ -0,0 +1,5 @@
+---
+deprecations:
+  - |
+    The configuration option ``[swift]max_retries`` is deprecated. It has been
+    doing nothing for a few releases already.

requirements.txt

@@ -16,8 +16,8 @@ keystonemiddleware>=4.17.0 # Apache-2.0
 netaddr>=0.7.18 # BSD
 pbr!=2.1.0,>=2.0.0 # Apache-2.0
 python-ironicclient>=2.3.0 # Apache-2.0
-python-swiftclient>=3.2.0 # Apache-2.0
 pytz>=2013.6 # MIT
+openstacksdk>=0.30.0 # Apache-2.0
 oslo.concurrency>=3.26.0 # Apache-2.0
 oslo.config>=5.2.0 # Apache-2.0
 oslo.context>=2.19.2 # Apache-2.0