Merge pull request #1009 from smunilla/nfs_question

Questions for NFS Storage Registry

5 files changed, 130 insertions, 28 deletions

diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py
index ace834323..c53ca7b18 100644
--- a/utils/src/ooinstall/cli_installer.py
+++ b/utils/src/ooinstall/cli_installer.py
@@ -163,8 +163,12 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen
         if masters_set or num_masters != 2:
             more_hosts = click.confirm('Do you want to add additional hosts?')
 
-    if num_masters >= 3:
+    if num_masters == 1:
+        master = next((host for host in hosts if host.master), None)
+        master.storage = True
+    elif num_masters >= 3:
         collect_master_lb(hosts)
+        collect_storage_host(hosts)
 
     return hosts
 
@@ -202,8 +206,9 @@ Please add one more to proceed."""
     elif len(masters) >= 3:
         ha_message = """
 NOTE: Multiple Masters specified, this will be an HA deployment with a separate
-etcd cluster. You will be prompted to provide the FQDN of a load balancer once
-finished entering hosts."""
+etcd cluster. You will be prompted to provide the FQDN of a load balancer and
+a host for storage once finished entering hosts.
+"""
         click.echo(ha_message)
 
         dedicated_nodes_message = """
@@ -243,6 +248,8 @@ def print_host_summary(all_hosts, host):
             click.echo("  - Etcd Member")
         else:
             click.echo("  - Etcd (Embedded)")
+    if host.storage:
+        click.echo("  - Storage")
 
 
 def collect_master_lb(hosts):
@@ -291,6 +298,43 @@ hostname.
     master_lb = Host(**host_props)
     hosts.append(master_lb)
 
+def collect_storage_host(hosts):
+    """
+    Get a valid host for storage from the user and append it to the list of
+    hosts.
+    """
+    message = """
+Setting up High Availability Masters requires a storage host. Please provide a
+host that will be configured as a Registry Storage.
+"""
+    click.echo(message)
+    host_props = {}
+
+    hostname_or_ip = click.prompt('Enter hostname or IP address',
+                                            value_proc=validate_prompt_hostname)
+    existing, existing_host = is_host_already_node_or_master(hostname_or_ip, hosts)
+    if existing and existing_host.node:
+        existing_host.storage = True
+    else:
+        host_props['connect_to'] = hostname_or_ip
+        host_props['preconfigured'] = False
+        host_props['master'] = False
+        host_props['node'] = False
+        host_props['storage'] = True
+        storage = Host(**host_props)
+        hosts.append(storage)
+
+def is_host_already_node_or_master(hostname, hosts):
+    is_existing = False
+    existing_host = None
+
+    for host in hosts:
+        if host.connect_to == hostname and (host.master or host.node):
+            is_existing = True
+            existing_host = host
+
+    return is_existing, existing_host
+
 def confirm_hosts_facts(oo_cfg, callback_facts):
     hosts = oo_cfg.hosts
     click.clear()
@@ -330,11 +374,15 @@ Notes:
     for h in hosts:
         if h.preconfigured == True:
             continue
-        default_facts[h.connect_to] = {}
-        h.ip = callback_facts[h.connect_to]["common"]["ip"]
-        h.public_ip = callback_facts[h.connect_to]["common"]["public_ip"]
-        h.hostname = callback_facts[h.connect_to]["common"]["hostname"]
-        h.public_hostname = callback_facts[h.connect_to]["common"]["public_hostname"]
+        try:
+            default_facts[h.connect_to] = {}
+            h.ip = callback_facts[h.connect_to]["common"]["ip"]
+            h.public_ip = callback_facts[h.connect_to]["common"]["public_ip"]
+            h.hostname = callback_facts[h.connect_to]["common"]["hostname"]
+            h.public_hostname = callback_facts[h.connect_to]["common"]["public_hostname"]
+        except KeyError:
+            click.echo("Problem fetching facts from {}".format(h.connect_to))
+            continue
 
         default_facts_lines.append(",".join([h.connect_to,
                                              h.ip,
diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py
index b1af21773..c9498542f 100644
--- a/utils/src/ooinstall/oo_config.py
+++ b/utils/src/ooinstall/oo_config.py
@@ -50,13 +50,17 @@ class Host(object):
         # Should this host run as an HAProxy:
         self.master_lb = kwargs.get('master_lb', False)
 
+        # Should this host run as an HAProxy:
+        self.storage = kwargs.get('storage', False)
+
         self.containerized = kwargs.get('containerized', False)
 
         if self.connect_to is None:
             raise OOConfigInvalidHostError("You must specify either an ip " \
                 "or hostname as 'connect_to'")
 
-        if self.master is False and self.node is False and self.master_lb is False:
+        if self.master is False and self.node is False and \
+           self.master_lb is False and self.storage is False:
             raise OOConfigInvalidHostError(
                 "You must specify each host as either a master or a node.")
 
@@ -70,7 +74,7 @@ class Host(object):
         """ Used when exporting to yaml. """
         d = {}
         for prop in ['ip', 'hostname', 'public_ip', 'public_hostname',
-                     'master', 'node', 'master_lb', 'containerized',
+                     'master', 'node', 'master_lb', 'storage', 'containerized',
                      'connect_to', 'preconfigured', 'new_host']:
             # If the property is defined (not None or False), export it:
             if getattr(self, prop):
diff --git a/utils/src/ooinstall/openshift_ansible.py b/utils/src/ooinstall/openshift_ansible.py
index 3a135139b..04cccf89d 100644
--- a/utils/src/ooinstall/openshift_ansible.py
+++ b/utils/src/ooinstall/openshift_ansible.py
@@ -21,13 +21,14 @@ def generate_inventory(hosts):
     nodes = [host for host in hosts if host.node]
     new_nodes = [host for host in hosts if host.node and host.new_host]
     proxy = determine_proxy_configuration(hosts)
+    storage = determine_storage_configuration(hosts)
     multiple_masters = len(masters) > 1
     scaleup = len(new_nodes) > 0
 
     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_children(base_inventory, multiple_masters, proxy, storage, scaleup)
 
     write_inventory_vars(base_inventory, multiple_masters, proxy)
 
@@ -73,11 +74,16 @@ def generate_inventory(hosts):
         base_inventory.write('\n[lb]\n')
         write_host(proxy, base_inventory)
 
+
     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)
+
     base_inventory.close()
     return base_inventory_path
 
@@ -87,11 +93,15 @@ def determine_proxy_configuration(hosts):
         if proxy.hostname == None:
             proxy.hostname = proxy.connect_to
             proxy.public_hostname = proxy.connect_to
-        return proxy
 
-    return None
+    return proxy
+
+def determine_storage_configuration(hosts):
+    storage = next((host for host in hosts if host.storage), None)
+
+    return storage
 
-def write_inventory_children(base_inventory, multiple_masters, proxy, scaleup):
+def write_inventory_children(base_inventory, multiple_masters, proxy, storage, scaleup):
     global CFG
 
     base_inventory.write('\n[OSEv3:children]\n')
@@ -103,6 +113,8 @@ def write_inventory_children(base_inventory, multiple_masters, proxy, scaleup):
         base_inventory.write('etcd\n')
     if not getattr(proxy, 'preconfigured', True):
         base_inventory.write('lb\n')
+    if storage:
+        base_inventory.write('nfs\n')
 
 def write_inventory_vars(base_inventory, multiple_masters, proxy):
     global CFG
diff --git a/utils/test/cli_installer_tests.py b/utils/test/cli_installer_tests.py
index baab5d56f..6ba5ec1eb 100644
--- a/utils/test/cli_installer_tests.py
+++ b/utils/test/cli_installer_tests.py
@@ -72,6 +72,14 @@ MOCK_FACTS_QUICKHA = {
             'public_hostname': 'proxy.example.com'
         }
     },
+    '10.1.0.1': {
+        'common': {
+            'ip': '10.1.0.1',
+            'public_ip': '10.1.0.1',
+            'hostname': 'storage-private.example.com',
+            'public_hostname': 'storage.example.com'
+        }
+    },
 }
 
 # Missing connect_to on some hosts:
@@ -137,6 +145,12 @@ hosts:
     public_ip: 24.222.0.5
     public_hostname: proxy.example.com
     master_lb: true
+  - connect_to: 10.1.0.1
+    ip: 10.1.0.1
+    hostname: storage-private.example.com
+    public_ip: 24.222.0.6
+    public_hostname: storage.example.com
+    storage: true
 """
 
 QUICKHA_2_MASTER_CONFIG = """
@@ -169,6 +183,12 @@ hosts:
     public_ip: 24.222.0.5
     public_hostname: proxy.example.com
     master_lb: true
+  - connect_to: 10.1.0.1
+    ip: 10.1.0.1
+    hostname: storage-private.example.com
+    public_ip: 24.222.0.6
+    public_hostname: storage.example.com
+    storage: true
 """
 
 QUICKHA_CONFIG_REUSED_LB = """
@@ -197,6 +217,12 @@ hosts:
     public_hostname: node2.example.com
     node: true
     master: true
+  - connect_to: 10.1.0.1
+    ip: 10.1.0.1
+    hostname: storage-private.example.com
+    public_ip: 24.222.0.6
+    public_hostname: storage.example.com
+    storage: true
 """
 
 QUICKHA_CONFIG_NO_LB = """
@@ -263,6 +289,12 @@ hosts:
     public_hostname: proxy.example.com
     master_lb: true
     preconfigured: true
+  - connect_to: 10.1.0.1
+    ip: 10.1.0.1
+    hostname: storage-private.example.com
+    public_ip: 24.222.0.6
+    public_hostname: storage.example.com
+    storage: true
 """
 
 class UnattendedCliTests(OOCliFixture):
