openstack
/
sahara
Code Issues Proposed changes

Add "null" for fields in cluster and node group template JSON schemas 

* Change 'type' for optional fields to list including 'null'

* Allow the default template tool to use "None" to
unset fields with replacement values when no replacement has been
specified.

* Fix cluster template update code to check for explicit setting of
the 'node_groups' field in a cluster template update

Partial-Bug: #1438077
Closes-Bug: #1439839

Change-Id: Ia180867c8e3ffb502da40f169a49dd3a54c23072
This commit is contained in:
Trevor McKay 2015-05-08 13:26:24 -04:00
parent 4c048161e0
commit 0ba7ece84f
10 changed files with 234 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
sahara/conductor/manager.py
View File

@ -261,7 +261,8 @@ class ConductorManager(db_base.Base):
values['tenant_id'] = context.tenant_id
values['id'] = id
values['node_groups'] = self._populate_node_groups(context, values)
if 'node_groups' in values:
values['node_groups'] = self._populate_node_groups(context, values)
return self.db.cluster_template_update(context, values, ignore_default)

8
sahara/db/sqlalchemy/api.py
View File

@ -441,7 +441,11 @@ def cluster_template_destroy(context, cluster_template_id,
def cluster_template_update(context, values, ignore_default=False):
node_groups = values.pop("node_groups", [])
explicit_node_groups = "node_groups" in values
if explicit_node_groups:
node_groups = values.pop("node_groups")
if node_groups is None:
node_groups = []
session = get_session()
cluster_template_id = values['id']
@ -475,7 +479,7 @@ def cluster_template_update(context, values, ignore_default=False):
# If node_groups has not been specified, then we are
# keeping the old ones so don't delete!
if node_groups:
if explicit_node_groups:
model_query(m.TemplatesRelation,
context, session=session).filter_by(
cluster_template_id=cluster_template_id).delete()

8
sahara/db/templates/api.py
View File

@ -261,13 +261,7 @@ def substitute_config_values(configs, template, path):
for opt, value in six.iteritems(configs):
if opt in opt_names and opt in template:
if value is None:
# TODO(tmckay): someday if we support 'null' in JSON
# we should replace this value with None. json.load
# will replace 'null' with None, and sqlalchemy will
# accept None as a value for a nullable field.
del template[opt]
LOG.debug("No replacement value specified for {opt} in "
"{path}, removing".format(opt=opt, path=path))
template[opt] = None
else:
# Use args to allow for keyword arguments to format
args = {opt: value}

12
sahara/service/validations/cluster_template_schema.py
View File

@ -63,30 +63,30 @@ CLUSTER_TEMPLATE_SCHEMA = {
"type": "string",
},
"default_image_id": {
"type": "string",
"type": ["string", "null"],
"format": "uuid",
},
"cluster_configs": {
"type": "configs",
"type": ["configs", "null"],
},
"node_groups": {
"type": "array",
"type": ["array", "null"],
"items": {
"oneOf": [_cluster_tmpl_ng_tmpl_schema,
_cluster_tmpl_ng_schema]
}
},
"anti_affinity": {
"type": "array",
"type": ["array", "null"],
"items": {
"type": "string",
},
},
"description": {
"type": "string",
"type": ["string", "null"],
},
"neutron_management_network": {
"type": "string",
"type": ["string", "null"],
"format": "uuid"
},
},

30
sahara/service/validations/node_group_template_schema.py
View File

@ -41,53 +41,51 @@ NODE_GROUP_TEMPLATE_SCHEMA = {
"minItems": 1
},
"image_id": {
"type": "string",
"type": ["string", "null"],
"format": "uuid",
},
"node_configs": {
"type": "configs",
"type": ["configs", "null"],
},
"volumes_per_node": {
"type": "integer",
"minimum": 0,
},
"volumes_size": {
"type": "integer",
"type": ["integer", "null"],
"minimum": 1,
},
"volume_type": {
"type": "string"
"type": ["string", "null"],
},
"volumes_availability_zone": {
"type": "string",
"type": ["string", "null"],
},
"volume_mount_prefix": {
"type": "string",
"type": ["string", "null"],
"format": "posix_path",
},
"description": {
"type": "string",
"type": ["string", "null"],
},
"floating_ip_pool": {
"type": "string",
"type": ["string", "null"],
},
"security_groups": {
"type": "array",
"items": {
"type": "string",
},
"type": ["array", "null"],
"items": {"type": "string"}
},
"auto_security_group": {
"type": "boolean"
"type": ["boolean", "null"],
},
"availability_zone": {
"type": "string",
"type": ["string", "null"],
},
"is_proxy_gateway": {
"type": "boolean"
"type": ["boolean", "null"],
},
"volume_local_to_instance": {
"type": "boolean"
"type": ["boolean", "null"]
},
},
"additionalProperties": False,

