]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
remove _check_container_exists from Swift backup driver
authorGiulio Fidente <gfidente@redhat.com>
Wed, 5 Feb 2014 00:13:41 +0000 (01:13 +0100)
committerAvishay Traeger <avishay@gmail.com>
Tue, 4 Mar 2014 05:56:26 +0000 (07:56 +0200)
The HTTP PUT on a Swift container is idempotent, we don't need to
check if it exists before uploading a backup.

This fixes a side issue where swiftclient was printing a TRACE of
the failed HTTP HEAD, previously in use to check if the container
existed.

Change-Id: I7bc267c948b56e30658997a9743f0d46ebb362ab

cinder/backup/drivers/swift.py
cinder/tests/test_backup_swift.py

index 30b818e4116e1ccf60f5ab2a09f26a4565d2fb1f..06f08359effcf1502e5e4545e1b26e14cb1c745e 100644 (file)
@@ -31,7 +31,6 @@
 """
 
 import hashlib
-import httplib
 import json
 import os
 import socket
@@ -135,20 +134,6 @@ class SwiftBackupDriver(BackupDriver):
                                          preauthtoken=self.context.auth_token,
                                          starting_backoff=self.swift_backoff)
 
-    def _check_container_exists(self, container):
-        LOG.debug(_('_check_container_exists: container: %s') % container)
-        try:
-            self.conn.head_container(container)
-        except swift.ClientException as error:
-            if error.http_status == httplib.NOT_FOUND:
-                LOG.debug(_('container %s does not exist') % container)
-                return False
-            else:
-                raise
-        else:
-            LOG.debug(_('container %s exists') % container)
-            return True
-
     def _create_container(self, context, backup):
         backup_id = backup['id']
         container = backup['container']
@@ -158,8 +143,11 @@ class SwiftBackupDriver(BackupDriver):
         if container is None:
             container = CONF.backup_swift_container
             self.db.backup_update(context, backup_id, {'container': container})
-        if not self._check_container_exists(container):
-            self.conn.put_container(container)
+        # NOTE(gfidente): accordingly to the Object Storage API reference, we
+        # do not need to check if a container already exists, container PUT
+        # requests are idempotent and a code of 202 (Accepted) is returned when
+        # the container already existed.
+        self.conn.put_container(container)
         return container
 
     def _generate_swift_object_name_prefix(self, backup):
index 5b495aa022b3c29bed5ce31c34cac27fee08d312..202d175e5dc575564e6f00ea303e1db13f694731 100644 (file)
@@ -121,16 +121,6 @@ class BackupSwiftTestCase(test.TestCase):
         backup = db.backup_get(self.ctxt, 123)
         self.assertEqual(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 = SwiftBackupDriver(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)
@@ -195,13 +185,3 @@ class BackupSwiftTestCase(test.TestCase):
         compressor = service._get_compressor('bz2')
         self.assertEqual(compressor, bz2)
         self.assertRaises(ValueError, service._get_compressor, 'fake')
-
-    def test_check_container_exists(self):
-        service = SwiftBackupDriver(self.ctxt)
-        exists = service._check_container_exists('fake_container')
-        self.assertEqual(exists, True)
-        exists = service._check_container_exists('missing_container')
-        self.assertEqual(exists, False)
-        self.assertRaises(swift.ClientException,
-                          service._check_container_exists,
-                          'unauthorized_container')