Merge pull request #1237 from detiber/infra_fixes

infra_node fixes

10 files changed, 151 insertions, 75 deletions

diff --git a/filter_plugins/oo_filters.py b/filter_plugins/oo_filters.py
index ae275b051..2b39bb59e 100644
--- a/filter_plugins/oo_filters.py
+++ b/filter_plugins/oo_filters.py
@@ -1,9 +1,9 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 # vim: expandtab:tabstop=4:shiftwidth=4
-'''
+"""
 Custom filters for use in openshift-ansible
-'''
+"""
 
 from ansible import errors
 from operator import itemgetter
@@ -15,26 +15,29 @@ import json
 import yaml
 from ansible.utils.unicode import to_unicode
 
+# Disabling too-many-public-methods, since filter methods are necessarily
+# public
+# pylint: disable=too-many-public-methods
 class FilterModule(object):
-    ''' Custom ansible filters '''
+    """ Custom ansible filters """
 
     @staticmethod
     def oo_pdb(arg):
-        ''' This pops you into a pdb instance where arg is the data passed in
+        """ This pops you into a pdb instance where arg is the data passed in
             from the filter.
             Ex: "{{ hostvars | oo_pdb }}"
-        '''
+        """
         pdb.set_trace()
         return arg
 
     @staticmethod
     def get_attr(data, attribute=None):
-        ''' This looks up dictionary attributes of the form a.b.c and returns
+        """ This looks up dictionary attributes of the form a.b.c and returns
             the value.
             Ex: data = {'a': {'b': {'c': 5}}}
                 attribute = "a.b.c"
                 returns 5
-        '''
+        """
         if not attribute:
             raise errors.AnsibleFilterError("|failed expects attribute to be set")
 
@@ -46,16 +49,16 @@ class FilterModule(object):
 
     @staticmethod
     def oo_flatten(data):
-        ''' This filter plugin will flatten a list of lists
-        '''
-        if not issubclass(type(data), list):
+        """ This filter plugin will flatten a list of lists
+        """
+        if not isinstance(data, list):
             raise errors.AnsibleFilterError("|failed expects to flatten a List")
 
         return [item for sublist in data for item in sublist]
 
     @staticmethod
     def oo_collect(data, attribute=None, filters=None):
-        ''' This takes a list of dict and collects all attributes specified into a
+        """ This takes a list of dict and collects all attributes specified into a
             list. If filter is specified then we will include all items that
             match _ALL_ of filters.  If a dict entry is missing the key in a
             filter it will be excluded from the match.
@@ -67,15 +70,15 @@ class FilterModule(object):
                 attribute = 'a'
                 filters   = {'z': 'z'}
                 returns [1, 2, 3]
-        '''
-        if not issubclass(type(data), list):
+        """
+        if not isinstance(data, list):
             raise errors.AnsibleFilterError("|failed expects to filter on a List")
 
         if not attribute:
             raise errors.AnsibleFilterError("|failed expects attribute to be set")
 
         if filters is not None:
-            if not issubclass(type(filters), dict):
+            if not isinstance(filters, dict):
                 raise errors.AnsibleFilterError("|failed expects filter to be a"
                                                 " dict")
             retval = [FilterModule.get_attr(d, attribute) for d in data if (
@@ -87,16 +90,16 @@ class FilterModule(object):
 
     @staticmethod
     def oo_select_keys_from_list(data, keys):
-        ''' This returns a list, which contains the value portions for the keys
+        """ This returns a list, which contains the value portions for the keys
             Ex: data = { 'a':1, 'b':2, 'c':3 }
                 keys = ['a', 'c']
                 returns [1, 3]
-        '''
+        """
 
-        if not issubclass(type(data), list):
+        if not isinstance(data, list):
             raise errors.AnsibleFilterError("|failed expects to filter on a list")
 
-        if not issubclass(type(keys), list):
+        if not isinstance(keys, list):
             raise errors.AnsibleFilterError("|failed expects first param is a list")
 
         # Gather up the values for the list of keys passed in
@@ -106,16 +109,16 @@ class FilterModule(object):
 
     @staticmethod
     def oo_select_keys(data, keys):
-        ''' This returns a list, which contains the value portions for the keys
+        """ This returns a list, which contains the value portions for the keys
             Ex: data = { 'a':1, 'b':2, 'c':3 }
                 keys = ['a', 'c']
                 returns [1, 3]
-        '''
+        """
 
