From 8c5ed29ba6e293a47112772e58bbed1ce4f0fe94 Mon Sep 17 00:00:00 2001
From: Valeriy Ponomaryov <vponomaryov@mirantis.com>
Date: Thu, 1 Dec 2016 21:49:15 +0300
Subject: [PATCH] [Devstack] Fix DHSS=False setup for Generic driver

Recent change to devstack [1] broke our DHSS=False CI job running
generic driver that was depending on ip route from host to
private network. So, to avoid this error use floating ip address
for connection to service Nova VMs from host machine.
Also, fix generation of second export location that should be dedicated
for service needs such as mounting share doing host-assisted
migration.

[1] If45e3fc15c050cfbac11b57c1eaf137dd7ed816f

Change-Id: Ieea992293ae02898741c939da15f0dbb4609d8b0
Closes-Bug: #1644523
Closes-Bug: #1646097
---
 contrib/ci/common.sh                       | 23 +++++---------------
 devstack/plugin.sh                         |  9 +++++---
 manila/share/drivers/generic.py            |  9 ++++++--
 manila/tests/share/drivers/test_generic.py | 25 ++++++++++++++++------
 4 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/contrib/ci/common.sh b/contrib/ci/common.sh
index 07ebfa549f..48505548f9 100755
--- a/contrib/ci/common.sh
+++ b/contrib/ci/common.sh
@@ -42,25 +42,12 @@ function manila_wait_for_generic_driver_init {
         GENERIC_DRIVER='manila.share.drivers.generic.GenericShareDriver'
         DHSS=$(iniget $MANILA_CONF $driver_group driver_handles_share_servers)
         if [[ $SHARE_DRIVER == $GENERIC_DRIVER && $(trueorfalse False DHSS) == False ]]; then
-            # Wait for availability
+            # Wait for service VM availability
             source /opt/stack/new/devstack/openrc admin demo
-            vm_id=$(iniget $MANILA_CONF $driver_group service_instance_name_or_id)
-            vm_ips=$(nova show $vm_id | grep "private network")
-            attempts=0
-            for vm_ip in ${vm_ips//,/ }; do
-                # Get IPv4 address
-                if [[ $vm_ip =~ ^[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}$ ]]; then
-                    # Check availability
-                    ((attempts++))
-                    manila_check_service_vm_availability $vm_ip
-                    break
-                fi
-            done
-            if [[ (( attempts < 1 )) ]]; then
-                echo "No IPv4 addresses found among private IPs of '$vm_id' for '$GENERIC_DRIVER'. "\
-                    "Reported IPs: '$vm_ips'."
-                exit 1
-            fi
+            vm_ip=$(iniget $MANILA_CONF $driver_group service_net_name_or_ip)
+
+            # Check availability
+            manila_check_service_vm_availability $vm_ip
         fi
     done
 }
diff --git a/devstack/plugin.sh b/devstack/plugin.sh
index 8548539c6c..637f195b0c 100755
--- a/devstack/plugin.sh
+++ b/devstack/plugin.sh
@@ -300,7 +300,7 @@ function is_driver_enabled {
 # create_service_share_servers - creates service Nova VMs, one per generic
 # driver, and only if it is configured to mode without handling of share servers.
 function create_service_share_servers {
-    private_net_id=$(nova net-list | grep ' private ' | get_field 1)
+    private_net_id=$(openstack network show $PRIVATE_NETWORK_NAME -f value -c id)
     created_admin_network=false
     for BE in ${MANILA_ENABLED_BACKENDS//,/ }; do
         driver_handles_share_servers=$(iniget $MANILA_CONF $BE driver_handles_share_servers)
@@ -321,9 +321,12 @@ function create_service_share_servers {
 
                 vm_id=$(nova show $vm_name | grep ' id ' | get_field 2)
 
+                floating_ip=$(openstack floating ip create $PUBLIC_NETWORK_NAME --subnet $PUBLIC_SUBNET_NAME | grep 'floating_ip_address' | get_field 2)
+                openstack server add floating ip $vm_name $floating_ip
+
                 iniset $MANILA_CONF $BE service_instance_name_or_id $vm_id
-                iniset $MANILA_CONF $BE service_net_name_or_ip private
-                iniset $MANILA_CONF $BE tenant_net_name_or_ip private
+                iniset $MANILA_CONF $BE service_net_name_or_ip $floating_ip
+                iniset $MANILA_CONF $BE tenant_net_name_or_ip $PRIVATE_NETWORK_NAME
             else
                 if is_service_enabled neutron; then
                     if ! [[ -z $MANILA_ADMIN_NET_RANGE ]]; then
diff --git a/manila/share/drivers/generic.py b/manila/share/drivers/generic.py
index 49bc5751b5..7e7fbfca4f 100644
--- a/manila/share/drivers/generic.py
+++ b/manila/share/drivers/generic.py
@@ -242,9 +242,14 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
                 "export_location_metadata_example": "example",
             },
         }]
-        if server_details.get('admin_ip'):
+        # NOTE(vponomaryov): 'admin_ip' exists only in case of DHSS=True mode.
+        # 'ip' exists in case of DHSS=False mode.
+        # Use one of these for creation of export location for service needs.
+        service_address = server_details.get(
+            "admin_ip", server_details.get("ip"))
+        if service_address:
             admin_location = location.replace(
-                server_details['public_address'], server_details['admin_ip'])
+                server_details['public_address'], service_address)
             export_list.append({
                 "path": admin_location,
                 "is_admin_only": True,
diff --git a/manila/tests/share/drivers/test_generic.py b/manila/tests/share/drivers/test_generic.py
index 7bbaaaf812..f9e380d74c 100644
--- a/manila/tests/share/drivers/test_generic.py
+++ b/manila/tests/share/drivers/test_generic.py
@@ -218,6 +218,7 @@ class GenericShareDriverTestCase(test.TestCase):
             'pk_path': 'fake_pk_path',
             'backend_details': {
                 'ip': '1.2.3.4',
+                'public_address': 'fake_public_address',
                 'instance_id': 'fake',
                 'service_ip': 'fake_ip',
             },
@@ -316,23 +317,33 @@ class GenericShareDriverTestCase(test.TestCase):
     def test_create_share(self):
         volume = 'fake_volume'
         volume2 = 'fake_volume2'
-        self._helper_nfs.create_export.return_value = 'fakelocation'
+        location = (
+            '%s:/fake/path' % self.server['backend_details']['public_address'])
+        self._helper_nfs.create_export.return_value = location
         self.mock_object(self._driver, '_allocate_container',
                          mock.Mock(return_value=volume))
         self.mock_object(self._driver, '_attach_volume',
                          mock.Mock(return_value=volume2))
         self.mock_object(self._driver, '_format_device')
         self.mock_object(self._driver, '_mount_device')
-        expected_el = [{
-            'is_admin_only': False,
-            'path': 'fakelocation',
-            'metadata': {'export_location_metadata_example': 'example'},
-        }]
+        expected_el = [
+            {'is_admin_only': False,
+             'path': location,
+             'metadata': {'export_location_metadata_example': 'example'}},
+            {'is_admin_only': True,
+             'path': location.replace(
+                 self.server['backend_details']['public_address'],
+                 self.server['backend_details']['ip']),
+             'metadata': {'export_location_metadata_example': 'example'}},
+        ]
 
         result = self._driver.create_share(
             self._context, self.share, share_server=self.server)
 
-        self.assertEqual(expected_el, result)
+        self.assertIsInstance(result, list)
+        self.assertEqual(2, len(result))
+        for el in expected_el:
+            self.assertIn(el, result)
         self._driver._allocate_container.assert_called_once_with(
             self._driver.admin_context, self.share)
         self._driver._attach_volume.assert_called_once_with(