Merge pull request #1766 from abutcher/router-selector

Bug 1328119 - router selector not obeyed

8 files changed, 51 insertions, 17 deletions

diff --git a/filter_plugins/oo_filters.py b/filter_plugins/oo_filters.py
index f6cc2edde..57c14e9a3 100644
--- a/filter_plugins/oo_filters.py
+++ b/filter_plugins/oo_filters.py
@@ -311,6 +311,16 @@ class FilterModule(object):
                           "color": "red"}}}]
                 selector = 'color=green'
                 returns = ['node1.example.com']
+
+                nodes = [{"kind": "Node", "metadata": {"name": "node1.example.com",
+                          "labels": {"kubernetes.io/hostname": "node1.example.com",
+                          "color": "green"}}},
+                         {"kind": "Node", "metadata": {"name": "node2.example.com",
+                          "labels": {"kubernetes.io/hostname": "node2.example.com",
+                          "color": "red"}}}]
+                selector = 'color=green,color=red'
+                returns = ['node1.example.com','node2.example.com']
+
             Args:
                 nodes (list[dict]): list of node definitions
                 selector (str): "label=value" node selector to filter `nodes` by
@@ -323,9 +333,15 @@ class FilterModule(object):
             raise errors.AnsibleFilterError("failed expects selector to be a string")
         if not re.match('.*=.*', selector):
             raise errors.AnsibleFilterError("failed selector does not match \"label=value\" format")
-        label = selector.split('=')[0]
-        value = selector.split('=')[1]
-        return FilterModule.oo_oc_nodes_with_label(nodes, label, value)
+        node_lists = []
+        for node_selector in ''.join(selector.split()).split(','):
+            label = node_selector.split('=')[0]
+            value = node_selector.split('=')[1]
+            node_lists.append(FilterModule.oo_oc_nodes_with_label(nodes, label, value))
+        nodes = set(node_lists[0])
+        for node_list in node_lists[1:]:
+            nodes.intersection_update(node_list)
+        return list(nodes)
 
     @staticmethod
     def oo_oc_nodes_with_label(nodes, label, value):
@@ -634,7 +650,9 @@ class FilterModule(object):
 
     @staticmethod
     def oo_openshift_env(hostvars):
-        ''' Return facts which begin with "openshift_"
+        ''' Return facts which begin with "openshift_" and translate
+            legacy facts to their openshift_env counterparts.
+
             Ex: hostvars = {'openshift_fact': 42,
                             'theyre_taking_the_hobbits_to': 'isengard'}
                 returns  = {'openshift_fact': 42}
@@ -647,6 +665,11 @@ class FilterModule(object):
         for key in hostvars:
             if regex.match(key):
                 facts[key] = hostvars[key]
+
+        migrations = {'openshift_router_selector': 'openshift_hosted_router_selector'}
+        for old_fact, new_fact in migrations.iteritems():
+            if old_fact in facts and new_fact not in facts:
+                facts[new_fact] = facts[old_fact]
         return facts
 
     @staticmethod
diff --git a/playbooks/aws/openshift-cluster/config.yml b/playbooks/aws/openshift-cluster/config.yml
index 9fba856a2..66ff3e5b8 100644
--- a/playbooks/aws/openshift-cluster/config.yml
+++ b/playbooks/aws/openshift-cluster/config.yml
@@ -11,7 +11,7 @@
     openshift_deployment_type: "{{ deployment_type }}"
     openshift_public_hostname: "{{ ec2_ip_address }}"
     openshift_registry_selector: 'type=infra'
-    openshift_router_selector: 'type=infra'
+    openshift_hosted_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"] }}"}'
     openshift_master_cluster_method: 'native'
diff --git a/playbooks/gce/openshift-cluster/config.yml b/playbooks/gce/openshift-cluster/config.yml
index ba37a3a1f..283f460a9 100644
--- a/playbooks/gce/openshift-cluster/config.yml
+++ b/playbooks/gce/openshift-cluster/config.yml
@@ -12,7 +12,7 @@
     openshift_deployment_type: "{{ deployment_type }}"
     openshift_hostname: "{{ gce_private_ip }}"
     openshift_registry_selector: 'type=infra'
-    openshift_router_selector: 'type=infra'
+    openshift_hosted_router_selector: 'type=infra'
     openshift_infra_nodes: "{{ g_infra_hosts }}"
     openshift_master_cluster_method: 'native'
     openshift_use_openshift_sdn: "{{ lookup('oo_option', 'use_openshift_sdn') }}"
diff --git a/playbooks/libvirt/openshift-cluster/config.yml b/playbooks/libvirt/openshift-cluster/config.yml
index 0e003ef67..5bfe61657 100644
--- a/playbooks/libvirt/openshift-cluster/config.yml
+++ b/playbooks/libvirt/openshift-cluster/config.yml
@@ -14,7 +14,7 @@
     openshift_debug_level: "{{ debug_level }}"
     openshift_deployment_type: "{{ deployment_type }}"
     openshift_registry_selector: 'type=infra'