-        if not issubclass(type(data), dict):
+        if not isinstance(data, dict):
             raise errors.AnsibleFilterError("|failed expects to filter on a dict")
 
-        if not issubclass(type(keys), list):
+        if not isinstance(keys, list):
             raise errors.AnsibleFilterError("|failed expects first param is a list")
 
         # Gather up the values for the list of keys passed in
@@ -125,13 +128,13 @@ class FilterModule(object):
 
     @staticmethod
     def oo_prepend_strings_in_list(data, prepend):
-        ''' This takes a list of strings and prepends a string to each item in the
+        """ This takes a list of strings and prepends a string to each item in the
             list
             Ex: data = ['cart', 'tree']
                 prepend = 'apple-'
                 returns ['apple-cart', 'apple-tree']
-        '''
-        if not issubclass(type(data), list):
+        """
+        if not isinstance(data, list):
             raise errors.AnsibleFilterError("|failed expects first param is a list")
         if not all(isinstance(x, basestring) for x in data):
             raise errors.AnsibleFilterError("|failed expects first param is a list"
@@ -141,10 +144,10 @@ class FilterModule(object):
 
     @staticmethod
     def oo_combine_key_value(data, joiner='='):
-        '''Take a list of dict in the form of { 'key': 'value'} and
+        """Take a list of dict in the form of { 'key': 'value'} and
            arrange them as a list of strings ['key=value']
-        '''
-        if not issubclass(type(data), list):
+        """
+        if not isinstance(data, list):
             raise errors.AnsibleFilterError("|failed expects first param is a list")
 
         rval = []
@@ -155,20 +158,20 @@ class FilterModule(object):
 
     @staticmethod
     def oo_combine_dict(data, in_joiner='=', out_joiner=' '):
-        '''Take a dict in the form of { 'key': 'value', 'key': 'value' } and
+        """Take a dict in the form of { 'key': 'value', 'key': 'value' } and
            arrange them as a string 'key=value key=value'
-        '''
-        if not issubclass(type(data), dict):
+        """
+        if not isinstance(data, dict):
             raise errors.AnsibleFilterError("|failed expects first param is a dict")
 
         return out_joiner.join([in_joiner.join([k, v]) for k, v in data.items()])
 
     @staticmethod
     def oo_ami_selector(data, image_name):
-        ''' This takes a list of amis and an image name and attempts to return
+        """ This takes a list of amis and an image name and attempts to return
             the latest ami.
-        '''
-        if not issubclass(type(data), list):
+        """
+        if not isinstance(data, list):
             raise errors.AnsibleFilterError("|failed expects first param is a list")
 
         if not data:
@@ -184,7 +187,7 @@ class FilterModule(object):
 
     @staticmethod
     def oo_ec2_volume_definition(data, host_type, docker_ephemeral=False):
-        ''' This takes a dictionary of volume definitions and returns a valid ec2
+        """ This takes a dictionary of volume definitions and returns a valid ec2
             volume definition based on the host_type and the values in the
             dictionary.
             The dictionary should look similar to this:
@@ -209,8 +212,8 @@ class FilterModule(object):
                         }
                     }
                 }
-        '''
-        if not issubclass(type(data), dict):
+        """
+        if not isinstance(data, dict):
             raise errors.AnsibleFilterError("|failed expects first param is a dict")
         if host_type not in ['master', 'node', 'etcd']:
             raise errors.AnsibleFilterError("|failed expects etcd, master or node"
@@ -243,15 +246,15 @@ class FilterModule(object):
 
     @staticmethod
     def oo_split(string, separator=','):
-        ''' This splits the input string into a list
-        '''
+        """ This splits the input string into a list
+        """
         return string.split(separator)
 
     @staticmethod
     def oo_haproxy_backend_masters(hosts):
-        ''' This takes an array of dicts and returns an array of dicts
+        """ This takes an array of dicts and returns an array of dicts
             to be used as a backend for the haproxy role
-        '''
+        """
         servers = []
         for idx, host_info in enumerate(hosts):
             server = dict(name="master%s" % idx)
@@ -264,7 +267,7 @@ class FilterModule(object):
 
     @staticmethod
     def oo_filter_list(data, filter_attr=None):
