openstack
/
cinderlib
Code Issues Proposed changes

Merge "Fix Volume snapshots and connections on deletion"

This commit is contained in:
Zuul 2019-04-08 18:13:54 +00:00 committed by Gerrit Code Review
parent cbb21bdd9d 4219b90330
commit 257b9d43ee
6 changed files with 156 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
cinderlib/cinderlib.py
View File

@ -42,6 +42,7 @@ from cinderlib import nos_brick
from cinderlib import objects
from cinderlib import persistence
from cinderlib import serialization
from cinderlib import utils as cinderlib_utils
__all__ = ['setup', 'Backend']
@ -157,11 +158,9 @@ class Backend(object):
return vol
def _volume_removed(self, volume):
if self._volumes:
for i, vol in enumerate(self._volumes):
if vol.id == volume.id:
del self._volumes[i]
break
i, vol = cinderlib_utils.find_by_id(volume.id, self._volumes)
if vol:
del self._volumes[i]
@classmethod
def _start_creating_volume(cls, volume):

56
cinderlib/objects.py
View File

@ -31,6 +31,7 @@ from oslo_utils import timeutils
import six
from cinderlib import exception
from cinderlib import utils
LOG = logging.getLogger(__name__)
@ -447,6 +448,29 @@ class Volume(NamedObject):
finally:
self.save()
def _snapshot_removed(self, snapshot):
# The snapshot instance in memory could be out of sync and not be
# identical, so check by ID.
i, snap = utils.find_by_id(snapshot.id, self._snapshots)
if snap:
del self._snapshots[i]
i, ovo = utils.find_by_id(snapshot.id, self._ovo.snapshots.objects)
if ovo:
del self._ovo.snapshots.objects[i]
def _connection_removed(self, connection):
# The connection instance in memory could be out of sync and not be
# identical, so check by ID.
i, conn = utils.find_by_id(connection.id, self._connections)
if conn:
del self._connections[i]
ovo_conns = getattr(self._ovo, CONNECTIONS_OVO_FIELD).objects
i, ovo_conn = utils.find_by_id(connection.id, ovo_conns)
if ovo_conn:
del ovo_conns[i]
def delete(self):
if self.snapshots:
msg = 'Cannot delete volume %s with snapshots' % self.id
@ -559,10 +583,7 @@ class Volume(NamedObject):
def _disconnect(self, connection):
self._remove_export()
if self._connections:
self._connections.remove(connection)
ovo_conns = getattr(self._ovo, CONNECTIONS_OVO_FIELD).objects
ovo_conns.remove(connection._ovo)
self._connection_removed(connection)
if not self.connections:
self._ovo.status = 'available'
@ -747,12 +768,12 @@ class Connection(Object, LazyVolumeAttr):
if save:
conn.save()
# Restore circular reference only if we have all the elements
if conn._volume and conn._volume._connections is not None:
conn._volume._connections.append(conn)
ovo_conns = getattr(conn._volume._ovo,
CONNECTIONS_OVO_FIELD).objects
if ovo not in ovo_conns:
ovo_conns.append(ovo)
if conn._volume:
utils.add_by_id(conn, conn._volume._connections)
connections = getattr(conn._volume._ovo,
CONNECTIONS_OVO_FIELD).objects
utils.add_by_id(conn._ovo, connections)
return conn
def _disconnect(self, force=False):
@ -876,10 +897,9 @@ class Snapshot(NamedObject, LazyVolumeAttr):
if save:
snap.save()
# Restore circular reference only if we have all the elements
if snap._volume and snap._volume._snapshots is not None:
snap._volume._snapshots.append(snap)
if ovo not in snap._volume._ovo.snapshots.objects:
snap._volume._ovo.snapshots.objects.append(ovo)
if snap._volume:
utils.add_by_id(snap, snap._volume._snapshots)
utils.add_by_id(snap._ovo, snap._volume._ovo.snapshots.objects)
return snap
def create(self):
@ -903,12 +923,8 @@ class Snapshot(NamedObject, LazyVolumeAttr):
self._ovo.status = 'error_deleting'
self.save()
self._raise_with_resource()
if self._volume is not None and self._volume._snapshots is not None:
try:
self._volume._snapshots.remove(self)
self._volume._ovo.snapshots.objects.remove(self._ovo)
except ValueError:
pass
if self._volume is not None:
self._volume._snapshot_removed(self)
def create_volume(self, **new_vol_params):
new_vol_params.setdefault('size', self.volume_size)

10
cinderlib/tests/unit/objects/test_snapshot.py
View File

@ -73,7 +73,11 @@ class TestSnapshot(base.BaseTest):
self.persistence.set_snapshot.assert_called_once_with(self.snap)
def test_delete(self):
self.snap.delete()
with mock.patch.object(
self.vol, '_snapshot_removed',
wraps=self.vol._snapshot_removed) as snap_removed_mock:
self.snap.delete()
snap_removed_mock.assert_called_once_with(self.snap)
self.backend.driver.delete_snapshot.assert_called_once_with(
self.snap._ovo)
self.persistence.delete_snapshot.assert_called_once_with(self.snap)
@ -81,13 +85,15 @@ class TestSnapshot(base.BaseTest):
self.assertEqual([], self.vol._ovo.snapshots.objects)
self.assertEqual('deleted', self.snap._ovo.status)
def test_delete_error(self):
@mock.patch('cinderlib.objects.Volume._snapshot_removed')
def test_delete_error(self, snap_removed_mock):
self.backend.driver.delete_snapshot.side_effect = exception.NotFound
with self.assertRaises(exception.NotFound) as assert_context:
self.snap.delete()
self.assertEqual(self.snap, assert_context.exception.resource)
self.backend.driver.delete_snapshot.assert_called_once_with(
self.snap._ovo)
snap_removed_mock.assert_not_called()
self.persistence.delete_snapshot.assert_not_called()
self.assertEqual([self.snap], self.vol.snapshots)
self.assertEqual([self.snap._ovo], self.vol._ovo.snapshots.objects)

