Improved fail_reason for cinder-backup swift connection errors

Modified swift backup service to catch socket errors when talking to
swift and raise a specific SwiftConnectionFailed exception in these
cases. This allows us to provide a more readable error message
detailing the problem connecting to swift to the end user when they
view the backup. Also reduced the default number of swift retries
so devstack environments fail faster - production environments can
tune these in cinder.conf.

Fixes bug: 1132791

Change-Id: Ibca744ea5adcbd31d068ac3d858bde6a4a0c9844

diff --git a/cinder/backup/services/swift.py b/cinder/backup/services/swift.py
index 259cb8e85f2efa620255e65bac126d68fa7b96d7..8ed568aba41acd9b4bed9e07fde26f922c45c4b2 100644 (file)
--- a/cinder/backup/services/swift.py
+++ b/cinder/backup/services/swift.py
@@ -34,6 +34,7 @@ import hashlib
 import httplib
 import json
 import os
+import socket
 import StringIO
 
 import eventlet
@@ -59,10 +60,10 @@ swiftbackup_service_opts = [
                default=52428800,
                help='The size in bytes of Swift backup objects'),
     cfg.IntOpt('backup_swift_retry_attempts',
-               default=10,
+               default=3,
                help='The number of retries to make for Swift operations'),
     cfg.IntOpt('backup_swift_retry_backoff',
-               default=10,
+               default=2,
                help='The backoff time in seconds between Swift retries'),
     cfg.StrOpt('backup_compression_algorithm',
                default='zlib',
@@ -206,7 +207,10 @@ class SwiftBackupService(base.Base):
             err = _('volume size %d is invalid.') % volume['size']
             raise exception.InvalidVolume(reason=err)
 
-        container = self._create_container(self.context, backup)
+        try:
+            container = self._create_container(self.context, backup)
+        except socket.error as err:
+            raise exception.SwiftConnectionFailed(reason=str(err))
 
         object_prefix = self._generate_swift_object_name_prefix(backup)
         backup['service_metadata'] = object_prefix
@@ -246,7 +250,10 @@ class SwiftBackupService(base.Base):
 
             reader = StringIO.StringIO(data)
             LOG.debug(_('About to put_object'))
-            etag = self.conn.put_object(container, object_name, reader)
+            try:
+                etag = self.conn.put_object(container, object_name, reader)
+            except socket.error as err:
+                raise exception.SwiftConnectionFailed(reason=str(err))
             LOG.debug(_('swift MD5 for %(object_name)s: %(etag)s') % locals())
             md5 = hashlib.md5(data).hexdigest()
             obj[object_name]['md5'] = md5
@@ -260,7 +267,10 @@ class SwiftBackupService(base.Base):
             object_id += 1
             LOG.debug(_('Calling eventlet.sleep(0)'))
             eventlet.sleep(0)
-        self._write_metadata(backup, volume_id, container, object_list)
+        try:
+            self._write_metadata(backup, volume_id, container, object_list)
+        except socket.error as err:
+            raise exception.SwiftConnectionFailed(reason=str(err))
         self.db.backup_update(self.context, backup_id, {'object_count':
                                                         object_id})
         LOG.debug(_('backup %s finished.') % backup_id)
@@ -279,7 +289,10 @@ class SwiftBackupService(base.Base):
                     ' container: %(container)s, to volume %(volume_id)s, '
                     'backup: %(backup_id)s') % locals())
         swift_object_names = self._generate_object_names(backup)
-        metadata_objects = self._read_metadata(backup)
+        try:
+            metadata_objects = self._read_metadata(backup)
+        except socket.error as err:
+            raise exception.SwiftConnectionFailed(reason=str(err))
         metadata_object_names = []
         for metadata_object in metadata_objects:
             metadata_object_names.extend(metadata_object.keys())
@@ -298,7 +311,10 @@ class SwiftBackupService(base.Base):
             LOG.debug(_('restoring object from swift. backup: %(backup_id)s, '
                         'container: %(container)s, swift object name: '
                         '%(object_name)s, volume: %(volume_id)s') % locals())
-            (resp, body) = self.conn.get_object(container, object_name)
+            try:
+                (resp, body) = self.conn.get_object(container, object_name)
+            except socket.error as err:
+                raise exception.SwiftConnectionFailed(reason=str(err))
             compression_algorithm = metadata_object[object_name]['compression']
             decompressor = self._get_compressor(compression_algorithm)
             if decompressor is not None:
@@ -336,6 +352,8 @@ class SwiftBackupService(base.Base):
             for swift_object_name in swift_object_names:
                 try:
                     self.conn.delete_object(container, swift_object_name)
+                except socket.error as err:
+                    raise exception.SwiftConnectionFailed(reason=str(err))
                 except Exception:
                     LOG.warn(_('swift error while deleting object %s, '
                                'continuing with delete') % swift_object_name)

diff --git a/cinder/exception.py b/cinder/exception.py
index 5d6b5c119ef46b587fba67a99672b1f2d538bc8f..24d13e984da2476d7341ea926b5376148e831650 100644 (file)
--- a/cinder/exception.py
+++ b/cinder/exception.py
@@ -558,3 +558,7 @@ class BackupNotFound(NotFound):
 
 class InvalidBackup(Invalid):
     message = _("Invalid backup: %(reason)s")