-        ''' This returns a list, which contains all items where filter_attr
+        """ This returns a list, which contains all items where filter_attr
             evaluates to true
             Ex: data = [ { a: 1, b: True },
                          { a: 3, b: False },
@@ -272,19 +275,81 @@ class FilterModule(object):
                 filter_attr = 'b'
                 returns [ { a: 1, b: True },
                           { a: 5, b: True } ]
-        '''
-        if not issubclass(type(data), list):
+        """
+        if not isinstance(data, list):
             raise errors.AnsibleFilterError("|failed expects to filter on a list")
 
-        if not issubclass(type(filter_attr), str):
-            raise errors.AnsibleFilterError("|failed expects filter_attr is a str")
+        if not isinstance(filter_attr, basestring):
+            raise errors.AnsibleFilterError("|failed expects filter_attr is a str or unicode")
 
         # Gather up the values for the list of keys passed in
         return [x for x in data if x.has_key(filter_attr) and x[filter_attr]]
 
     @staticmethod
+    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, basestring):
+            raise errors.AnsibleFilterError("failed expects label to be a string")
+        if value is not None and not isinstance(value, basestring):
+            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, basestring):
+                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)]
+
+
+    @staticmethod
     def oo_parse_heat_stack_outputs(data):
-        ''' Formats the HEAT stack output into a usable form
+        """ Formats the HEAT stack output into a usable form
 
             The goal is to transform something like this:
 
@@ -323,7 +388,7 @@ class FilterModule(object):
                 "value_B2"
               ]
             }
-        '''
+        """
 
         # Extract the “outputs” JSON snippet from the pretty-printed array
         in_outputs = False
@@ -352,7 +417,7 @@ class FilterModule(object):
     @staticmethod
     # pylint: disable=too-many-branches
     def oo_parse_named_certificates(certificates, named_certs_dir, internal_hostnames):
-        ''' Parses names from list of certificate hashes.
+        """ Parses names from list of certificate hashes.
 
             Ex: certificates = [{ "certfile": "/root/custom1.crt",
                                   "keyfile": "/root/custom1.key" },
@@ -366,11 +431,11 @@ class FilterModule(object):
                          { "certfile": "/etc/origin/master/named_certificates/custom2.crt",
                            "keyfile": "/etc/origin/master/named_certificates/custom2.key",
                            "names": [ "some-hostname.com" ] }]
-        '''
-        if not issubclass(type(named_certs_dir), unicode):
-            raise errors.AnsibleFilterError("|failed expects named_certs_dir is unicode")
+        """
+        if not isinstance(named_certs_dir, basestring):
+            raise errors.AnsibleFilterError("|failed expects named_certs_dir is str or unicode")
 
-        if not issubclass(type(internal_hostnames), list):
+        if not isinstance(internal_hostnames, list):
             raise errors.AnsibleFilterError("|failed expects internal_hostnames is list")
 
         for certificate in certificates:
@@ -410,7 +475,7 @@ class FilterModule(object):
 
     @staticmethod
     def oo_pretty_print_cluster(data):
-        ''' Read a subset of hostvars and build a summary of the cluster
+        """ Read a subset of hostvars and build a summary of the cluster
             in the following layout:
 
 "c_id": {
@@ -427,14 +492,14 @@ class FilterModule(object):
   ...
     ]
   }
-        '''
+        """
 
         def _get_tag_value(tags, key):
-            ''' Extract values of a map implemented as a set.
+            """ Extract values of a map implemented as a set.
                 Ex: tags = { 'tag_foo_value1', 'tag_bar_value2', 'tag_baz_value3' }
                     key = 'bar'
                     returns 'value2'
