From 1db9c8db2d671a78840a74c8a7c98cdebe6b0da7 Mon Sep 17 00:00:00 2001 From: bhagyashree-sarawate <50358719+bhagyashree-sarawate@users.noreply.github.com> Date: Tue, 3 Sep 2019 16:24:59 +0530 Subject: [PATCH] fix for #718, #714, #711, #710 (#723) * fix for #718,#714,#711 * fixed uts related to message change * Added fix for #710 * Modified document as per review comment * fixed pep8 issues * done review comment --- docs/active-passive-based-replication.md | 4 ++-- docs/peer-persistence-based-replication.md | 4 ++-- hpedockerplugin/hpe/hpe_3par_common.py | 12 +++++------ hpedockerplugin/hpe_storage_api.py | 25 ++++++++++++++++------ hpedockerplugin/volume_manager.py | 7 +++--- test/createsnapshot_tester.py | 4 ++-- test/createvolume_tester.py | 4 ++-- 7 files changed, 36 insertions(+), 24 deletions(-) diff --git a/docs/active-passive-based-replication.md b/docs/active-passive-based-replication.md index 1b7ac3f5..c1fb6ba6 100644 --- a/docs/active-passive-based-replication.md +++ b/docs/active-passive-based-replication.md @@ -40,7 +40,7 @@ replication_device = backend_id:, 1. In case of asynchronous replication mode, *sync_period* field can optionally be defined as part of *replication_device* entry and it should be between range 300 and 31622400 seconds. If not defined, it defaults to 900 seconds. -2. Both *cpg_map* and *snap_cpg_map* in *replication_device* section are mandatory. +2. *cpg_map* and *snap_cpg_map* in *replication_device* section are mandatory. If *snap_cpg_map* is not mentioned then it will be same as *cpg_map* 3. If password is encrypted for primary array, it must be encrypted for secondary array as well using the same *pass-phrase* @@ -74,7 +74,7 @@ replication_device = backend_id:, ``` *Note*: -1. Both *cpg_map* and *snap_cpg_map* in *replication_device* section are mandatory. +1. *cpg_map* and *snap_cpg_map* in *replication_device* section are mandatory. If *snap_cpg_map* is not mentioned then it will be same as *cpg_map* 2. *hpe3par_iscsi_ips* MUST be defined upfront for both source and target arrays. 3. *hpe3par_iscsi_ips* can be a single ISCSI IP or a list of ISCSI IPs delimited by semi-colon. Delimiter for this field is applicable for *replication_device* section ONLY. diff --git a/docs/peer-persistence-based-replication.md b/docs/peer-persistence-based-replication.md index 65bd52dd..49db1864 100644 --- a/docs/peer-persistence-based-replication.md +++ b/docs/peer-persistence-based-replication.md @@ -53,7 +53,7 @@ replication_device = backend_id:, 1. *replication_mode* MUST be set to *synchronous* as a pre-requisite for Peer Persistence based replication. -2. Both *cpg_map* and *snap_cpg_map* in *replication_device* section are mandatory +2. *cpg_map* and *snap_cpg_map* in *replication_device* section are mandatory. If *snap_cpg_map* is not mentioned then it will be same as *cpg_map* 3. If password is encrypted for primary array, it must be encrypted for secondary array as well using the same *pass-phrase* @@ -87,7 +87,7 @@ replication_device = backend_id:, ``` *Note*: -1. Both *cpg_map* and *snap_cpg_map* in *replication_device* section are mandatory. +1. *cpg_map* and *snap_cpg_map* in *replication_device* section are mandatory. If *snap_cpg_map* is not mentioned then it will be same as *cpg_map* 2. *hpe3par_iscsi_ips* MUST be defined upfront for both source and target arrays. 3. *hpe3par_iscsi_ips* can be a single ISCSI IP or a list of ISCSI IPs delimited by semi-colon. Delimiter for this field is applicable for *replication_device* section ONLY. diff --git a/hpedockerplugin/hpe/hpe_3par_common.py b/hpedockerplugin/hpe/hpe_3par_common.py index 4d50d61b..faab093d 100644 --- a/hpedockerplugin/hpe/hpe_3par_common.py +++ b/hpedockerplugin/hpe/hpe_3par_common.py @@ -848,9 +848,9 @@ def create_volume(self, volume): extras['compression'] = compression else: err = (_("To create compression enabled volume, size of " - "the volume should be atleast 16GB. Fully " - "provisioned volume can not be compressed. " - "Please re enter requested volume size or " + "the volume should be at least 16GB. Fully " + "provisioned volume cannot be compressed. " + "Please re-enter requested volume size or " "provisioning type. ")) # LOG.error(err) raise exception.HPEDriverInvalidSizeForCompressedVolume( @@ -887,10 +887,10 @@ def create_volume(self, volume): msg = "For thin volume, 'provisioning' must be specified " \ "as 'thin'. And for deduplicated and compressed " \ "volume, 'provisioning' must be specified as 'dedup' " \ - "and 'compression' must be specified to true. " \ + "and 'compression' must be specified as true. " \ "If any of " \ - "these conditions for a given type of volume" \ - "is not met volume creation will fail" + "these conditions for a given type of volume " \ + "is not met volume creation will fail." raise exception.HPEDriverInvalidInput(reason=msg) raise exception.HPEDriverInvalidInput(reason=ex.get_description()) diff --git a/hpedockerplugin/hpe_storage_api.py b/hpedockerplugin/hpe_storage_api.py index 9c728a8f..c67b9323 100644 --- a/hpedockerplugin/hpe_storage_api.py +++ b/hpedockerplugin/hpe_storage_api.py @@ -357,24 +357,35 @@ def volumedriver_create(self, request, opts=None): if ('size' in contents['Opts'] and contents['Opts']['size'] != ""): vol_size = int(contents['Opts']['size']) + if vol_size == 0: + msg = ("Please enter the valid integer value for size \ + parameter") + LOG.error(msg) + return json.dumps({u'Err': six.text_type(msg)}) if ('provisioning' in contents['Opts'] and contents['Opts']['provisioning'] != ""): vol_prov = str(contents['Opts']['provisioning']) - if ('compression' in contents['Opts'] and - contents['Opts']['compression'] != ""): - compression_val = str(contents['Opts']['compression']) + if 'compression' in contents['Opts']: + compression_val = str(contents['Opts'].get('compression')) if compression_val is not None: if compression_val.lower() not in valid_bool_opts: msg = \ - _('create volume failed, error is:' + _('create volume failed, error is: ' 'passed compression parameter' - ' do not have a valid value. ' + ' does not have a valid value. ' 'Valid values are: %(valid)s') % { 'valid': valid_bool_opts} LOG.error(msg) return json.dumps({u'Err': six.text_type(msg)}) + else: + msg = \ + _('parameter compression passed without a value. ' + 'Valid values are: %(valid)s') % { + 'valid': valid_bool_opts} + LOG.error(msg) + return json.dumps({u'Err': six.text_type(msg)}) if ('flash-cache' in contents['Opts'] and contents['Opts']['flash-cache'] != ""): @@ -709,7 +720,7 @@ def volumedriver_create_snapshot(self, name, mount_conflict_delay, if exphrs is not None: if rethrs > exphrs: msg = ('create schedule failed, error is: ' - 'expiration hours cannot be greater than ' + 'expiration hours must be greater than ' 'retention hours') LOG.error(msg) response = json.dumps({'Err': msg}) @@ -746,7 +757,7 @@ def volumedriver_create_snapshot(self, name, mount_conflict_delay, schedNameLength = len(schedName) snapPrefixLength = len(snapPrefix) if schedNameLength > 31 or snapPrefixLength > 15: - msg = ('Please provide a schedlueName with max 31 ' + msg = ('Please provide a scheduleName with max 31 ' 'characters and snapshotPrefix with max ' 'length of 15 characters') LOG.error(msg) diff --git a/hpedockerplugin/volume_manager.py b/hpedockerplugin/volume_manager.py index 32a5ca0f..d9a68b2c 100644 --- a/hpedockerplugin/volume_manager.py +++ b/hpedockerplugin/volume_manager.py @@ -119,9 +119,10 @@ def _initialize_configuration(self): "Failed to initialize driver - cpg_map not defined for" "replication device") - self.tgt_bkend_config.hpe3par_snapcpg = \ - self._extract_remote_cpgs( - self.tgt_bkend_config.snap_cpg_map) + if self.tgt_bkend_config.snap_cpg_map: + self.tgt_bkend_config.hpe3par_snapcpg = \ + self._extract_remote_cpgs( + self.tgt_bkend_config.snap_cpg_map) if not self.tgt_bkend_config.hpe3par_snapcpg: self.tgt_bkend_config.hpe3par_snapcpg = \ self.tgt_bkend_config.hpe3par_cpg diff --git a/test/createsnapshot_tester.py b/test/createsnapshot_tester.py index 2580a5bd..88e22d23 100644 --- a/test/createsnapshot_tester.py +++ b/test/createsnapshot_tester.py @@ -229,7 +229,7 @@ def get_request_params(self): "retHrs": '2'}} def check_response(self, resp): - expected = 'Please provide a schedlueName with max 31 characters '\ + expected = 'Please provide a scheduleName with max 31 characters '\ 'and snapshotPrefix with max length of 15 characters' self._test_case.assertEqual(resp, {u"Err": expected}) @@ -283,7 +283,7 @@ def get_request_params(self): def check_response(self, resp): expected = 'create schedule failed, error is: expiration hours '\ - 'cannot be greater than retention hours' + 'must be greater than retention hours' self._test_case.assertEqual(resp, {u"Err": expected}) diff --git a/test/createvolume_tester.py b/test/createvolume_tester.py index 5d29e1fb..ce31ba08 100644 --- a/test/createvolume_tester.py +++ b/test/createvolume_tester.py @@ -427,8 +427,8 @@ class TestCreateCompressedVolumeNegativeSize(CreateVolumeUnitTest): def check_response(self, resp): expected_msg = 'Invalid input received: To create compression '\ 'enabled volume, size of the volume should be '\ - 'atleast 16GB. Fully provisioned volume can not be '\ - 'compressed. Please re enter requested volume size '\ + 'at least 16GB. Fully provisioned volume cannot be '\ + 'compressed. Please re-enter requested volume size '\ 'or provisioning type. ' self._test_case.assertEqual(resp, {u"Err": expected_msg})