@@ -595,8 +627,8 @@ class UnattendedCliTests(OOCliFixture):
         # Make sure we ran on the expected masters and nodes:
         hosts = run_playbook_mock.call_args[0][0]
         hosts_to_run_on = run_playbook_mock.call_args[0][1]
-        self.assertEquals(5, len(hosts))
-        self.assertEquals(5, len(hosts_to_run_on))
+        self.assertEquals(6, len(hosts))
+        self.assertEquals(6, len(hosts_to_run_on))
 
     #unattended with two masters, one node, and haproxy
     @patch('ooinstall.openshift_ansible.run_main_playbook')
@@ -665,8 +697,8 @@ class UnattendedCliTests(OOCliFixture):
         # Make sure we ran on the expected masters and nodes:
         hosts = run_playbook_mock.call_args[0][0]
         hosts_to_run_on = run_playbook_mock.call_args[0][1]
-        self.assertEquals(5, len(hosts))
-        self.assertEquals(5, len(hosts_to_run_on))
+        self.assertEquals(6, len(hosts))
+        self.assertEquals(6, len(hosts_to_run_on))
 
 class AttendedCliTests(OOCliFixture):
 
@@ -805,17 +837,18 @@ class AttendedCliTests(OOCliFixture):
                                       ssh_user='root',
                                       variant_num=1,
                                       confirm_facts='y',
