Browse Source

Merge "Fix cross-project incremental backups" into stable/stein

stable/stein
Zuul 2 weeks ago
committed by Gerrit Code Review
parent
commit
16e6cb4a7e
8 changed files with 45 additions and 25 deletions
  1. +3
    -2
      cinder/backup/api.py
  2. +2
    -2
      cinder/db/api.py
  3. +2
    -2
      cinder/db/sqlalchemy/api.py
  4. +4
    -2
      cinder/objects/backup.py
  5. +14
    -11
      cinder/tests/unit/backup/test_backup.py
  6. +5
    -3
      cinder/tests/unit/objects/test_backup.py
  7. +9
    -3
      cinder/tests/unit/test_db_api.py
  8. +6
    -0
      releasenotes/notes/bug-1869746-cross-project-incremental-backup-error.yaml

+ 3
- 2
cinder/backup/api.py View File

@@ -247,8 +247,9 @@ class API(base.Base):
# incremental backup.
latest_backup = None
if incremental:
backups = objects.BackupList.get_all_by_volume(context.elevated(),
volume_id)
backups = objects.BackupList.get_all_by_volume(
context, volume_id, volume['project_id'],
filters={'project_id': context.project_id})
if backups.objects:
# NOTE(xyang): The 'data_timestamp' field records the time
# when the data on the volume was first saved. If it is


+ 2
- 2
cinder/db/api.py View File

@@ -1225,9 +1225,9 @@ def backup_get_all_by_project(context, project_id, filters=None, marker=None,
sort_dirs=sort_dirs)