88
sahara/tests/unit/conductor/manager/test_templates.py
View File

@ -14,26 +14,28 @@
# limitations under the License.
import copy
import uuid
import six
from sqlalchemy import exc as sa_ex
import testtools
from sahara.conductor import manager
from sahara import context
from sahara import exceptions as ex
from sahara.service.validations import cluster_template_schema as cl_schema
from sahara.service.validations import node_group_template_schema as ngt_schema
import sahara.tests.unit.conductor.base as test_base
import sahara.tests.unit.conductor.manager.test_clusters as cluster_tests
SAMPLE_NGT = {
"plugin_name": "test_plugin",
"flavor_id": "42",
"tenant_id": "tenant_1",
"hadoop_version": "test_version",
"name": "ngt_test",
"flavor_id": "42",
"plugin_name": "test_plugin",
"hadoop_version": "test_version",
"node_processes": ["p1", "p2"],
"floating_ip_pool": None,
"availability_zone": None,
"image_id": str(uuid.uuid4()),
"node_configs": {
"service_1": {
"config_1": "value_1"
@ -41,14 +43,26 @@ SAMPLE_NGT = {
"service_2": {
"config_1": "value_1"
}
}
},
"volumes_per_node": 1,
"volumes_size": 1,
"volume_type": "big",
"volumes_availability_zone": "here",
"volume_mount_prefix": "/tmp",
"description": "my template",
"floating_ip_pool": "public",
"security_groups": ["cat", "dog"],
"auto_security_group": False,
"availability_zone": "here",
"is_proxy_gateway": False,
"volume_local_to_instance": False
}
SAMPLE_CLT = {
"plugin_name": "test_plugin",
"tenant_id": "tenant_1",
"hadoop_version": "test_version",
"name": "clt_test",
"plugin_name": "test_plugin",
"hadoop_version": "test_version",
"default_image_id": str(uuid.uuid4()),
"cluster_configs": {
"service_1": {
"config_1": "value_1"
@ -77,7 +91,10 @@ SAMPLE_CLT = {
"availability_zone": None,
}
]
],
"anti_affinity": ["datanode"],
"description": "my template",
"neutron_management_network": str(uuid.uuid4())
}
@ -213,6 +230,28 @@ class NodeGroupTemplates(test_base.ConductorManagerTestCase):
ignore_default=True)
self.assertEqual(UPDATE_NAME, updated_ngt["name"])
def test_ngt_update_with_nulls(self):
ctx = context.ctx()
ngt = self.api.node_group_template_create(ctx, SAMPLE_NGT)
ngt_id = ngt["id"]
updated_values = copy.deepcopy(SAMPLE_NGT)
for prop, value in six.iteritems(
ngt_schema.NODE_GROUP_TEMPLATE_SCHEMA["properties"]):
if type(value["type"]) is list and "null" in value["type"]:
updated_values[prop] = None
# Prove that we can call update on these fields with null values
# without an exception
self.api.node_group_template_update(ctx,
ngt_id,
updated_values)
updated_ngt = self.api.node_group_template_get(ctx, ngt_id)
for prop, value in six.iteritems(updated_values):
if value is None:
self.assertIsNone(updated_ngt[prop])
class ClusterTemplates(test_base.ConductorManagerTestCase):
def __init__(self, *args, **kwargs):
@ -383,3 +422,30 @@ class ClusterTemplates(test_base.ConductorManagerTestCase):
update_values,
ignore_default=True)
self.assertEqual(UPDATE_NAME, updated_clt["name"])
def test_clt_update_with_nulls(self):
ctx = context.ctx()
clt = self.api.cluster_template_create(ctx, SAMPLE_CLT)
clt_id = clt["id"]
updated_values = copy.deepcopy(SAMPLE_CLT)
for prop, value in six.iteritems(
cl_schema.CLUSTER_TEMPLATE_SCHEMA["properties"]):
if type(value["type"]) is list and "null" in value["type"]:
updated_values[prop] = None
# Prove that we can call update on these fields with null values
# without an exception
self.api.cluster_template_update(ctx,
clt_id,
updated_values)
updated_clt = self.api.cluster_template_get(ctx, clt_id)
for prop, value in six.iteritems(updated_values):
if value is None:
# Conductor populates node groups with [] when
# the value given is null
if prop == "node_groups":
self.assertEqual([], updated_clt[prop])
else:
self.assertIsNone(updated_clt[prop])

