From 13f2dd2e9a5ab50ce2f2561d88f225f59eb8e213 Mon Sep 17 00:00:00 2001 From: lkuchlan Date: Wed, 17 Feb 2021 14:48:36 +0200 Subject: [PATCH] Use local variable instead of instance variable "create_share" method defines an instance variable self.share that is not really needed which causes confusion in reading the code. Change-Id: Iec00424cfee7efd262989a49df1f7e7ebef83bed --- .../tests/scenario/manager_share.py | 16 ++--- .../tests/scenario/test_share_basic_ops.py | 66 ++++++++++--------- .../tests/scenario/test_share_extend.py | 4 +- .../scenario/test_share_manage_unmanage.py | 2 +- .../tests/scenario/test_share_shrink.py | 4 +- 5 files changed, 48 insertions(+), 44 deletions(-) diff --git a/manila_tempest_tests/tests/scenario/manager_share.py b/manila_tempest_tests/tests/scenario/manager_share.py index 2e405694..d68ab04d 100644 --- a/manila_tempest_tests/tests/scenario/manager_share.py +++ b/manila_tempest_tests/tests/scenario/manager_share.py @@ -198,7 +198,6 @@ class ShareScenarioTest(manager.NetworkScenarioTest): # NOTE(u_glide): Workaround for bug #1465682 remote_client = remote_client.ssh_client - self.share = self.shares_client.get_share(self.share['id'])['share'] return remote_client def validate_ping_to_export_location(self, export, remote_client, @@ -278,8 +277,8 @@ class ShareScenarioTest(manager.NetworkScenarioTest): kwargs.update({'share_type_id': default_share_type_id}) if CONF.share.multitenancy_enabled: kwargs.update({'share_network_id': self.share_network['id']}) - self.share = self._create_share(**kwargs) - return self.share + share = self._create_share(**kwargs) + return share def get_remote_client(self, *args, **kwargs): if not CONF.share.image_with_share_tools: @@ -357,7 +356,6 @@ class ShareScenarioTest(manager.NetworkScenarioTest): snapshot=None, access_level='rw', client=None): - share = share or self.share client = client or self.shares_v2_client if not CONF.share.multitenancy_enabled: if self.ipv6_enabled and not self.storage_network: @@ -406,7 +404,6 @@ class ShareScenarioTest(manager.NetworkScenarioTest): """ client = client or self.shares_v2_client if not access_rule: - share = share or self.share access_to = access_to or data_utils.rand_name( self.__class__.__name__ + '-cephx-id') # Check if access is already granted to the client @@ -746,9 +743,10 @@ class BaseShareScenarioNFSTest(ShareScenarioTest): def allow_access(self, access_level='rw', **kwargs): snapshot = kwargs.get('snapshot') + share = kwargs.get('share') return self._provide_access_to_client_identified_by_ip( - instance=kwargs['instance'], access_level=access_level, - snapshot=snapshot) + share=share, instance=kwargs['instance'], + access_level=access_level, snapshot=snapshot) def mount_share(self, location, ssh_client, target_dir=None): @@ -772,9 +770,11 @@ class BaseShareScenarioCIFSTest(ShareScenarioTest): raise cls.skipException(message) def allow_access(self, access_level='rw', **kwargs): + share = kwargs.get('share') snapshot = kwargs.get('snapshot') return self._provide_access_to_client_identified_by_ip( instance=kwargs['instance'], + share=share, snapshot=snapshot, access_level=access_level) @@ -794,7 +794,7 @@ class BaseShareScenarioCEPHFSTest(ShareScenarioTest): def allow_access(self, access_level='rw', access_rule=None, **kwargs): return self._provide_access_to_client_identified_by_cephx( - remote_client=kwargs['remote_client'], + share=kwargs['share'], remote_client=kwargs['remote_client'], locations=kwargs['locations'], access_level=access_level, access_rule=access_rule) diff --git a/manila_tempest_tests/tests/scenario/test_share_basic_ops.py b/manila_tempest_tests/tests/scenario/test_share_basic_ops.py index e440cb37..d8329744 100644 --- a/manila_tempest_tests/tests/scenario/test_share_basic_ops.py +++ b/manila_tempest_tests/tests/scenario/test_share_basic_ops.py @@ -49,12 +49,12 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND) def test_mount_share_one_vm(self): instance = self.boot_instance(wait_until="BUILD") - self.create_share() - locations = self.get_user_export_locations(self.share) + share = self.create_share() + locations = self.get_user_export_locations(share) instance = self.wait_for_active_instance(instance["id"]) remote_client = self.init_remote_client(instance) - self.allow_access(instance=instance, remote_client=remote_client, - locations=locations) + self.allow_access(share=share, instance=instance, + remote_client=remote_client, locations=locations) for location in locations: self.mount_share(location, remote_client) @@ -67,23 +67,24 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): test_data = "Some test data to write" instance = self.boot_instance(wait_until="BUILD") - self.create_share() - location = self.get_user_export_locations(self.share)[0] + share = self.create_share() + location = self.get_user_export_locations(share)[0] instance = self.wait_for_active_instance(instance["id"]) remote_client_inst = self.init_remote_client(instance) # First, check if write works RW access. acc_rule_id = self.allow_access( - instance=instance, remote_client=remote_client_inst, + share=share, instance=instance, remote_client=remote_client_inst, locations=location)['id'] self.mount_share(location, remote_client_inst) self.write_data_to_mounted_share(test_data, remote_client_inst) - self.deny_access(self.share['id'], acc_rule_id) + self.deny_access(share['id'], acc_rule_id) - self.allow_access(instance=instance, remote_client=remote_client_inst, - locations=location, access_level='ro') + self.allow_access(share=share, instance=instance, + remote_client=remote_client_inst, locations=location, + access_level='ro') self.addCleanup(self.unmount_share, remote_client_inst) @@ -101,14 +102,14 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): # Boot two VMs and create share instance1 = self.boot_instance(wait_until="BUILD") instance2 = self.boot_instance(wait_until="BUILD") - self.create_share() - location = self.get_user_export_locations(self.share)[0] + share = self.create_share() + location = self.get_user_export_locations(share)[0] instance1 = self.wait_for_active_instance(instance1["id"]) instance2 = self.wait_for_active_instance(instance2["id"]) # Write data to first VM remote_client_inst1 = self.init_remote_client(instance1) - access = self.allow_access(instance=instance1, + access = self.allow_access(share=share, instance=instance1, remote_client=remote_client_inst1, locations=location) @@ -120,7 +121,7 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): # Read from second VM remote_client_inst2 = self.init_remote_client(instance2) if not CONF.share.override_ip_for_nfs_access or self.ipv6_enabled: - self.allow_access(instance=instance2, + self.allow_access(share=share, instance=instance2, remote_client=remote_client_inst2, locations=location, access_rule=access) @@ -164,17 +165,15 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): "needed to run share migration tests.") instance = self.boot_instance(wait_until="BUILD") - self.create_share() - export_location = self.get_user_export_locations(self.share)[0] + share = self.create_share() + export_location = self.get_user_export_locations(share)[0] instance = self.wait_for_active_instance(instance["id"]) - self.share = self.shares_admin_v2_client.get_share( - self.share['id'])['share'] + share = self.shares_admin_v2_client.get_share(share['id'])['share'] default_type = self.shares_v2_client.list_share_types( default=True)['share_type'] - dest_pool = utils.choose_matching_backend( - self.share, pools, default_type) + dest_pool = utils.choose_matching_backend(share, pools, default_type) self.assertIsNotNone(dest_pool) self.assertIsNotNone(dest_pool.get('name')) @@ -183,7 +182,8 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): remote_client = self.init_remote_client(instance) - self.allow_access(instance=instance, + self.allow_access(share=share, + instance=instance, remote_client=remote_client, locations=export_location) @@ -212,8 +212,8 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): if force_host_assisted else constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE) - self.share = self.migrate_share( - self.share['id'], dest_pool, task_state, force_host_assisted) + share = self.migrate_share( + share['id'], dest_pool, task_state, force_host_assisted) if force_host_assisted: self.assertRaises( @@ -223,13 +223,13 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): self.unmount_share(remote_client) - self.share = self.migration_complete(self.share['id'], dest_pool) + share = self.migration_complete(share['id'], dest_pool) - new_exports = self.get_user_export_locations(self.share) + new_exports = self.get_user_export_locations(share) - self.assertEqual(dest_pool, self.share['host']) + self.assertEqual(dest_pool, share['host']) self.assertEqual(constants.TASK_STATE_MIGRATION_SUCCESS, - self.share['task_state']) + share['task_state']) self.mount_share(new_exports[0], remote_client) @@ -266,7 +266,8 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): remote_client = self.init_remote_client(instance) # 4 - Provide RW access to S1, ok, provided - self.allow_access(instance=instance, + self.allow_access(share=parent_share, + instance=instance, remote_client=remote_client, locations=parent_share_export_location) @@ -307,7 +308,8 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): ) # 11 - Provide RW access to S2, ok, provided - self.allow_access(instance=instance, + self.allow_access(share=child_share, + instance=instance, remote_client=remote_client, locations=child_share_export_location) @@ -364,7 +366,8 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): remote_client = self.init_remote_client(instance) # 4 - Provide RW access to S1, ok, provided - self.allow_access(instance=instance, + self.allow_access(share=parent_share, + instance=instance, remote_client=remote_client, locations=user_export_location) @@ -390,7 +393,8 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): remote_client.exec_command("sudo touch %s/file2" % parent_share_dir) # 9 - Allow access to SS1 - self.allow_access(instance=instance, + self.allow_access(share=parent_share, + instance=instance, snapshot=snapshot, remote_client=remote_client, locations=snapshot_export_location) diff --git a/manila_tempest_tests/tests/scenario/test_share_extend.py b/manila_tempest_tests/tests/scenario/test_share_extend.py index 4b74f780..f4dce61e 100644 --- a/manila_tempest_tests/tests/scenario/test_share_extend.py +++ b/manila_tempest_tests/tests/scenario/test_share_extend.py @@ -65,8 +65,8 @@ class ShareExtendBase(manager.ShareScenarioTest): LOG.debug('Step 4 - grant access') location = self.get_user_export_locations(share)[0] - self.allow_access(instance=instance, remote_client=remote_client, - locations=location) + self.allow_access(share=share, instance=instance, + remote_client=remote_client, locations=location) LOG.debug('Step 5 - mount') self.mount_share(location, remote_client) diff --git a/manila_tempest_tests/tests/scenario/test_share_manage_unmanage.py b/manila_tempest_tests/tests/scenario/test_share_manage_unmanage.py index 7f4833d0..ef2a6afa 100644 --- a/manila_tempest_tests/tests/scenario/test_share_manage_unmanage.py +++ b/manila_tempest_tests/tests/scenario/test_share_manage_unmanage.py @@ -123,7 +123,7 @@ class ShareManageUnmanageBase(manager.ShareScenarioTest): self.assertRaises( exceptions.NotFound, self.shares_admin_v2_client.get_share, - self.share['id']) + share['id']) LOG.debug('Step 10 - manage share') share_type = self.get_share_type() diff --git a/manila_tempest_tests/tests/scenario/test_share_shrink.py b/manila_tempest_tests/tests/scenario/test_share_shrink.py index 2b68d8aa..b4bfcd5a 100644 --- a/manila_tempest_tests/tests/scenario/test_share_shrink.py +++ b/manila_tempest_tests/tests/scenario/test_share_shrink.py @@ -69,8 +69,8 @@ class ShareShrinkBase(manager.ShareScenarioTest): LOG.debug('Step 4 - grant access') location = self.get_user_export_locations(share)[0] - self.allow_access(instance=instance, remote_client=remote_client, - locations=location) + self.allow_access(share=share, instance=instance, + remote_client=remote_client, locations=location) LOG.debug('Step 5 - mount') self.mount_share(location, remote_client)