Merge pull request #7568 from mgugino-upstream-stage/fix-etcd-hosts-list-master

Remove etcd_hosts and etcd_urls from openshift_facts

Author: michaelgugino

playbooks/init/cluster_facts.yml

@@ -45,3 +45,29 @@
       role: node
       local_facts:
         sdn_mtu: "{{ openshift_node_sdn_mtu | default(None) }}"
+
+- name: Initialize etcd host variables
+  hosts: oo_masters_to_config
+  roles:
+  - role: openshift_facts
+  - role: lib_utils
+  tasks:
+  # This needs to run after openshift_facts common role has run on etcd hosts.
+  # lib_utils_oo_select_keys, lib_utils_oo_collect
+  # are custom filter plugins in roles/lib_utils/filter_plugins/oo_filters.py
+  - set_fact:
+      openshift_no_proxy_etcd_host_ips: "{{ hostvars | lib_utils_oo_select_keys(groups['oo_etcd_to_config'] | default([]))
+                                                  | lib_utils_oo_collect('openshift.common.ip') | default([]) | join(',')
+                                                  }}"
+      openshift_master_etcd_port: "{{ etcd_client_port | default('2379') }}"
+      openshift_master_etcd_hosts: "{{ hostvars
+                                       | lib_utils_oo_select_keys(groups['oo_etcd_to_config']
+                                                        | default([]))
+                                       | lib_utils_oo_collect('openshift.common.hostname')
+                                       | default(none, true) }}"
+  # This fact requires the facts set above, so needs to happen in it's own task.
+  - set_fact:
+      # lib_utils_oo_etcd_host_urls is a custom filter plugin in roles/lib_utils/filter_plugins/oo_filters.py
+      openshift_master_etcd_urls: "{{ openshift_master_etcd_hosts | lib_utils_oo_etcd_host_urls(l_use_ssl, openshift_master_etcd_port) }}"
+    vars:
+      l_use_ssl: "{{ openshift_master_etcd_use_ssl | default(True) | bool}}"

playbooks/openshift-etcd/private/scaleup.yml

@@ -64,12 +64,23 @@
 - name: Update master etcd client urls
   hosts: oo_masters_to_config
   serial: 1
-  vars:
-    openshift_master_etcd_hosts: "{{ hostvars
-                                     | lib_utils_oo_select_keys(groups['oo_etcd_to_config'] | union(groups['oo_new_etcd_to_config'] | default([]) ))
-                                     | lib_utils_oo_collect('openshift.common.hostname')
-                                     | default(none, true) }}"
-    openshift_master_etcd_port: "{{ (etcd_client_port | default('2379')) if (groups.oo_etcd_to_config is defined and groups.oo_etcd_to_config) else none }}"
+  pre_tasks:
+  - set_fact:
+      openshift_master_etcd_hosts: "{{ hostvars
+                                       | lib_utils_oo_select_keys(groups['oo_etcd_to_config'] | union(groups['oo_new_etcd_to_config'] | default([]) ))
+                                       | lib_utils_oo_collect('openshift.common.hostname')
+                                       | default(none, true) }}"
+      openshift_master_etcd_port: "{{ etcd_client_port | default('2379') }}"
+      openshift_no_proxy_etcd_host_ips: "{{ hostvars | lib_utils_oo_select_keys(groups['oo_etcd_to_config'] | default([]))
+                                                  | lib_utils_oo_collect('openshift.common.ip') | default([]) | join(',')
+                                                  }}"
+  # This fact requires the facts set above, so needs to happen in it's own task.
+  - set_fact:
+      # lib_utils_oo_etcd_host_urls is a custom filter plugin in roles/lib_utils/filter_plugins/oo_filters.py
+      openshift_master_etcd_urls: "{{ openshift_master_etcd_hosts | lib_utils_oo_etcd_host_urls(l_use_ssl, openshift_master_etcd_port) }}"
+    vars:
+      l_use_ssl: "{{ openshift_master_etcd_use_ssl | default(True) | bool}}"
+
   roles:
   - role: openshift_master_facts
   post_tasks:

playbooks/openshift-master/private/additional_config.yml

@@ -15,7 +15,7 @@
   hosts: oo_first_master
   vars:
     cockpit_plugins: "{{ osm_cockpit_plugins | default(['cockpit-kubernetes']) }}"
-    etcd_urls: "{{ openshift.master.etcd_urls }}"
+    etcd_urls: "{{ openshift_master_etcd_urls }}"
     omc_cluster_hosts: "{{ groups.oo_masters | join(' ')}}"
   roles:
   # TODO: this is currently required in order to schedule pods onto the masters, but