4
sahara/tests/unit/db/templates/test_update.py
View File

@ -204,7 +204,7 @@ class TemplateUpdateTestCase(base.ConductorManagerTestCase):
"floating_ip_pool": None}
template_api.substitute_config_values(configs, ngt, "/path")
self.assertEqual("2", ngt["flavor_id"])
self.assertNotIn("floating_ip_pool", ngt)
self.assertIsNone(ngt["floating_ip_pool"])
def test_substitute_config_values_clt(self):
clt = copy.copy(c.SAMPLE_CLT)
@ -216,7 +216,7 @@ class TemplateUpdateTestCase(base.ConductorManagerTestCase):
"default_image_id": None}
template_api.substitute_config_values(configs, clt, "/path")
self.assertEqual(netid, clt["neutron_management_network"])
self.assertNotIn("default_image_id", clt)
self.assertIsNone(clt["default_image_id"])
def _write_files(self, tempdir, templates):
files = []

48
sahara/tests/unit/service/validation/test_cluster_template_create_validation.py
View File

@ -13,6 +13,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import uuid
import mock
from sahara.service import api
from sahara.service.validations import cluster_template_schema as ct_schema
from sahara.service.validations import cluster_templates as ct
@ -194,12 +198,52 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase):
u"'hadoop_version' is a required property")
)
def test_cluster_template_create_v_right(self):
@mock.patch("sahara.service.validations.base.check_all_configurations")
@mock.patch("sahara.service.validations.base.check_network_exists")
@mock.patch("sahara.service.validations.base.check_required_image_tags")
@mock.patch("sahara.service.validations.base.check_image_registered")
def test_cluster_template_create_v_right(self, image_reg, image_tags,
net_exists, all_configs):
self._assert_create_object_validation(
data={
'name': 'testname',
'plugin_name': 'vanilla',
'hadoop_version': '1.2.1'
'hadoop_version': '1.2.1',
'default_image_id': str(uuid.uuid4()),
'cluster_configs': {
"service_1": {
"config_1": "value_1"
}
},
'node_groups': [
{
"node_group_template_id": "550e8400-e29b-41d4-a716-"
"446655440000",
"name": "charlie",
'count': 3
}
],
'anti_affinity': ['datanode'],
'description': 'my template',
'neutron_management_network': str(uuid.uuid4())
})
@mock.patch("sahara.service.validations.base.check_network_exists")
@mock.patch("sahara.service.validations.base.check_required_image_tags")
@mock.patch("sahara.service.validations.base.check_image_registered")
def test_cluster_template_create_v_right_nulls(self, image_reg, image_tags,
net_exists):
self._assert_create_object_validation(
data={
'name': 'testname',
'plugin_name': 'vanilla',
'hadoop_version': '1.2.1',
'default_image_id': None,
'cluster_configs': None,
'node_groups': None,
'anti_affinity': None,
'description': None,
'neutron_management_network': None
})
def test_cluster_template_create_v_plugin_name_exists(self):

54
sahara/tests/unit/service/validation/test_ng_template_validation_create.py
View File

