From c30ce044b713113317bbeb1c3df2593bd6a6ec2b Mon Sep 17 00:00:00 2001 From: John Griffith Date: Tue, 24 Nov 2015 14:46:19 -0700 Subject: [PATCH] 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 --- cinder/volume/driver.py | 2 +- cinder/volume/manager.py | 28 +++++++++++----------------- doc/source/devref/replication.rst | 26 +++++++++++++------------- 3 files changed, 25 insertions(+), 31 deletions(-) diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index de3a0d183df..97817155a8b 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:," + "target_device_id:," "managed_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 2a637cfd88f..efe42f362e0 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: + 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. + + Example response: {'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'}, - ....]} - - 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. + 'targets':[,...]' """ # 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 46a7595995a..bd1371e675b 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: +* target_device_id: * managed_backend_name:," +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. +Used by the admin to query a volume for a list of configured replication targets. -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:: - - {'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 ` +Example response: + {'volume_id': volume['id'], + 'targets':[,...]'