playbooks/openshift-master/private/certificates.yml

@@ -8,7 +8,3 @@
   - role: openshift_named_certificates
   - role: openshift_ca
   - role: openshift_master_certificates
-    openshift_master_etcd_hosts: "{{ hostvars
-                                     | lib_utils_oo_select_keys(groups['oo_etcd_to_config'] | default([]))
-                                     | lib_utils_oo_collect('openshift.common.hostname')
-                                     | default(none, true) }}"

playbooks/openshift-master/private/config.yml

@@ -53,13 +53,6 @@
     - node
     - .config_managed
 
-  - set_fact:
-      openshift_master_etcd_port: "{{ (etcd_client_port | default('2379')) if (groups.oo_etcd_to_config is defined and groups.oo_etcd_to_config) else none }}"
-      openshift_master_etcd_hosts: "{{ hostvars
-                                       | lib_utils_oo_select_keys(groups['oo_etcd_to_config']
-                                                        | default([]))
-                                       | lib_utils_oo_collect('openshift.common.hostname')
-                                       | default(none, true) }}"
   roles:
   - openshift_facts
   post_tasks:
@@ -169,13 +162,6 @@
     openshift_master_session_auth_secrets: "{{ hostvars[groups.oo_first_master.0].openshift.master.session_auth_secrets }}"
     openshift_master_session_encryption_secrets: "{{ hostvars[groups.oo_first_master.0].openshift.master.session_encryption_secrets }}"
     openshift_ca_host: "{{ groups.oo_first_master.0 }}"
-    openshift_master_etcd_hosts: "{{ hostvars
-                                     | lib_utils_oo_select_keys(groups['oo_etcd_to_config'] | default([]))
-                                     | lib_utils_oo_collect('openshift.common.hostname')
-                                     | default(none, true) }}"
-    openshift_no_proxy_etcd_host_ips: "{{ hostvars | lib_utils_oo_select_keys(groups['oo_etcd_to_config'] | default([]))
-                                                | lib_utils_oo_collect('openshift.common.ip') | default([]) | join(',')
-                                                }}"
   pre_tasks:
   # This will be moved into the control plane role once openshift_master is removed
   - name: Add static pod and systemd shim commands

playbooks/openshift-node/private/additional_config.yml

@@ -32,7 +32,7 @@
   hosts: oo_nodes_use_flannel
   roles:
   - role: flannel
-    etcd_urls: "{{ hostvars[groups.oo_first_master.0].openshift.master.etcd_urls }}"
+    etcd_urls: "{{ hostvars[groups.oo_first_master.0].openshift_master_etcd_urls }}"
     when: openshift_use_flannel | default(false) | bool
 
 - name: Additional node config

roles/calico_master/tasks/certs.yml

@@ -16,7 +16,7 @@
     calico_etcd_ca_cert_file: "/etc/origin/master/master.etcd-ca.crt"
     calico_etcd_cert_file: "/etc/origin/master/master.etcd-client.crt"
     calico_etcd_key_file: "/etc/origin/master/master.etcd-client.key"
-    calico_etcd_endpoints: "{{ hostvars[groups.oo_first_master.0].openshift.master.etcd_urls | join(',') }}"
+    calico_etcd_endpoints: "{{ hostvars[groups.oo_first_master.0].openshift_master_etcd_urls | join(',') }}"
 
 - name: Calico Node | Error if no certs set.
   fail:

roles/lib_utils/filter_plugins/oo_filters.py

@@ -680,6 +680,17 @@ def map_to_pairs(source, delim="="):
     return ','.join(["{}{}{}".format(key, delim, value) for key, value in iteritems(source)])
 
 
+def lib_utils_oo_etcd_host_urls(hosts, use_ssl=True, port='2379'):
+    '''Return a list of urls for etcd hosts'''
+    urls = []
+    port = str(port)
+    proto = "https://" if use_ssl else "http://"
+    for host in hosts:
+        url_string = "{}{}:{}".format(proto, host, port)
+        urls.append(url_string)
+    return urls
+
+
 class FilterModule(object):
     """ Custom ansible filter mapping """
 
@@ -713,4 +724,5 @@ def filters(self):
             "lib_utils_oo_l_of_d_to_csv": lib_utils_oo_l_of_d_to_csv,
             "map_from_pairs": map_from_pairs,
             "map_to_pairs": map_to_pairs,