-            '''
+            """
             for tag in tags:
                 if tag[:len(key)+4] == 'tag_' + key:
                     return tag[len(key)+5:]
@@ -445,7 +510,7 @@ class FilterModule(object):
                       host_type,
                       sub_host_type,
                       host):
-            ''' Add a new host in the clusters data structure '''
+            """ Add a new host in the clusters data structure """
             if clusterid not in clusters:
                 clusters[clusterid] = {}
             if host_type not in clusters[clusterid]:
@@ -470,9 +535,9 @@ class FilterModule(object):
 
     @staticmethod
     def oo_generate_secret(num_bytes):
-        ''' generate a session secret '''
+        """ generate a session secret """
 
-        if not issubclass(type(num_bytes), int):
+        if not isinstance(num_bytes, int):
             raise errors.AnsibleFilterError("|failed expects num_bytes is int")
 
         secret = os.urandom(num_bytes)
@@ -480,7 +545,7 @@ class FilterModule(object):
 
     @staticmethod
     def to_padded_yaml(data, level=0, indent=2, **kw):
-        ''' returns a yaml snippet padded to match the indent level you specify '''
+        """ returns a yaml snippet padded to match the indent level you specify """
         if data in [None, ""]:
             return ""
 
@@ -492,7 +557,7 @@ class FilterModule(object):
             raise errors.AnsibleFilterError('Failed to convert: %s', my_e)
 
     def filters(self):
-        ''' returns a mapping of filters to methods '''
+        """ returns a mapping of filters to methods """
         return {
             "oo_select_keys": self.oo_select_keys,
             "oo_select_keys_from_list": self.oo_select_keys_from_list,
@@ -512,4 +577,5 @@ class FilterModule(object):
             "oo_pretty_print_cluster": self.oo_pretty_print_cluster,
             "oo_generate_secret": self.oo_generate_secret,
             "to_padded_yaml": self.to_padded_yaml,
+            "oo_nodes_with_label": self.oo_nodes_with_label,
         }
diff --git a/playbooks/aws/openshift-cluster/config.yml b/playbooks/aws/openshift-cluster/config.yml
index abdb23d78..0b6edd70b 100644
--- a/playbooks/aws/openshift-cluster/config.yml
+++ b/playbooks/aws/openshift-cluster/config.yml
@@ -1,4 +1,3 @@
----
 - include: ../../common/openshift-cluster/config.yml
   vars_files:
   - ../../aws/openshift-cluster/vars.yml
@@ -12,3 +11,6 @@
     openshift_deployment_type: "{{ deployment_type }}"
     openshift_hostname: "{{ ec2_private_ip_address }}"
     openshift_public_hostname: "{{ ec2_ip_address }}"
+    openshift_router_selector: 'type=infra'
+    openshift_infra_nodes: "{{ g_infra_hosts }}"
+    openshift_node_labels: '{"region": "{{ ec2_region }}", "type": "{{ hostvars[inventory_hostname]["ec2_tag_sub-host-type"] if inventory_hostname in groups["tag_host-type_node"] else hostvars[inventory_hostname]["ec2_tag_host-type"] }}"}'
diff --git a/playbooks/aws/openshift-cluster/terminate.yml b/playbooks/aws/openshift-cluster/terminate.yml
index 4b9c80b14..c20f370bf 100644
--- a/playbooks/aws/openshift-cluster/terminate.yml
+++ b/playbooks/aws/openshift-cluster/terminate.yml
@@ -74,4 +74,4 @@
         tags:
           Name: "{{ item.item.item.ec2_tag_Name }}-terminate"
       with_items: ec2_stop.results
-      when: "'oo_hosts_to_terminate' in groups"
+      when: ec2_stop | changed
diff --git a/playbooks/common/openshift-master/config.yml b/playbooks/common/openshift-master/config.yml
index 12497bf94..6f86703d6 100644
--- a/playbooks/common/openshift-master/config.yml
+++ b/playbooks/common/openshift-master/config.yml
@@ -164,6 +164,11 @@
                                 | list ) }}"
       master_cert_subdir: master-{{ openshift.common.hostname }}
       master_cert_config_dir: "{{ openshift.common.config_base }}/master"
+  - set_fact:
+      openshift_infra_nodes: "{{ hostvars | oo_select_keys(groups['nodes'])
+                                 | oo_nodes_with_label('region', 'infra')
+                                 | oo_collect('inventory_hostname') }}"
+    when: openshift_infra_nodes is not defined
 
 - name: Configure master certificates
   hosts: oo_first_master
diff --git a/playbooks/gce/openshift-cluster/config.yml b/playbooks/gce/openshift-cluster/config.yml
index 84a3f84d4..80095d072 100644
--- a/playbooks/gce/openshift-cluster/config.yml
+++ b/playbooks/gce/openshift-cluster/config.yml
@@ -13,3 +13,5 @@
     openshift_debug_level: "{{ debug_level }}"
     openshift_deployment_type: "{{ deployment_type }}"
     openshift_hostname: "{{ gce_private_ip }}"
+    openshift_router_selector: 'type=infra'
+    openshift_infra_nodes: "{{ g_infra_hosts }}"
diff --git a/playbooks/libvirt/openshift-cluster/config.yml b/playbooks/libvirt/openshift-cluster/config.yml
index be9cbbfaa..b5cda6187 100644
--- a/playbooks/libvirt/openshift-cluster/config.yml
+++ b/playbooks/libvirt/openshift-cluster/config.yml
@@ -13,3 +13,5 @@
     openshift_cluster_id: "{{ cluster_id }}"
     openshift_debug_level: "{{ debug_level }}"
     openshift_deployment_type: "{{ deployment_type }}"
+    openshift_router_selector: 'type=infra'
+    openshift_infra_nodes: "{{ g_infra_hosts }}"
diff --git a/playbooks/openstack/openshift-cluster/config.yml b/playbooks/openstack/openshift-cluster/config.yml
index b338d2eb4..6618c6a7f 100644
--- a/playbooks/openstack/openshift-cluster/config.yml
+++ b/playbooks/openstack/openshift-cluster/config.yml
@@ -11,3 +11,5 @@
     openshift_debug_level: "{{ debug_level }}"
     openshift_deployment_type: "{{ deployment_type }}"
     openshift_hostname: "{{ ansible_default_ipv4.address }}"
+    openshift_router_selector: 'type=infra'
+    openshift_infra_nodes: "{{ g_infra_hosts }}"
diff --git a/roles/openshift_master/tasks/main.yml b/roles/openshift_master/tasks/main.yml
index 57b50bee4..aa5e593b6 100644
--- a/roles/openshift_master/tasks/main.yml
+++ b/roles/openshift_master/tasks/main.yml
@@ -82,7 +82,7 @@
       registry_selector: "{{ openshift_registry_selector | default(None) }}"
       api_server_args: "{{ osm_api_server_args | default(None) }}"
       controller_args: "{{ osm_controller_args | default(None) }}"
-      infra_nodes: "{{ num_infra | default(None) }}"
+      infra_nodes: "{{ openshift_infra_nodes | default(None) }}"
       disabled_features: "{{ osm_disabled_features | default(None) }}"
       master_count: "{{ openshift_master_count | default(None) }}"
       controller_lease_ttl: "{{ osm_controller_lease_ttl | default(None) }}"
diff --git a/roles/openshift_router/tasks/main.yml b/roles/openshift_router/tasks/main.yml
index 498a65127..355cbf84b 100644
--- a/roles/openshift_router/tasks/main.yml
+++ b/roles/openshift_router/tasks/main.yml
@@ -1,14 +1,9 @@
 ---
-
-- set_fact: _ortr_images="--images='{{ openshift.master.registry_url }}'"
-
-- set_fact: _ortr_selector="--selector='{{ openshift.master.router_selector }}'"
-
 - name: Deploy OpenShift Router
   command: >
     {{ openshift.common.admin_binary }} router
-    --create --replicas={{ openshift.master.infra_nodes }}
-    --service-account=router {{ _ortr_selector }}
-    --credentials={{ openshift_master_config_dir }}/openshift-router.kubeconfig {{ _ortr_images }}
-  register: _ortr_results
-  changed_when: "'service exists' not in _ortr_results.stdout"
+    --create --replicas={{ openshift.master.infra_nodes | length }}
+    --service-account=router {{ ortr_selector }}
+    --credentials={{ openshift_master_config_dir }}/openshift-router.kubeconfig {{ ortr_images }}
+  register: ortr_results
+  changed_when: "'service exists' not in ortr_results.stdout"
diff --git a/roles/openshift_router/vars/main.yml b/roles/openshift_router/vars/main.yml
index 9967e26f4..bcac12068 100644
--- a/roles/openshift_router/vars/main.yml
+++ b/roles/openshift_router/vars/main.yml
@@ -1,2 +1,4 @@
 ---
 openshift_master_config_dir: "{{ openshift.common.config_base }}/master"
+ortr_images: "--images='{{ openshift.master.registry_url }}'"
+ortr_selector: "--selector='{{ openshift.master.router_selector }}'"