From 193f3e97ce19bd8926bac23781e51d68de8ddcd4 Mon Sep 17 00:00:00 2001 From: Sofia Enriquez Date: Fri, 8 May 2020 20:24:48 +0000 Subject: [PATCH] Creating image-volume cache on NFS backend fails snapshot.id doesn't have `tmp-snap` in it as id is not passed as a option while creating snapshot object. This should be change to `snapshot.display_name` at same location Also includes regression and NITs from other patches (I33da9c65ef56b3e4967170e3a0fb25f12e067876) (Ia620043904f4f31a81bc8463fb36476a149f9808) (Id0b4981be5bf4e79aedcdad2258fc64aad8eeb72) Closes-Bug: #1875570 Change-Id: I66da0212a61d778fe9795d382b3af5b225a8aa54 (cherry picked from commit f68a64540f20967f933a3c9546837ddde23c20cd) --- cinder/tests/unit/volume/drivers/test_remotefs.py | 14 ++++++++++---- cinder/volume/drivers/remotefs.py | 3 ++- ...70-nfs-image-volume-cache-c45e840a6ec2a702.yaml | 6 ++++++ 3 files changed, 18 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/bug-1875570-nfs-image-volume-cache-c45e840a6ec2a702.yaml diff --git a/cinder/tests/unit/volume/drivers/test_remotefs.py b/cinder/tests/unit/volume/drivers/test_remotefs.py index 3d01f7cf00a..7ed53133706 100644 --- a/cinder/tests/unit/volume/drivers/test_remotefs.py +++ b/cinder/tests/unit/volume/drivers/test_remotefs.py @@ -256,10 +256,13 @@ class RemoteFsSnapDriverTestCase(test.TestCase): mock.call(*command3, run_as_root=True)] self._driver._execute.assert_has_calls(calls) - def _test_create_snapshot(self, volume_in_use=False, tmp_snap=False): + def _test_create_snapshot(self, volume_in_use=False, tmp_snap=False, + display_name=None): fake_snapshot_info = {} fake_snapshot_file_name = os.path.basename(self._fake_snapshot_path) + snapshot = self._fake_snapshot + snapshot.display_name = display_name self._driver._local_path_volume_info = mock.Mock( return_value=mock.sentinel.fake_info_path) self._driver._read_info_file = mock.Mock( @@ -278,16 +281,15 @@ class RemoteFsSnapDriverTestCase(test.TestCase): self._fake_snapshot.id: fake_snapshot_file_name } exp_acceptable_states = ['available', 'in-use', 'backing-up'] - if tmp_snap: + if display_name and display_name.startswith('tmp-snap-'): exp_acceptable_states.append('downloading') - self._fake_snapshot.id = 'tmp-snap-%s' % self._fake_snapshot.id + self._fake_snapshot.volume.status = 'downloading' if volume_in_use: self._fake_snapshot.volume.status = 'backing-up' self._fake_snapshot.volume.attach_status = 'attached' expected_method_called = '_create_snapshot_online' else: - self._fake_snapshot.volume.status = 'available' expected_method_called = '_do_create_snapshot' self._driver._create_snapshot(self._fake_snapshot) @@ -315,6 +317,10 @@ class RemoteFsSnapDriverTestCase(test.TestCase): self._driver._create_snapshot, self._fake_snapshot) + @ddt.data(None, 'test', 'tmp-snap-404f-404') + def test_create_snapshot_names(self, display_name): + self._test_create_snapshot(display_name=display_name) + @mock.patch('cinder.db.snapshot_get') @mock.patch('time.sleep') def test_create_snapshot_online_with_concurrent_delete( diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index 531db027574..1e5dde3e80f 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -1438,7 +1438,8 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): status = snapshot.volume.status acceptable_states = ['available', 'in-use', 'backing-up'] - if snapshot.id.startswith('tmp-snap-'): + if (snapshot.display_name and + snapshot.display_name.startswith('tmp-snap-')): # This is an internal volume snapshot. In order to support # image caching, we'll allow creating/deleting such snapshots # while having volumes in 'downloading' state. diff --git a/releasenotes/notes/bug-1875570-nfs-image-volume-cache-c45e840a6ec2a702.yaml b/releasenotes/notes/bug-1875570-nfs-image-volume-cache-c45e840a6ec2a702.yaml new file mode 100644 index 00000000000..01da7f0e03f --- /dev/null +++ b/releasenotes/notes/bug-1875570-nfs-image-volume-cache-c45e840a6ec2a702.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Bug #1875570 `_: + Fixed issue with NFS backend where the image-volume cache was + never used to create a volume, even when the cache was enabled.