-                                      master_lb=('10.0.0.5', False))
+                                      master_lb=('10.0.0.5', False),
+                                      storage='10.1.0.1',)
         self.cli_args.append("install")
         result = self.runner.invoke(cli.cli, self.cli_args,
             input=cli_input)
         self.assert_result(result, 0)
 
         self._verify_load_facts(load_facts_mock)
-        self._verify_run_playbook(run_playbook_mock, 5, 5)
+        self._verify_run_playbook(run_playbook_mock, 6, 6)
 
         written_config = read_yaml(self.config_file)
-        self._verify_config_hosts(written_config, 5)
+        self._verify_config_hosts(written_config, 6)
 
         inventory = ConfigParser.ConfigParser(allow_no_value=True)
         inventory.read(os.path.join(self.work_dir, '.ansible/hosts'))
@@ -845,17 +878,18 @@ class AttendedCliTests(OOCliFixture):
                                       ssh_user='root',
                                       variant_num=1,
                                       confirm_facts='y',
-                                      master_lb=('10.0.0.5', False))
+                                      master_lb=('10.0.0.5', False),
+                                      storage='10.1.0.1',)
         self.cli_args.append("install")
         result = self.runner.invoke(cli.cli, self.cli_args,
             input=cli_input)
         self.assert_result(result, 0)
 
         self._verify_load_facts(load_facts_mock)
-        self._verify_run_playbook(run_playbook_mock, 4, 4)
+        self._verify_run_playbook(run_playbook_mock, 5, 5)
 
         written_config = read_yaml(self.config_file)
-        self._verify_config_hosts(written_config, 4)
+        self._verify_config_hosts(written_config, 5)
 
         inventory = ConfigParser.ConfigParser(allow_no_value=True)
         inventory.read(os.path.join(self.work_dir, '.ansible/hosts'))
@@ -881,7 +915,8 @@ class AttendedCliTests(OOCliFixture):
                                       ssh_user='root',
                                       variant_num=1,
                                       confirm_facts='y',
-                                      master_lb=(['10.0.0.2', '10.0.0.5'], False))
+                                      master_lb=(['10.0.0.2', '10.0.0.5'], False),
+                                      storage='10.1.0.1')
         self.cli_args.append("install")
         result = self.runner.invoke(cli.cli, self.cli_args,
             input=cli_input)
diff --git a/utils/test/fixture.py b/utils/test/fixture.py
index 1b1c2e5c2..d6222dfaa 100644
--- a/utils/test/fixture.py
+++ b/utils/test/fixture.py
@@ -92,7 +92,7 @@ class OOCliFixture(OOInstallFixture):
             self.assertTrue('hostname' in host)
             self.assertTrue('public_hostname' in host)
             if 'preconfigured' not in host:
-                self.assertTrue(host['node'])
+                self.assertTrue('node' in host or 'storage' in host)
                 self.assertTrue('ip' in host)
                 self.assertTrue('public_ip' in host)
 
@@ -142,7 +142,7 @@ class OOCliFixture(OOInstallFixture):
 #pylint: disable=too-many-arguments,too-many-branches,too-many-statements
 def build_input(ssh_user=None, hosts=None, variant_num=None,
                 add_nodes=None, confirm_facts=None, schedulable_masters_ok=None,
-                master_lb=None):
+                master_lb=None, storage=None):
     """
     Build an input string simulating a user entering values in an interactive
     attended install.
@@ -197,7 +197,10 @@ def build_input(ssh_user=None, hosts=None, variant_num=None,
             inputs.append(master_lb[0])
         inputs.append('y' if master_lb[1] else 'n')
 
-    inputs.append('example.com')
+    if storage:
+        inputs.append(storage)
+
+    inputs.append('subdomain.example.com')
 
     # TODO: support option 2, fresh install
     if add_nodes: