Fix NetApp loop in clone of NFS backed images

If there is a list of locations for glance images and an unsuitable
location is encountered when iterating through the list, current code
breaks out of the loop, failing to find later, valid locations.

Here we fix this issue by ignoring invalid locations and continuing
through the loop rather than breaking out when we find invalid locations.

Closes-bug: 1487074

Change-Id: Id2435ba633629367fa1bd8f339fc283f47eeb012

diff --git a/cinder/tests/unit/test_netapp_nfs.py b/cinder/tests/unit/test_netapp_nfs.py
index 77861ab99347f8e42cc8633f116ccdd9663e6941..ac9567799ed8747e74741d9d1d46a7e98a1272b8 100644 (file)
--- a/cinder/tests/unit/test_netapp_nfs.py
+++ b/cinder/tests/unit/test_netapp_nfs.py
@@ -874,16 +874,45 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
     def test_construct_image_url_loc(self):
         drv = self._driver
         img_loc = (None,
+                   # Valid metdata
                    [{'metadata':
                      {'share_location': 'nfs://host/path',
                       'mountpoint': '/opt/stack/data/glance',
                       'id': 'abc-123',
                       'type': 'nfs'},
-                     'url': 'file:///opt/stack/data/glance/image-id'}])
+                     'url': 'file:///opt/stack/data/glance/image-id-0'},
+                    # missing metadata
+                    {'metadata': {},
+                     'url': 'file:///opt/stack/data/glance/image-id-1'},
+                    # missing location_type
+                    {'metadata': {'location_type': None},
+                     'url': 'file:///opt/stack/data/glance/image-id-2'},
+                    # non-nfs location_type
+                    {'metadata': {'location_type': 'not-NFS'},
+                     'url': 'file:///opt/stack/data/glance/image-id-3'},
+                    # missing share_location
+                    {'metadata': {'location_type': 'nfs',
+                                  'share_location': None},
+                     'url': 'file:///opt/stack/data/glance/image-id-4'},
+                    # missing mountpoint
+                    {'metadata': {'location_type': 'nfs',
+                                  'share_location': 'nfs://host/path',
+                                  # Pre-kilo we documented "mount_point"
+                                  'mount_point': '/opt/stack/data/glance'},
+                     'url': 'file:///opt/stack/data/glance/image-id-5'},
+                    # Valid metadata
+                    {'metadata':
+                     {'share_location': 'nfs://host/path',
+                      'mountpoint': '/opt/stack/data/glance',
+                      'id': 'abc-123',
+                      'type': 'nfs'},
+                     'url': 'file:///opt/stack/data/glance/image-id-6'}])
 
         locations = drv._construct_image_nfs_url(img_loc)
 
-        self.assertIn("nfs://host/path/image-id", locations)
+        self.assertIn("nfs://host/path/image-id-0", locations)
+        self.assertIn("nfs://host/path/image-id-6", locations)
+        self.assertEqual(2, len(locations))
 
     def test_construct_image_url_direct(self):
         drv = self._driver

diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py
index 55f35078ce9dbdee6be7f66bf13edf3c658f0eb0..e3f562b3c7a952cb5b68b73d0de2479f718af6e3 100644 (file)
--- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py
+++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py
@@ -676,19 +676,16 @@ class NetAppNfsDriver(driver.ManageableVD,
             urls.append(direct_url)
         else:
             for location in locations:
-                url = location['url']
                 if not location['metadata']:
-                    urls.append(url)
-                    break
+                    continue
                 location_type = location['metadata'].get('type')
                 if not location_type or location_type.lower() != "nfs":
-                    urls.append(url)
-                    break
+                    continue
                 share_location = location['metadata'].get('share_location')
                 mountpoint = location['metadata'].get('mountpoint')
                 if not share_location or not mountpoint:
-                    urls.append(url)
-                    break
+                    continue
+                url = location['url']
                 url_parse = urllib.parse.urlparse(url)
                 abs_path = os.path.join(url_parse.netloc, url_parse.path)
                 rel_path = os.path.relpath(abs_path, mountpoint)