openstack/rally
Code Issues Proposed changes

[Context] Fix side effect changes of context config

Browse Source 
Since context configuration passed to Context.__init__() is
a mutable type (dict or list), sometimes we have unexpected
changes done by unpredictable code (for example, in wrappers).

This patch uses types `common.utils.LockedDict' and `tuple'
for locking this data from further changes.

Also there was missing unit test for Context.__init__()
with DEFAULT_CONFIG set - this unit test is added.

Change-Id: Ic1f11281fcf95b35d9fbb3ee82cb1d67f1d8abd7
Closes-Bug: #1570328
This commit is contained in:
Alexander Maretskiy 2016-04-15 17:52:21 +03:00
parent 9ff6383ab7
commit 404f0fab68
11 changed files with 68 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
rally/plugins/openstack/context/network/networks.py
View File

@ -80,11 +80,12 @@ class Network(context.Context):
for i in range(self.config["networks_per_tenant"]):
# NOTE(amaretskiy): add_router and subnets_num take effect
# for Neutron only.
network_create_args = self.config["network_create_args"].copy()
network = net_wrapper.create_network(
tenant_id,
add_router=True,
subnets_num=self.config["subnets_per_network"],
network_create_args=self.config["network_create_args"])
network_create_args=network_create_args)
self.context["tenants"][tenant_id]["networks"].append(network)
@logging.log_task_wrapper(LOG.info, _("Exit context: `network`"))

20
rally/task/context.py
View File

@ -71,10 +71,22 @@ class Context(plugin.Plugin, functional.FunctionalMixin,
CONFIG_SCHEMA = {}
def __init__(self, ctx):
self.config = ctx.get("config", {}).get(self.get_name(), {})
if hasattr(self, "DEFAULT_CONFIG"):
for key, value in self.DEFAULT_CONFIG.items():
self.config.setdefault(key, value)
config = ctx.get("config", {}).get(self.get_name(), {})
# NOTE(amaretskiy): self.config is a constant data and must be
# immutable or write-protected type to prevent
# unexpected changes in runtime
if type(config) == dict:
if hasattr(self, "DEFAULT_CONFIG"):
for key, value in self.DEFAULT_CONFIG.items():
config.setdefault(key, value)
self.config = utils.LockedDict(config)
elif type(config) == list:
self.config = tuple(config)
else:
# NOTE(amaretskiy): It is improbable that config can be a None,
# number, boolean or even string,
# however we handle this
self.config = config
self.context = ctx
self.task = self.context.get("task", {})

14
tests/unit/plugins/openstack/context/ceilometer/test_samples.py
View File

@ -57,14 +57,14 @@ class CeilometerSampleGeneratorTestCase(test.TestCase):
"resources_per_tenant": resources_per_tenant,
"samples_per_resource": samples_per_resource,
"timestamp_interval": 60,
"metadata_list": [
"metadata_list": (
{"status": "active", "name": "fake_resource",
"deleted": "False",
"created_at": "2015-09-04T12:34:19.000000"},
{"status": "not_active", "name": "fake_resource_1",
"deleted": "False",
"created_at": "2015-09-10T06:55:12.000000"}
]
"created_at": "2015-09-10T06:55:12.000000"},
)
}
},
"admin": {
@ -86,15 +86,15 @@ class CeilometerSampleGeneratorTestCase(test.TestCase):
"counter_volume": 1.0,
"resources_per_tenant": 5,
"samples_per_resource": 5,
"timestamp_intervals": 60,
"metadata_list": [
"timestamp_interval": 60,
"metadata_list": (
{"status": "active", "name": "fake_resource",
"deleted": "False",
"created_at": "2015-09-04T12:34:19.000000"},
{"status": "not_active", "name": "fake_resource_1",
"deleted": "False",
"created_at": "2015-09-10T06:55:12.000000"}
]
"created_at": "2015-09-10T06:55:12.000000"},
)
}
}

4
tests/unit/plugins/openstack/context/cleanup/test_admin.py
View File

