Merge pull request #6456 from mgugino-upstream-stage/node-facts

Remove openshift_node_facts part 1

11 files changed, 68 insertions, 105 deletions

diff --git a/roles/openshift_facts/library/openshift_facts.py b/roles/openshift_facts/library/openshift_facts.py
index a10ba9310..58e64994f 100755
--- a/roles/openshift_facts/library/openshift_facts.py
+++ b/roles/openshift_facts/library/openshift_facts.py
@@ -536,8 +536,7 @@ def set_aggregate_facts(facts):
 
 def set_deployment_facts_if_unset(facts):
     """ Set Facts that vary based on deployment_type. This currently
-        includes master.registry_url, node.registry_url,
-        node.storage_plugin_deps
+        includes master.registry_url, node.registry_url
 
         Args:
             facts (dict): existing facts
@@ -564,11 +563,6 @@ def set_deployment_facts_if_unset(facts):
             if facts['common']['deployment_subtype'] == 'registry':
                 facts['master']['disabled_features'] = openshift_features
 
-    if 'node' in facts:
-        deployment_type = facts['common']['deployment_type']
-        if 'storage_plugin_deps' not in facts['node']:
-            facts['node']['storage_plugin_deps'] = ['ceph', 'glusterfs', 'iscsi']
-
     return facts
 
 
@@ -792,62 +786,6 @@ def get_current_config(facts):
     return current_config
 
 
-def build_kubelet_args(facts):
-    """Build node kubelet_args
-
-In the node-config.yaml file, kubeletArgument sub-keys have their
-values provided as a list. Hence the gratuitous use of ['foo'] below.
-    """
-    cloud_cfg_path = os.path.join(
-        facts['common']['config_base'],
-        'cloudprovider')
-
-    # We only have to do this stuff on hosts that are nodes
-    if 'node' in facts:
-        # Any changes to the kubeletArguments parameter are stored
-        # here first.
-        kubelet_args = {}
-
-        if 'cloudprovider' in facts:
-            # EVERY cloud is special <3
-            if 'kind' in facts['cloudprovider']:
-                if facts['cloudprovider']['kind'] == 'aws':
-                    kubelet_args['cloud-provider'] = ['aws']
-                    kubelet_args['cloud-config'] = [cloud_cfg_path + '/aws.conf']
-                if facts['cloudprovider']['kind'] == 'openstack':
-                    kubelet_args['cloud-provider'] = ['openstack']
-                    kubelet_args['cloud-config'] = [cloud_cfg_path + '/openstack.conf']
-                if facts['cloudprovider']['kind'] == 'gce':
-                    kubelet_args['cloud-provider'] = ['gce']
-                    kubelet_args['cloud-config'] = [cloud_cfg_path + '/gce.conf']
-
-        # Automatically add node-labels to the kubeletArguments
-        # parameter. See BZ1359848 for additional details.
-        #
-        # Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1359848
-        if 'labels' in facts['node'] and isinstance(facts['node']['labels'], dict):
-            # tl;dr: os_node_labels="{'foo': 'bar', 'a': 'b'}" turns
-            # into ['foo=bar', 'a=b']
-            #
-            # On the openshift_node_labels inventory variable we loop
-            # over each key-value tuple (from .items()) and join the
-            # key to the value with an '=' character, this produces a
-            # list.
-            #
-            # map() seems to be returning an itertools.imap object
-            # instead of a list. We cast it to a list ourselves.
-            # pylint: disable=unnecessary-lambda
-            labels_str = list(map(lambda x: '='.join(x), facts['node']['labels'].items()))
-            if labels_str != '':
-                kubelet_args['node-labels'] = labels_str
-
-        # If we've added items to the kubelet_args dict then we need
-        # to merge the new items back into the main facts object.
-        if kubelet_args != {}:
-            facts = merge_facts({'node': {'kubelet_args': kubelet_args}}, facts, [])
-    return facts
-
-
 def build_controller_args(facts):
     """ Build master controller_args """
     cloud_cfg_path = os.path.join(facts['common']['config_base'],
@@ -1367,7 +1305,6 @@ def set_container_facts_if_unset(facts):
     deployment_type = facts['common']['deployment_type']
     if deployment_type == 'openshift-enterprise':
         master_image = 'openshift3/ose'
-        node_image = 'openshift3/node'
         ovs_image = 'openshift3/openvswitch'
         pod_image = 'openshift3/ose-pod'
         router_image = 'openshift3/ose-haproxy-router'
@@ -1375,7 +1312,6 @@ def set_container_facts_if_unset(facts):
         deployer_image = 'openshift3/ose-deployer'
     else:
         master_image = 'openshift/origin'
-        node_image = 'openshift/node'
         ovs_image = 'openshift/openvswitch'
         pod_image = 'openshift/origin-pod'
         router_image = 'openshift/origin-haproxy-router'
@@ -1398,9 +1334,6 @@ def set_container_facts_if_unset(facts):
         facts['master']['master_image'] = master_image
         facts['master']['master_system_image'] = master_image
     if 'node' in facts:
-        if 'node_image' not in facts['node']:
-            facts['node']['node_image'] = node_image
-            facts['node']['node_system_image'] = node_image
         if 'ovs_image' not in facts['node']:
             facts['node']['ovs_image'] = ovs_image
             facts['node']['ovs_system_image'] = ovs_image
@@ -1545,7 +1478,6 @@ class OpenShiftFacts(object):
         facts = set_deployment_facts_if_unset(facts)
         facts = set_sdn_facts_if_unset(facts, self.system_facts)
         facts = set_container_facts_if_unset(facts)
-        facts = build_kubelet_args(facts)
         facts = build_controller_args(facts)
         facts = build_api_server_args(facts)
         facts = set_version_facts_if_unset(facts)
@@ -1607,10 +1539,7 @@ class OpenShiftFacts(object):
                                       max_requests_inflight=500)
 
         if 'node' in roles:
-            defaults['node'] = dict(labels={}, annotations={},
-                                    iptables_sync_period='30s',
-                                    local_quota_per_fsgroup="",
-                                    set_node_ip=False)
+            defaults['node'] = dict(labels={}, annotations={})
 
         if 'cloudprovider' in roles:
             defaults['cloudprovider'] = dict(kind=None)
diff --git a/roles/openshift_manage_node/tasks/main.yml b/roles/openshift_manage_node/tasks/main.yml
index a15f336e4..af22a1a03 100644
--- a/roles/openshift_manage_node/tasks/main.yml
+++ b/roles/openshift_manage_node/tasks/main.yml
@@ -50,10 +50,9 @@
     name: "{{ openshift.node.nodename }}"
     kind: node
     state: add
-    labels: "{{ openshift.node.labels | oo_dict_to_list_of_dict }}"
+    labels: "{{ openshift_node_labels | oo_dict_to_list_of_dict }}"
     namespace: default
   when:
     - "'nodename' in openshift.node"
-    - "'labels' in openshift.node"
-    - openshift.node.labels != {}
+    - openshift_node_labels | default({}) != {}
   delegate_to: "{{ openshift_master_host }}"
diff --git a/roles/openshift_node/defaults/main.yml b/roles/openshift_node/defaults/main.yml
index fff927944..906c76fba 100644
--- a/roles/openshift_node/defaults/main.yml
+++ b/roles/openshift_node/defaults/main.yml
@@ -1,6 +1,48 @@
 ---
 openshift_node_debug_level: "{{ debug_level | default(2) }}"
-
+openshift_node_iptables_sync_period: '30s'
+osn_storage_plugin_deps:
+- ceph
+- glusterfs
+- iscsi
+openshift_node_local_quota_per_fsgroup: ""
+openshift_node_proxy_mode: iptables
+openshift_set_node_ip: False
+openshift_config_base: '/etc/origin'
+
+# Create list of 'k=v' pairs.
+l_node_kubelet_node_labels: "{{ openshift_node_labels | default({}) | oo_dict_to_keqv_list }}"
+
+openshift_node_kubelet_args_dict:
+  aws:
+    cloud-provider:
+    - aws
+    cloud-config:
+    - "{{ openshift_config_base ~ '/aws.conf' }}"
+    node-labels: "{{ l_node_kubelet_node_labels }}"
+  openstack:
+    cloud-provider:
+    - openstack
+    cloud-config:
+    - "{{ openshift_config_base ~ '/openstack.conf' }}"
+    node-labels: "{{ l_node_kubelet_node_labels }}"
+  gce:
+    cloud-provider:
+    - gce
+    cloud-config:
+    - "{{ openshift_config_base ~ '/gce.conf' }}"
+    node-labels: "{{ l_node_kubelet_node_labels }}"
+  undefined:
+    node-labels: "{{ l_node_kubelet_node_labels }}"
+
+l_node_kubelet_args_default: "{{ openshift_node_kubelet_args_dict[openshift_cloudprovider_kind | default('undefined')] }}"
+
+l_openshift_node_kubelet_args: "{{ openshift_node_kubelet_args | default({}) }}"
+# Combine the default kubelet_args dictionary (based on cloud provider, if provided)
+# with user-supplied openshift_node_kubelet_args.
+# openshift_node_kubelet_args will override the defaults, if keys and/or subkeys
+# are present in both.
+l2_openshift_node_kubelet_args: "{{ l_node_kubelet_args_default | combine(l_openshift_node_kubelet_args, recursive=True) }}"
 openshift_node_dnsmasq_install_network_manager_hook: true
 
 # lo must always be present in this list or dnsmasq will conflict with
@@ -14,10 +56,15 @@ r_openshift_node_use_firewalld: "{{ os_firewall_use_firewalld | default(False) }
 l_is_node_system_container: "{{ (openshift_use_node_system_container | default(openshift_use_system_containers | default(false)) | bool) }}"
 
 openshift_deployment_type: "{{ openshift_deployment_type | default('origin') }}"
+
+openshift_node_image_dict:
+  origin: 'openshift/node'
+  openshift-enterprise: 'openshift3/node'
+osn_image: "{{ openshift_node_image_dict[openshift_deployment_type] }}"
+
 openshift_service_type_dict:
   origin: origin
   openshift-enterprise: atomic-openshift
-
 openshift_service_type: "{{ openshift_service_type_dict[openshift_deployment_type] }}"
 
 system_images_registry_dict:
diff --git a/roles/openshift_node/tasks/config.yml b/roles/openshift_node/tasks/config.yml
index 8a55cd428..a96785d5c 100644
--- a/roles/openshift_node/tasks/config.yml
+++ b/roles/openshift_node/tasks/config.yml
@@ -24,9 +24,9 @@
     ovs_service_status_changed: "{{ ovs_start_result is changed }}"
 
 - file:
-    dest: "{{ (openshift_node_kubelet_args|default({'config':None})).config}}"
+    dest: "{{ l2_openshift_node_kubelet_args['config'] }}"
     state: directory
-  when: openshift_node_kubelet_args is defined and 'config' in openshift_node_kubelet_args
+  when: ('config' in l2_openshift_node_kubelet_args) | bool
 
 # TODO: add the validate parameter when there is a validation command to run
 - name: Create the Node config
diff --git a/roles/openshift_node/tasks/install.yml b/roles/openshift_node/tasks/install.yml
index b1fcf4068..9f004e8dd 100644
--- a/roles/openshift_node/tasks/install.yml
+++ b/roles/openshift_node/tasks/install.yml
@@ -30,6 +30,6 @@
   block:
   - name: Pre-pull node image when containerized
     command: >
-      docker pull {{ openshift.node.node_image }}:{{ openshift_image_tag }}
+      docker pull {{ osn_image }}:{{ openshift_image_tag }}
     register: pull_result
     changed_when: "'Downloaded newer image' in pull_result.stdout"
diff --git a/roles/openshift_node/tasks/main.yml b/roles/openshift_node/tasks/main.yml
index 8bd8f2536..2daa6c75f 100644
--- a/roles/openshift_node/tasks/main.yml
+++ b/roles/openshift_node/tasks/main.yml
@@ -85,15 +85,15 @@
 
 - name: GlusterFS storage plugin configuration
   include_tasks: storage_plugins/glusterfs.yml
-  when: "'glusterfs' in openshift.node.storage_plugin_deps"
+  when: "'glusterfs' in osn_storage_plugin_deps"
 
 - name: Ceph storage plugin configuration
   include_tasks: storage_plugins/ceph.yml
-  when: "'ceph' in openshift.node.storage_plugin_deps"
+  when: "'ceph' in osn_storage_plugin_deps"
 
 - name: iSCSI storage plugin configuration
   include_tasks: storage_plugins/iscsi.yml
-  when: "'iscsi' in openshift.node.storage_plugin_deps"
+  when: "'iscsi' in osn_storage_plugin_deps"
 
 ##### END Storage #####
 
diff --git a/roles/openshift_node/tasks/node_system_container.yml b/roles/openshift_node/tasks/node_system_container.yml
index 98978ec6f..06b879050 100644
--- a/roles/openshift_node/tasks/node_system_container.yml
+++ b/roles/openshift_node/tasks/node_system_container.yml
@@ -2,14 +2,14 @@
 
 - name: Pre-pull node system container image
   command: >
-    atomic pull --storage=ostree {{ 'docker:' if system_images_registry == 'docker' else system_images_registry + '/' }}{{ openshift.node.node_system_image }}:{{ openshift_image_tag }}
+    atomic pull --storage=ostree {{ 'docker:' if system_images_registry == 'docker' else system_images_registry + '/' }}{{ osn_image }}:{{ openshift_image_tag }}
   register: pull_result
   changed_when: "'Pulling layer' in pull_result.stdout"
 
 - name: Install or Update node system container
   oc_atomic_container:
     name: "{{ openshift_service_type }}-node"
-    image: "{{ 'docker:' if system_images_registry == 'docker' else system_images_registry + '/' }}{{ openshift.node.node_system_image }}:{{ openshift_image_tag }}"
+    image: "{{ 'docker:' if system_images_registry == 'docker' else system_images_registry + '/' }}{{ osn_image }}:{{ openshift_image_tag }}"
     values:
     - "DNS_DOMAIN={{ openshift.common.dns_domain }}"
     - "DOCKER_SERVICE={{ openshift_docker_service_name }}.service"
diff --git a/roles/openshift_node/tasks/upgrade/containerized_upgrade_pull.yml b/roles/openshift_node/tasks/upgrade/containerized_upgrade_pull.yml
index 71f00dcd2..3e7455bbb 100644
--- a/roles/openshift_node/tasks/upgrade/containerized_upgrade_pull.yml
+++ b/roles/openshift_node/tasks/upgrade/containerized_upgrade_pull.yml
@@ -1,7 +1,7 @@
 ---
 - name: Pre-pull node image
   command: >
-    docker pull {{ openshift.node.node_image }}:{{ openshift_image_tag }}
+    docker pull {{ osn_image }}:{{ openshift_image_tag }}
   register: pull_result
   changed_when: "'Downloaded newer image' in pull_result.stdout"
 
diff --git a/roles/openshift_node/templates/node.yaml.v1.j2 b/roles/openshift_node/templates/node.yaml.v1.j2
index 261cac6f1..b673733df 100644
--- a/roles/openshift_node/templates/node.yaml.v1.j2
+++ b/roles/openshift_node/templates/node.yaml.v1.j2
@@ -10,12 +10,12 @@ dnsIP: {{ openshift.node.dns_ip }}
 {% endif %}
 dockerConfig:
   execHandlerName: ""
-iptablesSyncPeriod: "{{ openshift.node.iptables_sync_period }}"
+iptablesSyncPeriod: "{{ openshift_node_iptables_sync_period }}"
 imageConfig:
   format: {{ openshift.node.registry_url }}
   latest: {{ openshift_node_image_config_latest }}
 kind: NodeConfig
-kubeletArguments: {{ openshift.node.kubelet_args | default(None) | to_padded_yaml(level=1) }}
+kubeletArguments: {{ l2_openshift_node_kubelet_args | default(None) | to_padded_yaml(level=1) }}
 {% if openshift_use_crio %}
   container-runtime:
   - remote
@@ -45,7 +45,7 @@ networkConfig:
 {% if openshift_node_use_openshift_sdn | bool or openshift_node_use_nuage | bool or openshift_node_use_contiv | bool or openshift_node_use_kuryr | bool or openshift_node_sdn_network_plugin_name == 'cni' %}
    networkPluginName: {{ openshift_node_sdn_network_plugin_name }}
 {% endif %}
-{% if openshift.node.set_node_ip | bool %}
+{% if openshift_set_node_ip | bool %}
 nodeIP: {{ openshift.common.ip }}
 {% endif %}
 nodeName: {{ openshift.node.nodename }}
@@ -68,8 +68,8 @@ volumeDirectory: {{ openshift_node_data_dir }}/openshift.local.volumes
 {% if not (openshift_node_use_kuryr | default(False)) | bool %}
 proxyArguments:
   proxy-mode:
-     - {{ openshift.node.proxy_mode }}
+     - {{ openshift_node_proxy_mode }}
 {% endif %}
 volumeConfig:
   localQuota:
-    perFSGroup: {{ openshift.node.local_quota_per_fsgroup }}
+    perFSGroup: {{ openshift_node_local_quota_per_fsgroup }}
diff --git a/roles/openshift_node/templates/openshift.docker.node.service b/roles/openshift_node/templates/openshift.docker.node.service
index b174c7023..ae7b147a6 100644
--- a/roles/openshift_node/templates/openshift.docker.node.service
+++ b/roles/openshift_node/templates/openshift.docker.node.service
@@ -38,7 +38,7 @@ ExecStart=/usr/bin/docker run --name {{ openshift_service_type }}-node \
   {% if openshift_use_nuage | default(false) -%} $NUAGE_ADDTL_BIND_MOUNTS {% endif -%} \
   -v /dev:/dev $DOCKER_ADDTL_BIND_MOUNTS -v /etc/pki:/etc/pki:ro \
   {% if l_bind_docker_reg_auth %} -v {{ oreg_auth_credentials_path }}:/root/.docker:ro{% endif %}\
-  {{ openshift.node.node_image }}:${IMAGE_VERSION}
+  {{ osn_image }}:${IMAGE_VERSION}
 ExecStartPost=/usr/bin/sleep 10
 ExecStop=/usr/bin/docker stop {{ openshift_service_type }}-node
 ExecStopPost=/usr/bin/rm /etc/dnsmasq.d/node-dnsmasq.conf
diff --git a/roles/openshift_node_facts/tasks/main.yml b/roles/openshift_node_facts/tasks/main.yml
index c234a3000..2ea7a4cec 100644
--- a/roles/openshift_node_facts/tasks/main.yml
+++ b/roles/openshift_node_facts/tasks/main.yml
@@ -4,22 +4,10 @@
     role: "{{ item.role }}"
     local_facts: "{{ item.local_facts }}"
   with_items:
-  # Reset node labels to an empty dictionary.
-  - role: node
-    local_facts:
-      labels: {}
   - role: node
     local_facts:
       annotations: "{{ openshift_node_annotations | default(none) }}"
-      iptables_sync_period: "{{ openshift_node_iptables_sync_period | default(None) }}"
-      kubelet_args: "{{ openshift_node_kubelet_args | default(None) }}"
-      labels: "{{ openshift_node_labels | default(None) }}"
       registry_url: "{{ oreg_url_node | default(oreg_url) | default(None) }}"
-      storage_plugin_deps: "{{ osn_storage_plugin_deps | default(None) }}"
-      set_node_ip: "{{ openshift_set_node_ip | default(None) }}"
-      node_image: "{{ osn_image | default(None) }}"
       ovs_image: "{{ osn_ovs_image | default(None) }}"
-      proxy_mode: "{{ openshift_node_proxy_mode | default('iptables') }}"
-      local_quota_per_fsgroup: "{{ openshift_node_local_quota_per_fsgroup | default(None) }}"
       dns_ip: "{{ openshift_dns_ip | default(none) | node_get_dns_ip(hostvars[inventory_hostname])}}"
       env_vars: "{{ openshift_node_env_vars | default(None) }}"