From 3fc515e77aa8a6bc3218dbd36b8ffbb9e7bf5b97 Mon Sep 17 00:00:00 2001 From: Stephen Mulcahy Date: Thu, 28 Feb 2013 12:11:39 +0000 Subject: [PATCH] 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 --- cinder/backup/services/swift.py | 32 ++++++++++++++---- cinder/exception.py | 4 +++ cinder/tests/backup/fake_swift_client.py | 11 ++++++- cinder/tests/test_backup_swift.py | 41 ++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 8 deletions(-) diff --git a/cinder/backup/services/swift.py b/cinder/backup/services/swift.py index 259cb8e85..8ed568aba 100644 --- 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 5d6b5c119..24d13e984 100644 --- 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 66ef167cf..0093f1194 100644 --- 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 163fb5a7e..d5dc56475 100644 --- 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') -- 2.45.2