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

sahara/conductor/manager.py

@@ -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)
 

sahara/db/sqlalchemy/api.py

@@ -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()

sahara/db/templates/api.py

@@ -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
+                template[opt] = None
-                # 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))
             else:
                 # Use args to allow for keyword arguments to format
                 args = {opt: value}

sahara/service/validations/cluster_template_schema.py

@@ -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"
         },
     },

sahara/service/validations/node_group_template_schema.py

@@ -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",
+            "type": ["array", "null"],
-            "items": {
+            "items": {"type": "string"}
-                "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,

sahara/tests/unit/conductor/manager/test_templates.py

@@ -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,
+    "image_id": str(uuid.uuid4()),
-    "availability_zone": None,
     "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])

sahara/tests/unit/db/templates/test_update.py

@@ -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 = []

sahara/tests/unit/service/validation/test_cluster_template_create_validation.py

@@ -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):

sahara/tests/unit/service/validation/test_ng_template_validation_create.py

@@ -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',
+                'volume_type': 'fish',
-                'floating_ip_pool': 'd9a3bebc-f788-4b81-9a93-aa048022c1ca'
+                '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
             }
         )
 

sahara/tests/unit/service/validation/utils.py

@@ -30,10 +30,11 @@ from sahara.tests.unit import testutils as tu
 m = {}
 
 _types_checks = {
-    "string": [1, (), {}],
+    "string": [1, (), {}, True],
-    "integer": ["a", (), {}],
+    "integer": ["a", (), {}, True],
-    "uuid": ["z550e8400-e29b-41d4-a716-446655440000", 1, "a", (), {}],
+    "uuid": ["z550e8400-e29b-41d4-a716-446655440000", 1, "a", (), {}, True],
-    "array": [{}, 'a', 1]
+    "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:
+            prop_types = prop["type"]
-                for type_ex in _types_checks[prop["type"]]:
+            if type(prop_types) is not list:
-                    data = default_data.copy()
+                prop_types = [prop_types]
-                    value = type_ex
+            for prop_type in prop_types:
-                    value_str = str(value)
+                if prop_type in _types_checks:
-                    if isinstance(value, str):
+                    for type_ex in _types_checks[prop_type]:
-                        value_str = "'%s'" % value_str
+                        data = default_data.copy()
-                    data.update({p_name: value})
+                        value = type_ex
-                    message = ("%s is not of type '%s'" %
+                        value_str = str(value)
-                               (value_str, prop["type"]))
+                        if isinstance(value, str):
-                    if "enum" in prop:
+                            value_str = "'%s'" % value_str
-                        message = [message, "%s is not one of %s" %
+                        data.update({p_name: value})
-                                            (value_str, prop["enum"])]
+                        message = ("%s is not of type %s" %
-                    self._assert_create_object_validation(
+                                   (value_str,
-                        data=data,
+                                    self._prop_types_str(prop_types)))
-                        bad_req_i=(1, 'VALIDATION_ERROR', message)
+                        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 = {