+            "lib_utils_oo_etcd_host_urls": lib_utils_oo_etcd_host_urls,
         }

roles/nuage_master/templates/nuage-master-config-daemonset.j2

@@ -46,7 +46,7 @@ data:
           certFile: {{ nuage_master_crt_dir }}/nuageEtcd-client.crt
           keyFile: {{ nuage_master_crt_dir }}/nuageEtcd-client.key
           urls:
-      {% for etcd_url in openshift.master.etcd_urls %}
+      {% for etcd_url in openshift_master_etcd_urls %}
               - {{ etcd_url }}
       {% endfor %}
 

roles/openshift_control_plane/tasks/update_etcd_client_urls.yml

@@ -2,6 +2,6 @@
 - yedit:
     src: "{{ openshift.common.config_base }}/master/master-config.yaml"
     key: 'etcdClientInfo.urls'
-    value: "{{ openshift.master.etcd_urls }}"
+    value: "{{ openshift_master_etcd_urls }}"
   notify:
   - restart master

roles/openshift_control_plane/templates/master.yaml.v1.j2

@@ -77,7 +77,7 @@ etcdClientInfo:
   certFile: master.etcd-client.crt
   keyFile: master.etcd-client.key
   urls:
-{% for etcd_url in openshift.master.etcd_urls %}
+{% for etcd_url in openshift_master_etcd_urls %}
     - {{ etcd_url }}
 {% endfor %}
 etcdStorageConfig:

roles/openshift_facts/library/openshift_facts.py

@@ -392,15 +392,13 @@ def set_url_facts_if_unset(facts):
         api_hostname = cluster_hostname if cluster_hostname else hostname
         api_public_hostname = cluster_public_hostname if cluster_public_hostname else public_hostname
         console_path = facts['master']['console_path']
-        etcd_hosts = facts['master']['etcd_hosts']
 
         use_ssl = dict(
             api=facts['master']['api_use_ssl'],
             public_api=facts['master']['api_use_ssl'],
             loopback_api=facts['master']['api_use_ssl'],
             console=facts['master']['console_use_ssl'],
             public_console=facts['master']['console_use_ssl'],
-            etcd=facts['master']['etcd_use_ssl']
         )
 
         ports = dict(
@@ -409,21 +407,8 @@ def set_url_facts_if_unset(facts):
             loopback_api=facts['master']['api_port'],
             console=facts['master']['console_port'],
             public_console=facts['master']['console_port'],
-            etcd=facts['master']['etcd_port'],
         )
 
-        etcd_urls = []
-        if etcd_hosts != '':
-            facts['master']['etcd_port'] = ports['etcd']
-            for host in etcd_hosts:
-                etcd_urls.append(format_url(use_ssl['etcd'], host,
-                                            ports['etcd']))
-        else:
-            etcd_urls = [format_url(use_ssl['etcd'], hostname,
-                                    ports['etcd'])]
-
-        facts['master'].setdefault('etcd_urls', etcd_urls)
-
         prefix_hosts = [('api', api_hostname),
                         ('public_api', api_public_hostname),
                         ('loopback_api', hostname)]
@@ -1069,7 +1054,7 @@ def set_container_facts_if_unset(facts):
 def pop_obsolete_local_facts(local_facts):
     """Remove unused keys from local_facts"""
     keys_to_remove = {
-        'master': ('etcd_port',)
+        'master': ('etcd_port', 'etcd_use_ssl', 'etcd_hosts')
     }
     for role in keys_to_remove:
         if role in local_facts:
@@ -1215,8 +1200,7 @@ def get_defaults(self, roles):
                                       controllers_port='8444',
                                       console_use_ssl=True,
                                       console_path='/console',
-                                      console_port='8443', etcd_use_ssl=True,
-                                      etcd_hosts='', etcd_port='2379',
+                                      console_port='8443',
                                       portal_net='172.30.0.0/16',
                                       embedded_kube=True,
                                       embedded_dns=True,

roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py

@@ -21,9 +21,9 @@ def run(self):
             default=int(0.5 * float(etcd_total_diskspace - etcd_avail_diskspace))
         )
 
-        etcd_is_ssl = self.get_var("openshift", "master", "etcd_use_ssl", default=False)
-        etcd_port = self.get_var("openshift", "master", "etcd_port", default=2379)
-        etcd_hosts = self.get_var("openshift", "master", "etcd_hosts")
+        etcd_is_ssl = self.get_var("openshift_master_etcd_use_ssl", default=True)
+        etcd_port = self.get_var("openshift_master_etcd_port", default=2379)
+        etcd_hosts = self.get_var("openshift_master_etcd_hosts")
 
         config_base = self.get_var("openshift", "common", "config_base")
 

