a-o-i: Better inventory group handling

A more dynamic and flexible method of writing out host groups to the ansible
inventory file. To accompany this, in the quick-installer interactive mode
the etcd role is automatically applied to any masters. Anyone hand writing
the quick installer config will now have to explicitly specify which hosts
will be etcd.

Fixes #2200

3 files changed, 49 insertions, 72 deletions

diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py
index 9fbb61a46..b9750fad1 100644
--- a/utils/src/ooinstall/cli_installer.py
+++ b/utils/src/ooinstall/cli_installer.py
@@ -121,7 +121,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen
     click.echo(message)
 
     hosts = []
-    roles = set(['master', 'node', 'storage'])
+    roles = set(['master', 'node', 'storage', 'etcd'])
     more_hosts = True
     num_masters = 0
     while more_hosts:
@@ -133,6 +133,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen
         if not masters_set:
             if click.confirm('Will this host be an OpenShift Master?'):
                 host_props['roles'].append('master')
+                host_props['roles'].append('etcd')
                 num_masters += 1
 
                 if oo_cfg.settings['variant_version'] == '3.0':
diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py
index bcf06b599..54178f0cd 100644
--- a/utils/src/ooinstall/openshift_ansible.py
+++ b/utils/src/ooinstall/openshift_ansible.py
@@ -1,5 +1,3 @@
-# TODO: Temporarily disabled due to importing old code into openshift-ansible
-# repo. We will work on these over time.
 # pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,global-statement,global-variable-not-assigned
 
 import socket
@@ -14,7 +12,9 @@ CFG = None
 ROLES_TO_GROUPS_MAP = {
     'master': 'masters',
     'node': 'nodes',
-    'storage': 'nfs'
+    'etcd': 'etcd',
+    'storage': 'nfs',
+    'master_lb': 'lb'
 }
 
 VARIABLES_MAP = {
@@ -34,75 +34,52 @@ def set_config(cfg):
 
 def generate_inventory(hosts):
     global CFG
+
     masters = [host for host in hosts if host.is_master()]
-    nodes = [host for host in hosts if host.is_node()]
-    new_nodes = [host for host in hosts if host.is_node() and host.new_host]
-    proxy = determine_proxy_configuration(hosts)
-    storage = determine_storage_configuration(hosts)
     multiple_masters = len(masters) > 1
+
+    new_nodes = [host for host in hosts if host.is_node() and host.new_host]
     scaleup = len(new_nodes) > 0
 
+    lb = determine_lb_configuration(hosts)
+
     base_inventory_path = CFG.settings['ansible_inventory_path']
     base_inventory = open(base_inventory_path, 'w')
 
-    write_inventory_children(base_inventory, multiple_masters, proxy, scaleup)
-
-    write_inventory_vars(base_inventory, multiple_masters, proxy)
-
-
-
-    base_inventory.write('\n[masters]\n')
-    for master in masters:
-        write_host(master, base_inventory)
-
-    if len(masters) > 1:
-        base_inventory.write('\n[etcd]\n')
-        for master in masters:
-            write_host(master, base_inventory)
+    write_inventory_children(base_inventory, scaleup)
 
-    base_inventory.write('\n[nodes]\n')
+    write_inventory_vars(base_inventory, multiple_masters, lb)
 
-    for node in nodes:
-        # Let the fact defaults decide if we're not a master:
-        schedulable = None
-
-        # If the node is also a master, we must explicitly set schedulablity:
-        if node.is_master():
-            schedulable = node.is_schedulable_node(hosts)
-        write_host(node, base_inventory, schedulable)
-
-    if not getattr(proxy, 'preconfigured', True):
-        base_inventory.write('\n[lb]\n')
-        write_host(proxy, base_inventory)
 
+    #write_inventory_hosts
+    for role in CFG.deployment.roles:
+        # write group block
+        group = ROLES_TO_GROUPS_MAP.get(role, role)
+        base_inventory.write("\n[{}]\n".format(group))
+        # write each host
+        group_hosts = [host for host in hosts if role in host.roles]
+        for host in group_hosts:
+            schedulable = host.is_schedulable_node(hosts)
+            write_host(host, role, base_inventory, schedulable)
 
     if scaleup:
         base_inventory.write('\n[new_nodes]\n')
         for node in new_nodes:
-            write_host(node, base_inventory)
-
-    if storage:
-        base_inventory.write('\n[nfs]\n')
-        write_host(storage, base_inventory)
+            write_host(node, 'new_nodes', base_inventory)
 
     base_inventory.close()
     return base_inventory_path
 
-def determine_proxy_configuration(hosts):
-    proxy = next((host for host in hosts if host.is_master_lb()), None)
-    if proxy:
-        if proxy.hostname == None:
-            proxy.hostname = proxy.connect_to
-            proxy.public_hostname = proxy.connect_to
-
-    return proxy
+def determine_lb_configuration(hosts):
+    lb = next((host for host in hosts if host.is_master_lb()), None)
+    if lb:
+        if lb.hostname == None:
+            lb.hostname = lb.connect_to
+            lb.public_hostname = lb.connect_to
 
-def determine_storage_configuration(hosts):
-    storage = next((host for host in hosts if host.is_storage()), None)
+    return lb
 
-    return storage
-
-def write_inventory_children(base_inventory, multiple_masters, proxy, scaleup):
+def write_inventory_children(base_inventory, scaleup):
     global CFG
 
     base_inventory.write('\n[OSEv3:children]\n')
@@ -112,13 +89,10 @@ def write_inventory_children(base_inventory, multiple_masters, proxy, scaleup):
 
     if scaleup:
         base_inventory.write('new_nodes\n')
-    if multiple_masters:
-        base_inventory.write('etcd\n')
-    if not getattr(proxy, 'preconfigured', True):
-        base_inventory.write('lb\n')
+
 
 # pylint: disable=too-many-branches
-def write_inventory_vars(base_inventory, multiple_masters, proxy):
+def write_inventory_vars(base_inventory, multiple_masters, lb):
     global CFG
     base_inventory.write('\n[OSEv3:vars]\n')
 
@@ -135,11 +109,11 @@ def write_inventory_vars(base_inventory, multiple_masters, proxy):
     if CFG.settings['ansible_ssh_user'] != 'root':
         base_inventory.write('ansible_become=yes\n')
 
-    if multiple_masters and proxy is not None:
+    if multiple_masters and lb is not None:
         base_inventory.write('openshift_master_cluster_method=native\n')
-        base_inventory.write("openshift_master_cluster_hostname={}\n".format(proxy.hostname))
+        base_inventory.write("openshift_master_cluster_hostname={}\n".format(lb.hostname))
         base_inventory.write(
-            "openshift_master_cluster_public_hostname={}\n".format(proxy.public_hostname))
+            "openshift_master_cluster_public_hostname={}\n".format(lb.public_hostname))
 
     if CFG.settings.get('variant_version', None) == '3.1':
         #base_inventory.write('openshift_image_tag=v{}\n'.format(CFG.settings.get('variant_version')))
