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

Remove openshift_node_facts part 1

12 files changed, 81 insertions, 167 deletions

diff --git a/filter_plugins/oo_filters.py b/filter_plugins/oo_filters.py
index 3eaf2aed5..07f0f4293 100644
--- a/filter_plugins/oo_filters.py
+++ b/filter_plugins/oo_filters.py
@@ -265,6 +265,18 @@ def oo_combine_dict(data, in_joiner='=', out_joiner=' '):
     return out_joiner.join([in_joiner.join([k, str(v)]) for k, v in data.items()])
 
 
+def oo_dict_to_keqv_list(data):
+    """Take a dict and return a list of k=v pairs
+
+        Input data:
+        {'a': 1, 'b': 2}
+
+        Return data:
+        ['a=1', 'b=2']
+    """
+    return ['='.join(str(e) for e in x) for x in data.items()]
+
+
 def oo_dict_to_list_of_dict(data, key_title='key', value_title='value'):
     """Take a dict and arrange them as a list of dicts
 
@@ -416,67 +428,6 @@ def oo_filter_list(data, filter_attr=None):
     return [x for x in data if filter_attr in x and x[filter_attr]]
 
 
-def oo_nodes_with_label(nodes, label, value=None):
-    """ Filters a list of nodes by label and value (if provided)
-
-        It handles labels that are in the following variables by priority:
-        openshift_node_labels, cli_openshift_node_labels, openshift['node']['labels']
-
-        Examples:
-            data = ['a': {'openshift_node_labels': {'color': 'blue', 'size': 'M'}},
-                    'b': {'openshift_node_labels': {'color': 'green', 'size': 'L'}},
-                    'c': {'openshift_node_labels': {'size': 'S'}}]
-            label = 'color'
-            returns = ['a': {'openshift_node_labels': {'color': 'blue', 'size': 'M'}},
-                       'b': {'openshift_node_labels': {'color': 'green', 'size': 'L'}}]
-
-            data = ['a': {'openshift_node_labels': {'color': 'blue', 'size': 'M'}},
-                    'b': {'openshift_node_labels': {'color': 'green', 'size': 'L'}},
-                    'c': {'openshift_node_labels': {'size': 'S'}}]
-            label = 'color'
-            value = 'green'
-            returns = ['b': {'labels': {'color': 'green', 'size': 'L'}}]
-
-        Args:
-            nodes (list[dict]): list of node to node variables
-            label (str): label to filter `nodes` by
-            value (Optional[str]): value of `label` to filter by Defaults
-                                   to None.
-
-        Returns:
-            list[dict]: nodes filtered by label and value (if provided)
-    """
-    if not isinstance(nodes, list):
-        raise errors.AnsibleFilterError("failed expects to filter on a list")
-    if not isinstance(label, string_types):
-        raise errors.AnsibleFilterError("failed expects label to be a string")
-    if value is not None and not isinstance(value, string_types):
-        raise errors.AnsibleFilterError("failed expects value to be a string")
-
-    def label_filter(node):
-        """ filter function for testing if node should be returned """
-        if not isinstance(node, dict):
-            raise errors.AnsibleFilterError("failed expects to filter on a list of dicts")
-        if 'openshift_node_labels' in node:
-            labels = node['openshift_node_labels']
-        elif 'cli_openshift_node_labels' in node:
-            labels = node['cli_openshift_node_labels']
-        elif 'openshift' in node and 'node' in node['openshift'] and 'labels' in node['openshift']['node']:
-            labels = node['openshift']['node']['labels']
-        else:
-            return False
-
-        if isinstance(labels, string_types):
-            labels = yaml.safe_load(labels)
-        if not isinstance(labels, dict):
-            raise errors.AnsibleFilterError(
-                "failed expected node labels to be a dict or serializable to a dict"
-            )
-        return label in labels and (value is None or labels[label] == value)
-
-    return [n for n in nodes if label_filter(n)]
-
-
 def oo_parse_heat_stack_outputs(data):
     """ Formats the HEAT stack output into a usable form
 
@@ -974,6 +925,7 @@ class FilterModule(object):
             "oo_ec2_volume_definition": oo_ec2_volume_definition,
             "oo_combine_key_value": oo_combine_key_value,
             "oo_combine_dict": oo_combine_dict,
+            "oo_dict_to_keqv_list": oo_dict_to_keqv_list,
             "oo_dict_to_list_of_dict": oo_dict_to_list_of_dict,
             "oo_split": oo_split,
             "oo_list_to_dict": oo_list_to_dict,
@@ -983,7 +935,6 @@ class FilterModule(object):
             "oo_haproxy_backend_masters": oo_haproxy_backend_masters,
             "oo_pretty_print_cluster": oo_pretty_print_cluster,
             "oo_generate_secret": oo_generate_secret,
-            "oo_nodes_with_label": oo_nodes_with_label,
             "oo_31_rpm_rename_conversion": oo_31_rpm_rename_conversion,
             "oo_pods_match_component": oo_pods_match_component,
             "oo_get_hosts_from_hostvars": oo_get_hosts_from_hostvars,
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) }}"