76
cinderlib/tests/unit/objects/test_volume.py
View File

@ -397,13 +397,15 @@ class TestVolume(base.BaseTest):
mock_conn._disconnect.assert_called_once_with(mock.sentinel.force)
mock_disconnect.assert_called_once_with(mock_conn)
@mock.patch('cinderlib.objects.Volume._connection_removed')
@mock.patch('cinderlib.objects.Volume._remove_export')
def test__disconnect(self, mock_remove_export):
def test__disconnect(self, mock_remove_export, mock_conn_removed):
vol = objects.Volume(self.backend_name, status='in-use', size=10)
vol._disconnect(mock.sentinel.connection)
mock_remove_export.assert_called_once_with()
mock_conn_removed.assert_called_once_with(mock.sentinel.connection)
self.assertEqual('available', vol.status)
self.persistence.set_volume.assert_called_once_with(vol)
@ -426,3 +428,75 @@ class TestVolume(base.BaseTest):
mock_remove_export.assert_called_once_with()
for c in connections:
c.detach.asssert_called_once_with()
def test__snapshot_removed_not_loaded(self):
vol = objects.Volume(self.backend,
name='vol_name', description='vol_desc', size=10)
vol._snapshots = None
snap = objects.Snapshot(vol)
# Just check it doesn't break
vol._snapshot_removed(snap)
def test__snapshot_removed_not_present(self):
vol = objects.Volume(self.backend,
name='vol_name', description='vol_desc', size=10)
snap = objects.Snapshot(vol)
snap2 = objects.Snapshot(vol)
vol._snapshots = [snap2]
vol._ovo.snapshots.objects = [snap2._ovo]
# Just check it doesn't break or remove any other snaps
vol._snapshot_removed(snap)
self.assertEqual([snap2], vol._snapshots)
self.assertEqual([snap2._ovo], vol._ovo.snapshots.objects)
def test__snapshot_removed(self):
vol = objects.Volume(self.backend,
name='vol_name', description='vol_desc', size=10)
snap = objects.Snapshot(vol)
snap2 = objects.Snapshot(vol)
snap_other_instance = objects.Snapshot(vol, id=snap.id,
description='d')
snap_other_instance2 = objects.Snapshot(vol, id=snap.id,
description='e')
vol._snapshots = [snap2, snap_other_instance]
vol._ovo.snapshots.objects = [snap2._ovo, snap_other_instance2._ovo]
# Just check it doesn't break or remove any other snaps
vol._snapshot_removed(snap)
self.assertEqual([snap2], vol._snapshots)
self.assertEqual([snap2._ovo], vol._ovo.snapshots.objects)
def test__connection_removed_not_loaded(self):
vol = objects.Volume(self.backend,
name='vol_name', description='vol_desc', size=10)
vol._connections = None
conn = objects.Connection(self.backend, connection_info={'conn': {}})
# Just check it doesn't break
vol._connection_removed(conn)
def test__connection_removed_not_present(self):
vol = objects.Volume(self.backend,
name='vol_name', description='vol_desc', size=10)
conn = objects.Connection(self.backend, connection_info={'conn': {}})
conn2 = objects.Connection(self.backend, connection_info={'conn': {}})
vol._connections = [conn2]
vol._ovo.volume_attachment.objects = [conn2._ovo]
# Just check it doesn't break or remove any other snaps
vol._connection_removed(conn)
self.assertEqual([conn2], vol._connections)
self.assertEqual([conn2._ovo], vol._ovo.volume_attachment.objects)
def test__connection_removed(self):
vol = objects.Volume(self.backend, size=10)
conn = objects.Connection(self.backend, connection_info={'conn': {}})
conn2 = objects.Connection(self.backend, connection_info={'conn': {}})
conn_other_instance = objects.Connection(self.backend, id=conn.id,
connection_info={'conn': {}})
conn_other_instance2 = objects.Connection(self.backend, id=conn.id,
connection_info={'conn': {}})
vol._connections = [conn2, conn_other_instance]
vol._ovo.volume_attachment.objects = [conn2._ovo,
conn_other_instance2._ovo]
# Just check it doesn't break or remove any other snaps
vol._connection_removed(conn)
self.assertEqual([conn2], vol._connections)
self.assertEqual([conn2._ovo], vol._ovo.volume_attachment.objects)

3
cinderlib/tests/unit/test_cinderlib.py
View File

@ -176,7 +176,8 @@ class TestCinderlib(base.BaseTest):
mock_vol.return_value.create.assert_called_once_with()
def test__volume_removed_no_list(self):
self.backend._volume_removed(mock.sentinel.volume)
vol = cinderlib.objects.Volume(self.backend, size=10)
self.backend._volume_removed(vol)
def test__volume_removed(self):
vol = cinderlib.objects.Volume(self.backend, size=10)

31
cinderlib/utils.py Normal file
View File

@ -0,0 +1,31 @@
# Copyright (c) 2019, Red Hat, Inc.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
def find_by_id(resource_id, elements):
if elements:
for i, element in enumerate(elements):
if resource_id == element.id:
return i, element
return None, None
def add_by_id(resource, elements):
if elements is not None:
i, element = find_by_id(resource.id, elements)
if element:
elements[i] = resource
else:
elements.append(resource)