@@ -195,9 +169,12 @@ def write_proxy_settings(base_inventory):
 
 
 # pylint: disable=too-many-branches
-def write_host(host, inventory, schedulable=None):
+def write_host(host, role, inventory, schedulable=None):
     global CFG
 
+    if host.preconfigured:
+        return
+
     facts = ''
     if host.ip:
         facts += ' openshift_ip={}'.format(host.ip)
@@ -215,14 +192,13 @@ def write_host(host, inventory, schedulable=None):
     if host.node_labels:
         facts += ' openshift_node_labels="{}"'.format(host.node_labels)
 
+
     # Distinguish between three states, no schedulability specified (use default),
     # explicitly set to True, or explicitly set to False:
-    if schedulable is None:
+    if role != 'node' or schedulable is None:
         pass
-    elif schedulable:
-        facts += ' openshift_schedulable=True'
-    elif not schedulable:
-        facts += ' openshift_schedulable=False'
+    else:
+        facts += " openshift_schedulable={}".format(schedulable)
 
     installer_host = socket.gethostname()
     if installer_host in [host.connect_to, host.hostname, host.public_hostname]:
diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py
index 13973f22f..d79163e91 100644
--- a/utils/test/cli_installer_tests.py
+++ b/utils/test/cli_installer_tests.py
@@ -815,9 +815,9 @@ class AttendedCliTests(OOCliFixture):
         self.assert_inventory_host_var(inventory, 'nodes', '10.0.0.1',
                                  'openshift_schedulable=False')
         self.assert_inventory_host_var_unset(inventory, 'nodes', '10.0.0.2',
-                                 'openshift_schedulable')
+                                 'openshift_schedulable=True')
         self.assert_inventory_host_var_unset(inventory, 'nodes', '10.0.0.3',
-                                 'openshift_schedulable')
+                                 'openshift_schedulable=True')
 
     # interactive with config file and some installed some uninstalled hosts
     @patch('ooinstall.openshift_ansible.run_main_playbook')
@@ -939,7 +939,7 @@ class AttendedCliTests(OOCliFixture):
         self.assert_inventory_host_var(inventory, 'nodes', '10.0.0.3',
                                        'openshift_schedulable=False')
         self.assert_inventory_host_var_unset(inventory, 'nodes', '10.0.0.4',
-                                             'openshift_schedulable')
+                                             'openshift_schedulable=True')
 
         self.assertTrue(inventory.has_section('etcd'))
         self.assertEquals(3, len(inventory.items('etcd')))
@@ -1122,9 +1122,9 @@ class AttendedCliTests(OOCliFixture):
         self.assert_inventory_host_var(inventory, 'nodes', '10.0.0.1',
                                  'openshift_schedulable=False')
         self.assert_inventory_host_var_unset(inventory, 'nodes', '10.0.0.2',
-                                 'openshift_schedulable')
+                                 'openshift_schedulable=True')
         self.assert_inventory_host_var_unset(inventory, 'nodes', '10.0.0.3',
-                                 'openshift_schedulable')
+                                 'openshift_schedulable=True')
 
 
 # TODO: test with config file, attended add node