-    openshift_router_selector: 'type=infra'
+    openshift_hosted_router_selector: 'type=infra'
     openshift_infra_nodes: "{{ g_infra_hosts }}"
     openshift_master_cluster_method: 'native'
     openshift_use_openshift_sdn: "{{ lookup('oo_option', 'use_openshift_sdn') }}"
diff --git a/playbooks/openstack/openshift-cluster/config.yml b/playbooks/openstack/openshift-cluster/config.yml
index 093beaf03..319202982 100644
--- a/playbooks/openstack/openshift-cluster/config.yml
+++ b/playbooks/openstack/openshift-cluster/config.yml
@@ -11,7 +11,7 @@
     openshift_debug_level: "{{ debug_level }}"
     openshift_deployment_type: "{{ deployment_type }}"
     openshift_registry_selector: 'type=infra'
-    openshift_router_selector: 'type=infra'
+    openshift_hosted_router_selector: 'type=infra'
     openshift_infra_nodes: "{{ g_infra_hosts }}"
     openshift_master_cluster_method: 'native'
     openshift_use_openshift_sdn: "{{ lookup('oo_option', 'use_openshift_sdn') }}"
diff --git a/roles/openshift_facts/library/openshift_facts.py b/roles/openshift_facts/library/openshift_facts.py
index 862871bd8..f2d17293a 100755
--- a/roles/openshift_facts/library/openshift_facts.py
+++ b/roles/openshift_facts/library/openshift_facts.py
@@ -95,6 +95,7 @@ def migrate_local_facts(facts):
     migrated_facts = migrate_docker_facts(migrated_facts)
     migrated_facts = migrate_common_facts(migrated_facts)
     migrated_facts = migrate_node_facts(migrated_facts)
+    migrated_facts = migrate_hosted_facts(migrated_facts)
     return migrated_facts
 
 def migrate_hosted_facts(facts):
@@ -1852,14 +1853,7 @@ class OpenShiftFacts(object):
                         val = [x.strip() for x in val.split(',')]
                     new_local_facts['docker'][key] = list(set(val) - set(['']))
 
-        for facts in new_local_facts.values():
-            keys_to_delete = []
-            if isinstance(facts, dict):
-                for fact, value in facts.iteritems():
-                    if value == "" or value is None:
-                        keys_to_delete.append(fact)
-                for key in keys_to_delete:
-                    del facts[key]
+        new_local_facts = self.remove_empty_facts(new_local_facts)
 
         if new_local_facts != local_facts:
             self.validate_local_facts(new_local_facts)
@@ -1870,6 +1864,23 @@ class OpenShiftFacts(object):
         self.changed = changed
         return new_local_facts
 
+    def remove_empty_facts(self, facts=None):
+        """ Remove empty facts
+
+            Args:
+                facts (dict): facts to clean
+        """
+        facts_to_remove = []
+        for fact, value in facts.iteritems():
+            if isinstance(facts[fact], dict):
+                facts[fact] = self.remove_empty_facts(facts[fact])
+            else:
+                if value == "" or value is None:
+                    facts_to_remove.append(fact)
+        for fact in facts_to_remove:
+            del facts[fact]
+        return facts
+
     def validate_local_facts(self, facts=None):
         """ Validate local facts
 
diff --git a/roles/openshift_hosted/tasks/router.yml b/roles/openshift_hosted/tasks/router.yml
index 6a36f74b2..4ccbf4430 100644
--- a/roles/openshift_hosted/tasks/router.yml
+++ b/roles/openshift_hosted/tasks/router.yml
@@ -32,6 +32,7 @@
     {{ openshift.common.client_binary }} --api-version='v1' -o json
     get nodes -n default --config={{ openshift.common.config_base }}/master/admin.kubeconfig
   register: openshift_hosted_router_nodes_json
+  changed_when: false
   when: openshift.hosted.router.replicas | default(None) == None
 
 - name: Collect nodes matching router selector
diff --git a/roles/openshift_master_facts/tasks/main.yml b/roles/openshift_master_facts/tasks/main.yml
index e3be12182..a020fdb41 100644
--- a/roles/openshift_master_facts/tasks/main.yml
+++ b/roles/openshift_master_facts/tasks/main.yml
@@ -54,7 +54,6 @@
       mcs_allocator_range: "{{ osm_mcs_allocator_range | default(None) }}"
       mcs_labels_per_project: "{{ osm_mcs_labels_per_project | default(None) }}"
       uid_allocator_range: "{{ osm_uid_allocator_range | default(None) }}"
-      router_selector: "{{ openshift_router_selector | default(None) }}"
       registry_selector: "{{ openshift_registry_selector | default(None) }}"
       api_server_args: "{{ osm_api_server_args | default(None) }}"
       controller_args: "{{ osm_controller_args | default(None) }}"