+
+
+class SwiftConnectionFailed(CinderException):
+    message = _("Connection to swift failed") + ": %(reason)s"

diff --git a/cinder/tests/backup/fake_swift_client.py b/cinder/tests/backup/fake_swift_client.py
index 66ef167cfd5e38817e132e5098492aabaa3aa10f..0093f1194d93873a5865365e6907bc9e1e10f6fc 100644 (file)
--- a/cinder/tests/backup/fake_swift_client.py
+++ b/cinder/tests/backup/fake_swift_client.py
@@ -16,6 +16,7 @@
 import httplib
 import json
 import os
+import socket
 import zlib
 
 from cinder.openstack.common import log as logging
@@ -45,9 +46,11 @@ class FakeSwiftConnection(object):
         if container == 'missing_container':
             raise swift.ClientException('fake exception',
                                         http_status=httplib.NOT_FOUND)
-        if container == 'unauthorized_container':
+        elif container == 'unauthorized_container':
             raise swift.ClientException('fake exception',
                                         http_status=httplib.UNAUTHORIZED)
+        elif container == 'socket_error_on_head':
+            raise socket.error(111, 'ECONNREFUSED')
         pass
 
     def put_container(self, container):
@@ -68,6 +71,8 @@ class FakeSwiftConnection(object):
 
     def get_object(self, container, name):
         LOG.debug("fake get_object(%s, %s)" % (container, name))
+        if container == 'socket_error_on_get':
+            raise socket.error(111, 'ECONNREFUSED')
         if 'metadata' in name:
             fake_object_header = None
             metadata = {}
@@ -92,8 +97,12 @@ class FakeSwiftConnection(object):
 
     def put_object(self, container, name, reader):
         LOG.debug("fake put_object(%s, %s)" % (container, name))
+        if container == 'socket_error_on_put':
+            raise socket.error(111, 'ECONNREFUSED')
         return 'fake-md5-sum'
 
     def delete_object(self, container, name):
         LOG.debug("fake delete_object(%s, %s)" % (container, name))
+        if container == 'socket_error_on_delete':
+            raise socket.error(111, 'ECONNREFUSED')
         pass

diff --git a/cinder/tests/test_backup_swift.py b/cinder/tests/test_backup_swift.py
index 163fb5a7e5e8cdf9e5f6e1b00ec422619859d137..d5dc564754f506622c9c348d823c7e88a78962b7 100644 (file)
--- a/cinder/tests/test_backup_swift.py
+++ b/cinder/tests/test_backup_swift.py
@@ -26,6 +26,7 @@ import zlib
 from cinder.backup.services.swift import SwiftBackupService
 from cinder import context
 from cinder import db
+from cinder import exception
 from cinder import flags
 from cinder.openstack.common import log as logging
 from cinder import test
@@ -121,6 +122,26 @@ class BackupSwiftTestCase(test.TestCase):
         backup = db.backup_get(self.ctxt, 123)
         self.assertEquals(backup['container'], container_name)
 
+    def test_create_backup_container_check_wraps_socket_error(self):
+        container_name = 'socket_error_on_head'
+        self._create_backup_db_entry(container=container_name)
+        service = SwiftBackupService(self.ctxt)
+        self.volume_file.seek(0)
+        backup = db.backup_get(self.ctxt, 123)
+        self.assertRaises(exception.SwiftConnectionFailed,
+                          service.backup,
+                          backup, self.volume_file)
+
+    def test_create_backup_put_object_wraps_socket_error(self):
+        container_name = 'socket_error_on_put'
+        self._create_backup_db_entry(container=container_name)
+        service = SwiftBackupService(self.ctxt)
+        self.volume_file.seek(0)
+        backup = db.backup_get(self.ctxt, 123)
+        self.assertRaises(exception.SwiftConnectionFailed,
+                          service.backup,
+                          backup, self.volume_file)
+
     def test_restore(self):
         self._create_backup_db_entry()
         service = SwiftBackupService(self.ctxt)
@@ -129,12 +150,32 @@ class BackupSwiftTestCase(test.TestCase):
             backup = db.backup_get(self.ctxt, 123)
             service.restore(backup, '1234-5678-1234-8888', volume_file)
 
+    def test_restore_wraps_socket_error(self):
+        container_name = 'socket_error_on_get'
+        self._create_backup_db_entry(container=container_name)
+        service = SwiftBackupService(self.ctxt)
+
+        with tempfile.NamedTemporaryFile() as volume_file:
+            backup = db.backup_get(self.ctxt, 123)
+            self.assertRaises(exception.SwiftConnectionFailed,
+                              service.restore,
+                              backup, '1234-5678-1234-8888', volume_file)
+
     def test_delete(self):
         self._create_backup_db_entry()
         service = SwiftBackupService(self.ctxt)
         backup = db.backup_get(self.ctxt, 123)
         service.delete(backup)
 
+    def test_delete_wraps_socket_error(self):
+        container_name = 'socket_error_on_delete'
+        self._create_backup_db_entry(container=container_name)
+        service = SwiftBackupService(self.ctxt)
+        backup = db.backup_get(self.ctxt, 123)
+        self.assertRaises(exception.SwiftConnectionFailed,
+                          service.delete,
+                          backup)
+
     def test_get_compressor(self):
         service = SwiftBackupService(self.ctxt)
         compressor = service._get_compressor('None')