From bb4321c61d40174b7314315568068a213697c370 Mon Sep 17 00:00:00 2001
From: Tom Barron <tpb@dyncloud.net>
Date: Mon, 24 Aug 2015 13:21:48 -0400
Subject: [PATCH] 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
---
 cinder/tests/unit/test_netapp_nfs.py          | 33 +++++++++++++++++--
 .../drivers/netapp/dataontap/nfs_base.py      | 11 +++----
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/cinder/tests/unit/test_netapp_nfs.py b/cinder/tests/unit/test_netapp_nfs.py
index 77861ab99..ac9567799 100644
--- 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 55f35078c..e3f562b3c 100644
--- 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)
-- 
2.45.2