@ -61,7 +61,7 @@ class AdminCleanupTestCase(test.TestCase):
admin_cleanup.setup()
admin_cleanup.cleanup()
mock_find_resource_managers.assert_called_once_with(["a", "b"], True)
mock_find_resource_managers.assert_called_once_with(("a", "b"), True)
mock_seek_and_destroy.assert_has_calls([
mock.call(
mock_find_resource_managers.return_value[0],
@ -104,7 +104,7 @@ class AdminCleanupTestCase(test.TestCase):
admin_cleanup.setup()
admin_cleanup.cleanup()
mock_find_resource_managers.assert_called_once_with(["a", "b"], True)
mock_find_resource_managers.assert_called_once_with(("a", "b"), True)
mock_seek_and_destroy.assert_has_calls([
mock.call(
mock_find_resource_managers.return_value[0],

2
tests/unit/plugins/openstack/context/cleanup/test_user.py
View File

@ -60,7 +60,7 @@ class UserCleanupTestCase(test.TestCase):
admin_cleanup.setup()
admin_cleanup.cleanup()
mock_find_resource_managers.assert_called_once_with(["a", "b"], False)
mock_find_resource_managers.assert_called_once_with(("a", "b"), False)
mock_seek_and_destroy.assert_has_calls([
mock.call(

4
tests/unit/plugins/openstack/context/keystone/test_users.py
View File

@ -223,8 +223,8 @@ class UserGeneratorTestCase(test.ScenarioTestCase):
@mock.patch("%s.keystone" % CTX)
def test__create_tenants(self, mock_keystone):
self.context["config"]["users"]["tenants"] = 1
user_generator = users.UserGenerator(self.context)
user_generator.config["tenants"] = 1
tenants = user_generator._create_tenants()
self.assertEqual(1, len(tenants))
id, tenant = tenants.popitem()
@ -232,10 +232,10 @@ class UserGeneratorTestCase(test.ScenarioTestCase):
@mock.patch("%s.keystone" % CTX)
def test__create_users(self, mock_keystone):
self.context["config"]["users"]["users_per_tenant"] = 2
user_generator = users.UserGenerator(self.context)
user_generator.context["tenants"] = {"t1": {"id": "t1", "name": "t1"},
"t2": {"id": "t2", "name": "t2"}}
user_generator.config["users_per_tenant"] = 2
users_ = user_generator._create_users()
self.assertEqual(4, len(users_))
for user in users_:

4
tests/unit/plugins/openstack/context/manila/test_manila_share_networks.py
View File

@ -70,7 +70,7 @@ class ManilaSampleGeneratorTestCase(test.TestCase):
inst = manila_share_networks.ManilaShareNetworks(context)
self.assertEqual(
context["config"][consts.SHARE_NETWORKS_CONTEXT_NAME],
{"foo": "bar", "share_networks": {}, "use_share_networks": False},
inst.config)
self.assertIn(
rally_consts.JSON_SCHEMA, inst.CONFIG_SCHEMA.get("$schema"))
@ -107,11 +107,11 @@ class ManilaSampleGeneratorTestCase(test.TestCase):
def test_setup_use_existing_share_networks(
self, mock_manila_scenario__list_share_networks, mock_clients):
existing_sns = self.existing_sns
expected_ctxt = copy.deepcopy(self.ctxt_use_existing)
inst = manila_share_networks.ManilaShareNetworks(
self.ctxt_use_existing)
mock_manila_scenario__list_share_networks.return_value = (
self.existing_sns)
expected_ctxt = copy.deepcopy(self.ctxt_use_existing)
expected_ctxt.update({
"delete_share_networks": False,
"tenants": {

2
tests/unit/plugins/openstack/context/neutron/test_existing_network.py
View File

@ -25,7 +25,7 @@ class ExistingNetworkTestCase(test.TestCase):
def setUp(self):
super(ExistingNetworkTestCase, self).setUp()
self.config = mock.MagicMock()
self.config = {"foo": "bar"}
self.context = test.get_test_context()
self.context.update({
"users": [

3
tests/unit/plugins/openstack/context/nova/test_servers.py
View File

@ -46,7 +46,8 @@ class ServerGeneratorTestCase(test.ScenarioTestCase):
"tenants": self._gen_tenants(tenants_count)})
inst = servers.ServerGenerator(self.context)
self.assertEqual(self.context["config"]["servers"], inst.config)
self.assertEqual({"auto_assign_nic": False, "servers_per_tenant": 5},
inst.config)
@mock.patch("%s.nova.utils.NovaScenario._boot_servers" % SCN,
return_value=[

13
tests/unit/plugins/openstack/context/test_api_versions.py
View File

@ -10,8 +10,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import copy
import jsonschema
import mock
@ -112,16 +110,17 @@ class OpenStackServicesTestCase(test.TestCase):
def test_setup_with_service_name(self):
self.mock_kc.services.list.return_value = [
utils.Struct(type="computev21", name="NovaV21")]
name = api_versions.OpenStackAPIVersions.get_name()
context = {
"config": {api_versions.OpenStackAPIVersions.get_name(): {
"nova": {"service_name": "NovaV21"}}},
"config": {name: {"nova": {"service_name": "NovaV21"}}},
"admin": {"credential": mock.MagicMock()},
"users": [{"credential": mock.MagicMock()}]}
ctx = api_versions.OpenStackAPIVersions(copy.deepcopy(context))
ctx = api_versions.OpenStackAPIVersions(context)
ctx.setup()
self.mock_kc.service_catalog.get_endpoints.assert_called_once_with()
self.mock_kc.services.list.assert_called_once_with()
self.assertEqual("computev21", ctx.config["nova"]["service_type"])
self.assertEqual(
"computev21",
ctx.context["config"]["api_versions"]["nova"]["service_type"])

41
tests/unit/task/test_context.py
View File

@ -13,7 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import ddt
import jsonschema
import mock
@ -23,26 +23,39 @@ from tests.unit import fakes
from tests.unit import test
@ddt.ddt
class BaseContextTestCase(test.TestCase):
def test_init(self):
ctx0 = {
"config": {
"a": 1,
"fake": mock.MagicMock()
},
"task": mock.MagicMock()
}
@ddt.data({"config": {"bar": "spam"}, "expected": {"bar": "spam"}},
{"config": {"bar": "spam"}, "expected": {"bar": "spam"}},
{"config": {}, "expected": {}},
{"config": None, "expected": None},
{"config": 42, "expected": 42},
{"config": "foo str", "expected": "foo str"},
{"config": [], "expected": ()},
{"config": [11, 22, 33], "expected": (11, 22, 33)})
@ddt.unpack
def test_init(self, config, expected):
ctx = {"config": {"foo": 42, "fake": config}, "task": "foo_task"}
ins = fakes.FakeContext(ctx)
self.assertEqual(ins.config, expected)
self.assertEqual(ins.task, "foo_task")
self.assertEqual(ins.context, ctx)
ctx = fakes.FakeContext(ctx0)
self.assertEqual(ctx.config, ctx0["config"]["fake"])
self.assertEqual(ctx.task, ctx0["task"])
self.assertEqual(ctx.context, ctx0)
def test_init_with_default_config(self):
@context.configure(name="foo", order=1)
class FooContext(fakes.FakeContext):
DEFAULT_CONFIG = {"alpha": "beta", "delta": "gamma"}
ctx = {"config": {"foo": {"ab": "cd"}, "bar": 42}, "task": "foo_task"}
ins = FooContext(ctx)
self.assertEqual({"ab": "cd", "alpha": "beta", "delta": "gamma"},
ins.config)
def test_init_empty_context(self):
ctx0 = {
"task": mock.MagicMock(),
"config": {"fake": "test"}
"config": {"fake": {"foo": 42}}
}
ctx = fakes.FakeContext(ctx0)
self.assertEqual(ctx.config, ctx0["config"]["fake"])