From 1e723337029b8ea77f12dd2d75a4a426570f1c63 Mon Sep 17 00:00:00 2001
From: Steven Hardy <shardy@redhat.com>
Date: Wed, 21 Sep 2016 11:10:47 +0100
Subject: [PATCH] Make defaults from roles_data.yaml more robust

The previous logic left out the default Count completely when it was
zero, which breaks nested validation and it's likely similar problems
would exist with the other optional defaults, so rework it so the
defaulting happens in the jinja2 logic, and document the interfaces
better in roles_data.yaml

Change-Id: I7f2eb4a3a0b43c5d2cd0d001ed3c73f783c95c74
Closes-Bug: #1625760
---
 overcloud.j2.yaml | 10 ++++------
 roles_data.yaml   | 27 ++++++++++++++++++++-------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/overcloud.j2.yaml b/overcloud.j2.yaml
index 187741ff5d..aad1af6232 100644
--- a/overcloud.j2.yaml
+++ b/overcloud.j2.yaml
@@ -121,16 +121,12 @@ parameters:
                  resource_registry) which represent nested stacks
                  for each service that should get installed on the {{role.name}} role.
     type: comma_delimited_list
-  {% if role.ServicesDefault %}
-    default: {{role.ServicesDefault}}
-  {% endif %}
+    default: {{role.ServicesDefault|default([])}}
 
   {{role.name}}Count:
     description: Number of {{role.name}} nodes to deploy
     type: number
-  {% if role.CountDefault %}
-    default: {{role.CountDefault}}
-  {% endif %}
+    default: {{role.CountDefault|default(0)}}
 
   {{role.name}}HostnameFormat:
     type: string
@@ -140,6 +136,8 @@ parameters:
       and %stackname% is replaced with the stack name e.g overcloud
   {% if role.HostnameFormatDefault %}
     default: "{{role.HostnameFormatDefault}}"
+  {% else %}
+    default: "%stackname%-{{role.name.lower()}}-%index%"
   {% endif %}
 
   {{role.name}}RemovalPolicies:
diff --git a/roles_data.yaml b/roles_data.yaml
index af1eba82de..e052aeef9e 100644
--- a/roles_data.yaml
+++ b/roles_data.yaml
@@ -1,6 +1,25 @@
+# Specifies which roles (groups of nodes) will be deployed
+# Note this is used as an input to the various *.j2.yaml
+# jinja2 templates, so that they are converted into *.yaml
+# during the plan creation (via a mistral action/workflow).
+#
+# The format is a list, with the following format:
+#
+# * name: (string) mandatory, name of the role, must be unique
+#
+# CountDefault: (number) optional, default number of nodes, defaults to 0
+# sets the default for the {{role.name}}Count parameter in overcloud.yaml
+#
+# HostnameFormatDefault: (string) optional default format string for hostname
+# defaults to '%stackname%-{{role.name.lower()}}-%index%'
+# sets the default for {{role.name}}HostnameFormat parameter in overcloud.yaml
+#
+# ServicesDefault: (list) optional default list of services to be deployed
+# on the role, defaults to an empty list. Sets the default for the
+# {{role.name}}Services parameter in overcloud.yaml
+
 - name: Controller
   CountDefault: 1
-  HostnameFormatDefault: '%stackname%-controller-%index%'
   ServicesDefault:
     - OS::TripleO::Services::CACerts
     - OS::TripleO::Services::CephMon
@@ -102,8 +121,6 @@
     - OS::TripleO::Services::VipHosts
 
 - name: BlockStorage
-  CountDefault: 0
-  HostnameFormatDefault: '%stackname%-blockstorage-%index%'
   ServicesDefault:
     - OS::TripleO::Services::CACerts
     - OS::TripleO::Services::CinderVolume
@@ -118,8 +135,6 @@
     - OS::TripleO::Services::VipHosts
 
 - name: ObjectStorage
-  CountDefault: 0
-  HostnameFormatDefault: '%stackname%-objectstorage-%index%'
   ServicesDefault:
     - OS::TripleO::Services::CACerts
     - OS::TripleO::Services::Kernel
@@ -135,8 +150,6 @@
     - OS::TripleO::Services::VipHosts
 
 - name: CephStorage
-  CountDefault: 0
-  HostnameFormatDefault: '%stackname%-cephstorage-%index%'
   ServicesDefault:
     - OS::TripleO::Services::CACerts
     - OS::TripleO::Services::CephOSD