def backup_get_all_by_volume(context, volume_id, filters=None):
def backup_get_all_by_volume(context, volume_id, vol_project_id, filters=None):
"""Get all backups belonging to a volume."""
return IMPL.backup_get_all_by_volume(context, volume_id,
return IMPL.backup_get_all_by_volume(context, volume_id, vol_project_id,
filters=filters)




+ 2
- 2
cinder/db/sqlalchemy/api.py View File

@@ -5184,9 +5184,9 @@ def backup_get_all_by_project(context, project_id, filters=None, marker=None,


@require_context
def backup_get_all_by_volume(context, volume_id, filters=None):
def backup_get_all_by_volume(context, volume_id, vol_project_id, filters=None):

authorize_project_context(context, volume_id)
authorize_project_context(context, vol_project_id)
if not filters:
filters = {}
else:


+ 4
- 2
cinder/objects/backup.py View File

@@ -261,8 +261,10 @@ class BackupList(base.ObjectListBase, base.CinderObject):
backups, expected_attrs=expected_attrs)

@classmethod
def get_all_by_volume(cls, context, volume_id, filters=None):
backups = db.backup_get_all_by_volume(context, volume_id, filters)
def get_all_by_volume(
cls, context, volume_id, vol_project_id, filters=None):
backups = db.backup_get_all_by_volume(
context, volume_id, vol_project_id, filters)
expected_attrs = Backup._get_expected_attrs(context)
return base.obj_make_list(context, cls(context), objects.Backup,
backups, expected_attrs=expected_attrs)


+ 14
- 11
cinder/tests/unit/backup/test_backup.py View File

@@ -123,7 +123,8 @@ class BaseBackupTest(test.TestCase):
previous_status='available',
size=1,
host='testhost',
encryption_key_id=None):
encryption_key_id=None,
project_id=None):
"""Create a volume entry in the DB.

Return the entry ID
@@ -131,8 +132,8 @@ class BaseBackupTest(test.TestCase):
vol = {}
vol['size'] = size
vol['host'] = host
vol['user_id'] = str(uuid.uuid4())
vol['project_id'] = str(uuid.uuid4())
vol['user_id'] = fake.USER_ID
vol['project_id'] = project_id or fake.PROJECT_ID
vol['status'] = status
vol['display_name'] = display_name
vol['display_description'] = display_description
@@ -2154,19 +2155,17 @@ class BackupAPITestCase(BaseBackupTest):
self.ctxt, backups=1, backup_gigabytes=1)
mock_rollback.assert_called_with(self.ctxt, "fake_reservation")

@mock.patch('cinder.db.backup_get_all_by_volume')
@mock.patch('cinder.backup.rpcapi.BackupAPI.create_backup')
@mock.patch.object(api.API, '_get_available_backup_service_host',
return_value='fake_host')
@mock.patch('cinder.objects.BackupList.get_all_by_volume')
@mock.patch.object(quota.QUOTAS, 'rollback')
@mock.patch.object(quota.QUOTAS, 'reserve')
def test_create_backup_failed_with_empty_backup_objects(
self, mock_reserve, mock_rollback, mock_get_service,
mock_create, mock_get_backups):
mock_get_backups.return_value = [v2_fakes.fake_backup('fake-1')]
backups = objects.BackupList.get_all_by_volume(self.ctxt,
fake.VOLUME_ID)
self, mock_reserve, mock_rollback, mock_get_backups,
mock_host):
backups = mock.Mock()
backups.objects = []
mock_get_backups.return_value = backups
is_incremental = True
self.ctxt.user_id = 'fake_user'
self.ctxt.project_id = 'fake_project'
@@ -2174,7 +2173,8 @@ class BackupAPITestCase(BaseBackupTest):

volume_id = self._create_volume_db_entry(status='available',
host='testhost#rbd',
size=1)
size=1,
project_id="vol_proj_id")
self.assertRaises(exception.InvalidBackup,
self.api.create,
self.ctxt,
@@ -2182,6 +2182,9 @@ class BackupAPITestCase(BaseBackupTest):
volume_id, None,
incremental=is_incremental)
mock_rollback.assert_called_with(self.ctxt, "fake_reservation")
mock_get_backups.assert_called_once_with(
self.ctxt, volume_id, 'vol_proj_id',
filters={'project_id': 'fake_project'})

@mock.patch('cinder.db.backup_get_all_by_volume',
return_value=[v2_fakes.fake_backup('fake-1')])


+ 5
- 3
cinder/tests/unit/objects/test_backup.py View File

@@ -290,11 +290,13 @@ class TestBackupList(test_objects.BaseObjectsTestCase):
@mock.patch('cinder.db.backup_get_all_by_volume',
return_value=[fake_backup])
def test_get_all_by_volume(self, get_all_by_volume):
backups = objects.BackupList.get_all_by_volume(self.context,
fake.VOLUME_ID)
backups = objects.BackupList.get_all_by_volume(
self.context, fake.VOLUME_ID, 'fake_proj')
self.assertEqual(1, len(backups))
get_all_by_volume.assert_called_once_with(self.context,
fake.VOLUME_ID, None)
fake.VOLUME_ID,
'fake_proj',
None)
TestBackup._compare(self, fake_backup, backups[0])




+ 9
- 3
cinder/tests/unit/test_db_api.py View File

@@ -2698,15 +2698,21 @@ class DBAPIBackupTestCase(BaseTest):
{'fake_key': 'fake'})
self._assertEqualListsOfObjects([], byproj)

def test_backup_get_all_by_volume(self):
byvol = db.backup_get_all_by_volume(self.ctxt,
self.created[1]['volume_id'])
@mock.patch.object(sqlalchemy_api, 'authorize_project_context')
def test_backup_get_all_by_volume(self, mock_authorize):
byvol = db.backup_get_all_by_volume(
self.ctxt, self.created[1]['volume_id'], 'fake_proj')
self._assertEqualObjects(self.created[1], byvol[0])

byvol = db.backup_get_all_by_volume(self.ctxt,
self.created[1]['volume_id'],
'fake_proj',
{'fake_key': 'fake'})
self._assertEqualListsOfObjects([], byvol)
mock_authorize.assert_has_calls([
mock.call(self.ctxt, 'fake_proj'),
mock.call(self.ctxt, 'fake_proj')
])

def test_backup_update_nonexistent(self):
self.assertRaises(exception.BackupNotFound,


+ 6
- 0
releasenotes/notes/bug-1869746-cross-project-incremental-backup-error.yaml View File

@@ -0,0 +1,6 @@
---
fixes:
- |
Cinder no longer allows an incremental backup to be
created while having the parent backup in another
project.

Loading…
Cancel
Save