@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import mock
from sahara.service import api
from sahara.service.validations import node_group_template_schema as ngt_schema
from sahara.service.validations import node_group_templates as nt
@ -106,7 +108,14 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
"['wrong_process']")
)
def test_ng_template_create_v_right(self):
@mock.patch(
"sahara.service.validations.base.check_volume_availability_zone_exist")
@mock.patch("sahara.service.validations.base.check_volume_type_exists")
@mock.patch(
"sahara.service.validations.base.check_availability_zone_exist")
@mock.patch("sahara.service.validations.base.check_security_groups_exist")
def test_ng_template_create_v_right(self,
s_groups, a_zone, v_type, v_a_zone):
self._assert_create_object_validation(
data={
'name': 'a',
@ -118,16 +127,53 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase):
'secondarynamenode',
'tasktracker',
'jobtracker'],
'image_id': '550e8400-e29b-41d4-a716-446655440000',
'node_configs': {
'HDFS': {
u'hadoop.tmp.dir': '/temp/'
}
},
'image_id': '550e8400-e29b-41d4-a716-446655440000',
'volumes_per_node': 2,
'volumes_size': 10,
'description': 'test node template',
'floating_ip_pool': 'd9a3bebc-f788-4b81-9a93-aa048022c1ca'
'volume_type': 'fish',
'volumes_availability_zone': 'ocean',
'volume_mount_prefix': '/tmp',
'description': "my node group",
'floating_ip_pool': 'd9a3bebc-f788-4b81-9a93-aa048022c1ca',
'security_groups': ['cat', 'dog'],
'auto_security_group': False,
'availability_zone': 'here',
'is_proxy_gateway': False,
'volume_local_to_instance': False
}
)
def test_ng_template_create_v_nulls(self):
self._assert_create_object_validation(
data={
'name': 'a',
'flavor_id': '42',
'plugin_name': 'vanilla',
'hadoop_version': '1.2.1',
'node_processes': ['namenode',
'datanode',
'secondarynamenode',
'tasktracker',
'jobtracker'],
'image_id': None,
'node_configs': None,
'volumes_size': None,
'volume_type': None,
'volumes_availability_zone': None,
'volume_mount_prefix': None,
'description': None,
'floating_ip_pool': None,
'security_groups': None,
'auto_security_group': None,
'availability_zone': None,
'is_proxy_gateway': None,
'volume_local_to_instance': None
}
)

51
sahara/tests/unit/service/validation/utils.py
View File

@ -30,10 +30,11 @@ from sahara.tests.unit import testutils as tu
m = {}
_types_checks = {
"string": [1, (), {}],
"integer": ["a", (), {}],
"uuid": ["z550e8400-e29b-41d4-a716-446655440000", 1, "a", (), {}],
"array": [{}, 'a', 1]
"string": [1, (), {}, True],
"integer": ["a", (), {}, True],
"uuid": ["z550e8400-e29b-41d4-a716-446655440000", 1, "a", (), {}, True],
"array": [{}, 'a', 1, True],
"boolean": [1, 'a', (), {}]
}
@ -332,26 +333,34 @@ class ValidationTestCase(base.SaharaTestCase):
u"'a-!' is not a 'valid_name_hostname'")
)
def _prop_types_str(self, prop_types):
return ", ".join(["'%s'" % prop for prop in prop_types])
def _assert_types(self, default_data):
for p_name in self.scheme['properties']:
prop = self.scheme['properties'][p_name]
if prop["type"] in _types_checks:
for type_ex in _types_checks[prop["type"]]:
data = default_data.copy()
value = type_ex
value_str = str(value)
if isinstance(value, str):
value_str = "'%s'" % value_str
data.update({p_name: value})
message = ("%s is not of type '%s'" %
(value_str, prop["type"]))
if "enum" in prop:
message = [message, "%s is not one of %s" %
(value_str, prop["enum"])]
self._assert_create_object_validation(
data=data,
bad_req_i=(1, 'VALIDATION_ERROR', message)
)
prop_types = prop["type"]
if type(prop_types) is not list:
prop_types = [prop_types]
for prop_type in prop_types:
if prop_type in _types_checks:
for type_ex in _types_checks[prop_type]:
data = default_data.copy()
value = type_ex
value_str = str(value)
if isinstance(value, str):
value_str = "'%s'" % value_str
data.update({p_name: value})
message = ("%s is not of type %s" %
(value_str,
self._prop_types_str(prop_types)))
if "enum" in prop:
message = [message, "%s is not one of %s" %
(value_str, prop["enum"])]
self._assert_create_object_validation(
data=data,
bad_req_i=(1, 'VALIDATION_ERROR', message)
)
def _assert_cluster_configs_validation(self, require_image_id=False):
data = {