roles/openshift_health_checker/test/etcd_imagedata_size_test.py

@@ -127,9 +127,9 @@ def execute_module(module_name, module_args, *_):
         etcd_max_image_data_size_bytes=size_limit,
         ansible_mounts=ansible_mounts,
         openshift=dict(
-            master=dict(etcd_hosts=["localhost"]),
             common=dict(config_base="/var/lib/origin")
-        )
+        ),
+        openshift_master_etcd_hosts=["localhost"]
     )
     if size_limit is None:
         task_vars.pop("etcd_max_image_data_size_bytes")
@@ -285,9 +285,9 @@ def execute_module(module_name, module_args, *_):
     task_vars = dict(
         ansible_mounts=ansible_mounts,
         openshift=dict(
-            master=dict(etcd_hosts=["localhost"]),
             common=dict(config_base="/var/lib/origin")
-        )
+        ),
+        openshift_master_etcd_hosts=["localhost"]
     )
 
     check = EtcdImageDataSize(execute_module, task_vars).run()
@@ -313,9 +313,9 @@ def execute_module(module_name, *_):
             'size_total': 80 * 10**9,
         }],
         openshift=dict(
-            master=dict(etcd_hosts=["localhost"]),
             common=dict(config_base="/var/lib/origin")
-        )
+        ),
+        openshift_master_etcd_hosts=["localhost"]
     )
 
     check = EtcdImageDataSize(execute_module, task_vars).run()

roles/openshift_master/tasks/update_etcd_client_urls.yml

@@ -2,7 +2,7 @@
 - yedit:
     src: "{{ openshift.common.config_base }}/master/master-config.yaml"
     key: 'etcdClientInfo.urls'
-    value: "{{ openshift.master.etcd_urls }}"
+    value: "{{ openshift_master_etcd_urls }}"
   notify:
   - restart master api
   - restart master controllers

roles/openshift_master/templates/master.yaml.v1.j2

@@ -76,7 +76,7 @@ etcdClientInfo:
   certFile: master.etcd-client.crt
   keyFile: master.etcd-client.key
   urls:
-{% for etcd_url in openshift.master.etcd_urls %}
+{% for etcd_url in openshift_master_etcd_urls %}
     - {{ etcd_url }}
 {% endfor %}
 etcdStorageConfig:

roles/openshift_master_certificates/tasks/main.yml

@@ -101,6 +101,7 @@
   file:
     path: "{{ openshift_master_generated_config_dir }}/{{ item }}"
     state: absent
+  # Do we need this boolean here?
   when: openshift_master_etcd_hosts | length > 0
   with_items:
   - master.etcd-client.crt

roles/openshift_master_facts/tasks/main.yml

@@ -41,10 +41,6 @@
       extension_scripts: "{{ openshift_master_extension_scripts | default(None) }}"
       extension_stylesheets: "{{ openshift_master_extension_stylesheets | default(None) }}"
       extensions: "{{ openshift_master_extensions | default(None) }}"
-      etcd_hosts: "{{ openshift_master_etcd_hosts | default(None) }}"
-      etcd_port: "{{ openshift_master_etcd_port | default(None) }}"
-      etcd_use_ssl: "{{ openshift_master_etcd_use_ssl | default(None) }}"
-      etcd_urls: "{{ openshift_master_etcd_urls | default(None) }}"
       embedded_kube: "{{ openshift_master_embedded_kube | default(None) }}"
       embedded_dns: "{{ openshift_master_embedded_dns | default(None) }}"
       bind_addr: "{{ openshift_master_bind_addr | default(None) }}"

roles/openshift_service_catalog/tasks/install.yml

@@ -118,7 +118,7 @@
     cpu_requests: none
     memory_request: none
     cors_allowed_origin: localhost
-    etcd_servers: "{{ openshift.master.etcd_urls | join(',') }}"
+    etcd_servers: "{{ openshift_master_etcd_urls | join(',') }}"
     etcd_cafile: "{{ '/etc/origin/master/master.etcd-ca.crt' if etcd_ca_crt.stat.exists else '/etc/origin/master/ca-bundle.crt' }}"
     node_selector: {'node-role.kubernetes.io/master': 'true'}
     # apiserver_ca is defined in generate_certs.yml