From: John Griffith <john.griffith8@gmail.com>
Date: Tue, 24 Nov 2015 21:46:19 +0000 (-0700)
Subject: Update list_replication_targets
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=c30ce044b713113317bbeb1c3df2593bd6a6ec2b;p=openstack-build%2Fcinder-build.git

Update list_replication_targets

We were planning to let the list targets call be a
free for all, and just be a generic dict of info.

This probably isn't such a great idea, inparticular since
we have a required identifier field for each replication
device it makes more sense to just use that in the response
and leave the rest of the info internal.

In the future if there's a need we can always implement a
show command that displays all the dirty details.

This patch changes the identifier name to 'target_device_id'
to be more self describing, and updates the docstrings and
devref docs to reflect the changes and agreed upon changes.

Change-Id: If14130f5d5bb2e6df4478bd0e14a1f33d706bf78
---

diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index de3a0d183..97817155a 100644
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -215,7 +215,7 @@ volume_opts = [
                       "times in a single config section to specify multiple "
                       "replication target devices.  Each entry takes the "
                       "standard dict config form: replication_device = "
-                      "device_target_id:<required>,"
+                      "target_device_id:<required>,"
                       "managed_backend_name:<host@backend_name>,"
                       "key1:value1,key2:value2..."),
     cfg.BoolOpt('image_upload_use_cinder_backend',
diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 2a637cfd8..efe42f362 100644
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -3323,28 +3323,22 @@ class VolumeManager(manager.SchedulerDependentManager):
 
         This method is used to query a backend to get the current
         replication config info for the specified volume.
-
         In the case of a volume that isn't being replicated,
         the driver should return an empty list.
 
+        There is one required field for configuration of
+        replication, (target_device_id).  As such we
+        use that for the response in list_replication_targets.
 
-        Example response for replicating to a managed backend:
-            {'volume_id': volume['id'],
-             'targets':[{'remote_device_id': 'vendor-id-for-target-device',
-                         'managed_host': 'backend_name'}...]
-
-        Example response for replicating to an unmanaged backend:
-            {'volume_id': volume['id'], 'targets':[
-                {'remote_device_id': 'vendor-id-for-target-device',
-                                      'san_ip': '1.1.1.1',
-                                      'san_login': 'admin'},
-                                      ....]}
+        Internal methods can be added to extract additional
+        details if needed, but the only detail that should be
+        exposed to an end user is the identifier.  In the case
+        of an Admin, (s)he can always cross check the cinder.conf
+        file that they have configured for additional info.
 
-        NOTE: It's the responsibility of the driver to mask out any
-        passwords or sensitive information.
-
-        `remote_device_id` is required and is used for drivers to identify
-        the devices they have in use.
+        Example response:
+            {'volume_id': volume['id'],
+             'targets':[<target-device-id>,...]'
 
         """
         # NOTE(hemna) We intentionally don't enforce the driver being
diff --git a/doc/source/devref/replication.rst b/doc/source/devref/replication.rst
index 46a759599..bd1371e67 100644
--- a/doc/source/devref/replication.rst
+++ b/doc/source/devref/replication.rst
@@ -31,9 +31,12 @@ like to configure.
 There are two standardized keys in the config
 entry, all others are vendor-unique:
 
-* device_target_id:<vendor-identifier-for-rep-target>
+* target_device_id:<vendor-identifier-for-rep-target>
 * managed_backend_name:<cinder-backend-host-entry>,"
 
+target_device_id is REQUIRED in all configurations
+
+
 
 An example config entry for a managed replication device
 would look like this::
@@ -209,17 +212,14 @@ act as a toggle, allowing to switch back and forth betweeen primary and secondar
 
 **list_replication_targets**
 
-Used by the admin to query a volume for a list of configured replication targets
-The expected return for this call is expected to mimic the form used in the config file.
-
-For a volume replicating to managed replication targets::
-
-    {'volume_id': volume['id'], 'targets':[{'type': 'managed',
-                                            'backend_name': 'backend_name'}...]
-
-For a volume replicating to external/unmanaged targets::
+Used by the admin to query a volume for a list of configured replication targets.
 
-    {'volume_id': volume['id'], 'targets':[{'type': 'unmanaged',
-                                            'san_ip': '127.0.0.1',
-                                            'san_login': 'admin'...}...]
+The expected response is simply the single required field in replication-device
+configuration.  It's possible that in the future we may want to add a show
+command that provides all the various details about a target replication
+device.  This would be of the form:
+`show_replication_target <target_device_id>`
 
+Example response:
+    {'volume_id': volume['id'],
+     'targets':[<target-device-id>,...]'