From b2922384054ddbd46e071fd07372a75a21d7f85d Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Tue, 3 Nov 2020 12:31:40 +0100 Subject: [PATCH] Add backup_swift_create_storage_policy config opt This configuration option is used by the Swift backup driver to select the storage policy to use when creating the container used to store backups. It defaults to None meaning the default behavior is preserved. Change-Id: I0a4a9d4e099c7d4a1d209be28ade5a517aeb96df --- cinder/backup/drivers/swift.py | 31 ++++++++++-- .../unit/backup/drivers/test_backup_swift.py | 48 +++++++++++++++++++ cinder/tests/unit/backup/fake_swift_client.py | 8 ++-- .../tests/unit/backup/fake_swift_client2.py | 2 +- ...ainer-storage-policy-8d4a268ed61b9fe2.yaml | 8 ++++ 5 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/add-backup-swift-container-storage-policy-8d4a268ed61b9fe2.yaml diff --git a/cinder/backup/drivers/swift.py b/cinder/backup/drivers/swift.py index 4afc5bae27b..00c77bdf053 100644 --- a/cinder/backup/drivers/swift.py +++ b/cinder/backup/drivers/swift.py @@ -51,6 +51,7 @@ from oslo_log import log as logging from oslo_utils import secretutils from oslo_utils import timeutils from swiftclient import client as swift +from swiftclient import exceptions as swift_exc from cinder.backup import chunkeddriver from cinder import exception @@ -108,6 +109,11 @@ swiftbackup_service_opts = [ cfg.StrOpt('backup_swift_container', default='volumebackups', help='The default Swift container to use'), + cfg.StrOpt('backup_swift_create_storage_policy', + default=None, + help='The storage policy to use when creating the Swift ' + 'container. If the container already exists the ' + 'storage policy cannot be enforced'), cfg.IntOpt('backup_swift_object_size', default=52428800, help='The size in bytes of Swift backup objects'), @@ -318,12 +324,31 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): return body def put_container(self, container): - """Create the container if needed. No failure if it pre-exists.""" + """Create the container if needed. + + Check if the container exist by issuing a HEAD request, if + the container does not exist we create it. + + We cannot enforce a new storage policy on an + existing container. + """ try: - self.conn.put_container(container) + self.conn.head_container(container) + except swift_exc.ClientException as e: + if e.http_status == 404: + try: + storage_policy = CONF.backup_swift_create_storage_policy + headers = ({'X-Storage-Policy': storage_policy} + if storage_policy else None) + self.conn.put_container(container, headers=headers) + except socket.error as err: + raise exception.SwiftConnectionFailed(reason=err) + return + LOG.warning("Failed to HEAD container to determine if it " + "exists and should be created.") + raise exception.SwiftConnectionFailed(reason=e) except socket.error as err: raise exception.SwiftConnectionFailed(reason=err) - return def get_container_entries(self, container, prefix): """Get container entry names""" diff --git a/cinder/tests/unit/backup/drivers/test_backup_swift.py b/cinder/tests/unit/backup/drivers/test_backup_swift.py index 608a85a0155..8c73d6c74c5 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_swift.py +++ b/cinder/tests/unit/backup/drivers/test_backup_swift.py @@ -299,6 +299,54 @@ class BackupSwiftTestCase(test.TestCase): starting_backoff=ANY, cacert=ANY) + @mock.patch.object(fake_swift_client.FakeSwiftConnection, 'put_container') + def test_default_backup_swift_create_storage_policy(self, mock_put): + service = swift_dr.SwiftBackupDriver(self.ctxt) + service.put_container('missing_container') + mock_put.assert_called_once_with('missing_container', headers=None) + + @mock.patch.object(fake_swift_client.FakeSwiftConnection, 'put_container') + def test_backup_swift_create_storage_policy(self, mock_put): + self.override_config('backup_swift_create_storage_policy', + 'mypolicy') + service = swift_dr.SwiftBackupDriver(self.ctxt) + service.put_container('missing_container') + mock_put.assert_called_once_with( + 'missing_container', + headers={'X-Storage-Policy': 'mypolicy'} + ) + + def test_default_backup_swift_create_storage_policy_put_socket_error(self): + service = swift_dr.SwiftBackupDriver(self.ctxt) + self.assertRaises(exception.SwiftConnectionFailed, + service.put_container, + 'missing_container_socket_error_on_put') + + def test_default_backup_swift_create_storage_policy_head_error(self): + service = swift_dr.SwiftBackupDriver(self.ctxt) + self.assertRaises(exception.SwiftConnectionFailed, + service.put_container, 'unauthorized_container') + + def test_backup_swift_create_storage_policy_head_error(self): + self.override_config('backup_swift_create_storage_policy', + 'mypolicy') + service = swift_dr.SwiftBackupDriver(self.ctxt) + self.assertRaises(exception.SwiftConnectionFailed, + service.put_container, + 'unauthorized_container') + + def test_default_backup_swift_create_storage_policy_head_sockerr(self): + service = swift_dr.SwiftBackupDriver(self.ctxt) + self.assertRaises(exception.SwiftConnectionFailed, + service.put_container, 'socket_error_on_head') + + def test_backup_swift_create_storage_policy_head_socket_error(self): + self.override_config('backup_swift_create_storage_policy', + 'mypolicy') + service = swift_dr.SwiftBackupDriver(self.ctxt) + self.assertRaises(exception.SwiftConnectionFailed, + service.put_container, 'socket_error_on_head') + def test_backup_uncompressed(self): volume_id = '2b9f10a3-42b4-4fdf-b316-000000ceb039' self._create_backup_db_entry(volume_id=volume_id) diff --git a/cinder/tests/unit/backup/fake_swift_client.py b/cinder/tests/unit/backup/fake_swift_client.py index 4602b436c96..0a9e885a59b 100644 --- a/cinder/tests/unit/backup/fake_swift_client.py +++ b/cinder/tests/unit/backup/fake_swift_client.py @@ -38,7 +38,8 @@ class FakeSwiftConnection(object): pass def head_container(self, container): - if container == 'missing_container': + if container in ['missing_container', + 'missing_container_socket_error_on_put']: raise swift.ClientException('fake exception', http_status=http_client.NOT_FOUND) elif container == 'unauthorized_container': @@ -48,8 +49,9 @@ class FakeSwiftConnection(object): raise socket.error(111, 'ECONNREFUSED') pass - def put_container(self, container): - pass + def put_container(self, container, headers=None): + if container == 'missing_container_socket_error_on_put': + raise socket.error(111, 'ECONNREFUSED') def get_container(self, container, **kwargs): fake_header = None diff --git a/cinder/tests/unit/backup/fake_swift_client2.py b/cinder/tests/unit/backup/fake_swift_client2.py index cf64fbae548..512fb392ac4 100644 --- a/cinder/tests/unit/backup/fake_swift_client2.py +++ b/cinder/tests/unit/backup/fake_swift_client2.py @@ -46,7 +46,7 @@ class FakeSwiftConnection2(object): elif container == 'socket_error_on_head': raise socket.error(111, 'ECONNREFUSED') - def put_container(self, container): + def put_container(self, container, headers=None): pass def get_container(self, container, **kwargs): diff --git a/releasenotes/notes/add-backup-swift-container-storage-policy-8d4a268ed61b9fe2.yaml b/releasenotes/notes/add-backup-swift-container-storage-policy-8d4a268ed61b9fe2.yaml new file mode 100644 index 00000000000..6bbb6667cd7 --- /dev/null +++ b/releasenotes/notes/add-backup-swift-container-storage-policy-8d4a268ed61b9fe2.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Swift backup driver: Added new configuration option ``backup_swift_create_storage_policy`` + for the Swift backup driver. If specified it will be used as the storage policy when + creating the Swift Container, default value is None meaning it will not be used and Swift + will use the system default. Please note that this only applies if a container doesn't exist + as we cannot update the storage policy on an already existing container.