Merge pull request #2892 from detiber/upgradeScheduler

Scheduler upgrades

playbooks/common/openshift-cluster/evaluate_groups.yml

@@ -7,27 +7,27 @@
   tasks:
   - fail:
       msg: This playbook requires g_etcd_hosts to be set
-    when: g_etcd_hosts is not defined
+    when: "{{ g_etcd_hosts is not defined }}"
 
   - fail:
       msg: This playbook requires g_master_hosts or g_new_master_hosts to be set
-    when: g_master_hosts is not defined and g_new_master_hosts is not defined
+    when: "{{ g_master_hosts is not defined and g_new_master_hosts is not defined }}"
 
   - fail:
       msg: This playbook requires g_node_hosts or g_new_node_hosts to be set
-    when: g_node_hosts is not defined and g_new_node_hosts is not defined
+    when: "{{ g_node_hosts is not defined and g_new_node_hosts is not defined }}"
 
   - fail:
       msg: This playbook requires g_lb_hosts to be set
-    when: g_lb_hosts is not defined
+    when: "{{ g_lb_hosts is not defined }}"
 
   - fail:
       msg: This playbook requires g_nfs_hosts to be set
-    when: g_nfs_hosts is not defined
+    when: "{{ g_nfs_hosts is not defined }}"
 
   - fail:
       msg: The nfs group must be limited to one host
-    when: (groups[g_nfs_hosts] | default([])) | length > 1
+    when: "{{ (groups[g_nfs_hosts] | default([])) | length > 1 }}"
 
   - name: Evaluate oo_all_hosts
     add_host:
@@ -82,7 +82,7 @@
       ansible_ssh_user: "{{ g_ssh_user | default(omit) }}"
       ansible_become: "{{ g_sudo | default(omit) }}"
     with_items: "{{ g_master_hosts | default([]) }}"
-    when: g_nodeonmaster | default(false) | bool and not g_new_node_hosts | default(false) | bool
+    when: "{{ g_nodeonmaster | default(false) | bool and not g_new_node_hosts | default(false) | bool }}"
     changed_when: no
 
   - name: Evaluate oo_first_etcd
@@ -90,7 +90,7 @@
       name: "{{ g_etcd_hosts[0] }}"
       groups: oo_first_etcd
       ansible_ssh_user: "{{ g_ssh_user | default(omit) }}"
-    when: g_etcd_hosts|length > 0
+    when: "{{ g_etcd_hosts|length > 0 }}"
     changed_when: no
 
   - name: Evaluate oo_first_master
@@ -99,7 +99,7 @@
       groups: oo_first_master
       ansible_ssh_user: "{{ g_ssh_user | default(omit) }}"
       ansible_become: "{{ g_sudo | default(omit) }}"
-    when: g_master_hosts|length > 0
+    when: "{{ g_master_hosts|length > 0 }}"
     changed_when: no
 
   - name: Evaluate oo_lb_to_config

playbooks/common/openshift-cluster/upgrades/upgrade_control_plane.yml

@@ -33,9 +33,6 @@
 
 - name: Upgrade master packages
   hosts: oo_masters_to_config
-  handlers:
-  - include: ../../../../roles/openshift_master/handlers/main.yml
-    static: yes
   roles:
   - openshift_facts
   tasks:
@@ -64,6 +61,8 @@
   - openshift_facts
   - openshift_master_facts
   tasks:
+  - include: upgrade_scheduler.yml
+
   - include: "{{ master_config_hook }}"
     when: master_config_hook is defined
 

playbooks/common/openshift-cluster/upgrades/upgrade_scheduler.yml

@@ -0,0 +1,166 @@
+---
+# Upgrade predicates
+- vars:
+    prev_predicates: "{{ lookup('openshift_master_facts_default_predicates', short_version=openshift_upgrade_min, deployment_type=openshift_deployment_type) }}"
+    prev_predicates_no_region: "{{ lookup('openshift_master_facts_default_predicates', short_version=openshift_upgrade_min, deployment_type=openshift_deployment_type, regions_enabled=False) }}"
+    default_predicates_no_region: "{{ lookup('openshift_master_facts_default_predicates', regions_enabled=False) }}"
+    # older_predicates are the set of predicates that have previously been
+    # hard-coded into openshift_facts
+    older_predicates:
+    - - name: MatchNodeSelector
+      - name: PodFitsResources
+      - name: PodFitsPorts
+      - name: NoDiskConflict
+      - name: NoVolumeZoneConflict
+      - name: MaxEBSVolumeCount
+      - name: MaxGCEPDVolumeCount
+      - name: Region
+        argument:
+          serviceAffinity:
+            labels:
+            - region
+    - - name: MatchNodeSelector
+      - name: PodFitsResources
+      - name: PodFitsPorts
+      - name: NoDiskConflict
+      - name: NoVolumeZoneConflict
+      - name: Region
+        argument:
+          serviceAffinity:
+            labels:
+            - region
+    - - name: MatchNodeSelector
+      - name: PodFitsResources
+      - name: PodFitsPorts
+      - name: NoDiskConflict
+      - name: Region
+        argument:
+          serviceAffinity:
+            labels:
+            - region
+    # older_predicates_no_region are the set of predicates that have previously
+    # been hard-coded into openshift_facts, with the Region predicate removed
+    older_predicates_no_region:
+    - - name: MatchNodeSelector
+      - name: PodFitsResources
+      - name: PodFitsPorts
+      - name: NoDiskConflict
+      - name: NoVolumeZoneConflict
+      - name: MaxEBSVolumeCount
+      - name: MaxGCEPDVolumeCount
+    - - name: MatchNodeSelector
+      - name: PodFitsResources
+      - name: PodFitsPorts
+      - name: NoDiskConflict
+      - name: NoVolumeZoneConflict
+    - - name: MatchNodeSelector
+      - name: PodFitsResources
+      - name: PodFitsPorts
+      - name: NoDiskConflict
+  block:
+
+  # Handle case where openshift_master_predicates is defined
+  - block:
+    - debug:
+        msg: "WARNING: openshift_master_scheduler_predicates is set to defaults from an earlier release of OpenShift current defaults are: {{ openshift_master_scheduler_default_predicates }}"
+      when: "{{ openshift_master_scheduler_predicates in older_predicates + older_predicates_no_region + [prev_predicates] + [prev_predicates_no_region] }}"
+
+    - debug:
+        msg: "WARNING: openshift_master_scheduler_predicates does not match current defaults of: {{ openshift_master_scheduler_default_predicates }}"
+      when: "{{ openshift_master_scheduler_predicates != openshift_master_scheduler_default_predicates }}"
+    when: "{{ openshift_master_scheduler_predicates | default(none) is not none }}"
+
+  # Handle cases where openshift_master_predicates is not defined
+  - block:
+    - debug:
+        msg: "WARNING: existing scheduler config does not match previous known defaults automated upgrade of scheduler config is disabled.\nexisting scheduler predicates: {{ openshift_master_scheduler_current_predicates }}\ncurrent scheduler default predicates are: {{ openshift_master_scheduler_default_predicates }}"
+      when: "{{ openshift_master_scheduler_current_predicates != openshift_master_scheduler_default_predicates and
+                openshift_master_scheduler_current_predicates not in older_predicates + [prev_predicates] }}"
+
+    - set_fact:
+        openshift_upgrade_scheduler_predicates: "{{ openshift_master_scheduler_default_predicates }}"
+      when: "{{ openshift_master_scheduler_current_predicates != openshift_master_scheduler_default_predicates and
+                openshift_master_scheduler_current_predicates in older_predicates + [prev_predicates] }}"
+
+    - set_fact:
+        openshift_upgrade_scheduler_predicates: "{{ default_predicates_no_region }}"
+      when: "{{ openshift_master_scheduler_current_predicates != default_predicates_no_region and
+                openshift_master_scheduler_current_predicates in older_predicates_no_region + [prev_predicates_no_region] }}"
+
+    when: "{{ openshift_master_scheduler_predicates | default(none) is none }}"
+
+
+# Upgrade priorities
+- vars:
+    prev_priorities: "{{ lookup('openshift_master_facts_default_priorities', short_version=openshift_upgrade_min, deployment_type=openshift_deployment_type) }}"
+    prev_priorities_no_zone: "{{ lookup('openshift_master_facts_default_priorities', short_version=openshift_upgrade_min, deployment_type=openshift_deployment_type, zones_enabled=False) }}"
+    default_priorities_no_zone: "{{ lookup('openshift_master_facts_default_priorities', zones_enabled=False) }}"
+    # older_priorities are the set of priorities that have previously been
+    # hard-coded into openshift_facts
+    older_priorities:
+    - - name: LeastRequestedPriority
+        weight: 1
+      - name: SelectorSpreadPriority
+        weight: 1
+      - name: Zone
+        weight: 2
+        argument:
+          serviceAntiAffinity:
+            label: zone
+    # older_priorities_no_region are the set of priorities that have previously
+    # been hard-coded into openshift_facts, with the Zone priority removed
+    older_priorities_no_zone:
+    - - name: LeastRequestedPriority
+        weight: 1
+      - name: SelectorSpreadPriority
+        weight: 1
+  block:
+
+  # Handle case where openshift_master_priorities is defined
+  - block:
+    - debug:
+        msg: "WARNING: openshift_master_scheduler_priorities is set to defaults from an earlier release of OpenShift current defaults are: {{ openshift_master_scheduler_default_priorities }}"
+      when: "{{ openshift_master_scheduler_priorities in older_priorities + older_priorities_no_zone + [prev_priorities] + [prev_priorities_no_zone] }}"
+
+    - debug:
+        msg: "WARNING: openshift_master_scheduler_priorities does not match current defaults of: {{ openshift_master_scheduler_default_priorities }}"
+      when: "{{ openshift_master_scheduler_priorities != openshift_master_scheduler_default_priorities }}"
+    when: "{{ openshift_master_scheduler_priorities | default(none) is not none }}"
+
+  # Handle cases where openshift_master_priorities is not defined
+  - block:
+    - debug:
+        msg: "WARNING: existing scheduler config does not match previous known defaults automated upgrade of scheduler config is disabled.\nexisting scheduler priorities: {{ openshift_master_scheduler_current_priorities }}\ncurrent scheduler default priorities are: {{ openshift_master_scheduler_default_priorities }}"
+      when: "{{ openshift_master_scheduler_current_priorities != openshift_master_scheduler_default_priorities and
+                openshift_master_scheduler_current_priorities not in older_priorities + [prev_priorities] }}"
+
+    - set_fact:
+        openshift_upgrade_scheduler_priorities: "{{ openshift_master_scheduler_default_priorities }}"
+      when: "{{ openshift_master_scheduler_current_priorities != openshift_master_scheduler_default_priorities and
+                openshift_master_scheduler_current_priorities in older_priorities + [prev_priorities] }}"
+
+    - set_fact:
+        openshift_upgrade_scheduler_priorities: "{{ default_priorities_no_zone }}"
+      when: "{{ openshift_master_scheduler_current_priorities != default_priorities_no_zone and
+                openshift_master_scheduler_current_priorities in older_priorities_no_zone + [prev_priorities_no_zone] }}"
+
+    when: "{{ openshift_master_scheduler_priorities | default(none) is none }}"
+
+
+# Update scheduler
+- vars:
+    scheduler_config:
+      kind: Policy
+      apiVersion: v1
+      predicates: "{{ openshift_upgrade_scheduler_predicates
+                      | default(openshift_master_scheduler_current_predicates) }}"
+      priorities: "{{ openshift_upgrade_scheduler_priorities
+                      | default(openshift_master_scheduler_current_priorities) }}"
+  block:
+  - name: Update scheduler config
+    copy:
+      content: "{{ scheduler_config | to_nice_json }}"
+      dest: "{{ openshift_master_scheduler_conf }}"
+      backup: true
+  when: "{{ openshift_upgrade_scheduler_predicates is defined or
+            openshift_upgrade_scheduler_priorities is defined }}"

roles/openshift_master_facts/lookup_plugins/openshift_master_facts_default_predicates.py

@@ -6,26 +6,24 @@ from ansible.plugins.lookup import LookupBase
 
 
 class LookupModule(LookupBase):
-    # pylint: disable=too-many-branches,too-many-statements
-
-    def run(self, terms, variables=None, regions_enabled=True, **kwargs):
-        if 'openshift' not in variables:
-            raise AnsibleError("This lookup module requires openshift_facts to be run prior to use")
-        if 'master' not in variables['openshift']:
-            raise AnsibleError("This lookup module is meant to be run against an OpenShift master host only")
-
-        if 'openshift_master_scheduler_predicates' in variables:
-            return variables['openshift_master_scheduler_predicates']
-        elif 'scheduler_predicates' in variables['openshift']['master']:
-            return variables['openshift']['master']['scheduler_predicates']
-        else:
-            predicates = []
+    # pylint: disable=too-many-branches,too-many-statements,too-many-arguments
+
+    def run(self, terms, variables=None, regions_enabled=True, short_version=None,
+            deployment_type=None, **kwargs):
+
+        predicates = []
 
-            if 'deployment_type' not in variables['openshift']['common']:
+        if short_version is None or deployment_type is None:
+            if 'openshift' not in variables:
+                raise AnsibleError("This lookup module requires openshift_facts to be run prior to use")
+
+        if deployment_type is None:
+            if 'common' not in variables['openshift'] or 'deployment_type' not in variables['openshift']['common']:
                 raise AnsibleError("This lookup module requires that the deployment_type be set")
 
             deployment_type = variables['openshift']['common']['deployment_type']
 
+        if short_version is None:
             if 'short_version' in variables['openshift']['common']:
                 short_version = variables['openshift']['common']['short_version']
             elif 'openshift_release' in variables:
@@ -40,57 +38,56 @@ class LookupModule(LookupBase):
             else:
                 # pylint: disable=line-too-long
                 raise AnsibleError("Either OpenShift needs to be installed or openshift_release needs to be specified")
-            if deployment_type not in ['origin', 'openshift-enterprise']:
-                raise AnsibleError("Unknown deployment_type %s" % deployment_type)
-
-            if deployment_type == 'origin':
-                if short_version not in ['1.1', '1.2', '1.3', '1.4']:
-                    raise AnsibleError("Unknown short_version %s" % short_version)
-            elif deployment_type == 'openshift_enterprise':
-                if short_version not in ['3.1', '3.2', '3.3', '3.4']:
-                    raise AnsibleError("Unknown short_version %s" % short_version)
-
-            if deployment_type == 'openshift-enterprise':
-                # convert short_version to origin short_version
-                short_version = re.sub('^3.', '1.', short_version)
-
-            if short_version in ['1.1', '1.2']:
-                predicates.append({'name': 'PodFitsHostPorts'})
-                predicates.append({'name': 'PodFitsResources'})
-
-            # applies to all known versions
-            predicates.append({'name': 'NoDiskConflict'})
-
-            # only 1.1 didn't include NoVolumeZoneConflict
-            if short_version != '1.1':
-                predicates.append({'name': 'NoVolumeZoneConflict'})
-
-            if short_version in ['1.1', '1.2']:
-                predicates.append({'name': 'MatchNodeSelector'})
-                predicates.append({'name': 'Hostname'})
-
-            if short_version != '1.1':
-                predicates.append({'name': 'MaxEBSVolumeCount'})
-                predicates.append({'name': 'MaxGCEPDVolumeCount'})
-
-            if short_version not in ['1.1', '1.2']:
-                predicates.append({'name': 'GeneralPredicates'})
-                predicates.append({'name': 'PodToleratesNodeTaints'})
-                predicates.append({'name': 'CheckNodeMemoryPressure'})
-
-            if short_version not in ['1.1', '1.2', '1.3']:
-                predicates.append({'name': 'CheckNodeDiskPressure'})
-                predicates.append({'name': 'MatchInterPodAffinity'})
-
-            if regions_enabled:
-                region_predicate = {
-                    'name': 'Region',
-                    'argument': {
-                        'serviceAffinity': {
-                            'labels': ['region']
-                        }
+        if deployment_type == 'origin':
+            if short_version not in ['1.1', '1.2', '1.3', '1.4']:
+                raise AnsibleError("Unknown short_version %s" % short_version)
+        elif deployment_type == 'openshift-enterprise':
+            if short_version not in ['3.1', '3.2', '3.3', '3.4']:
+                raise AnsibleError("Unknown short_version %s" % short_version)
+        else:
+            raise AnsibleError("Unknown deployment_type %s" % deployment_type)
+
+        if deployment_type == 'openshift-enterprise':
+            # convert short_version to origin short_version
+            short_version = re.sub('^3.', '1.', short_version)
+
+        if short_version in ['1.1', '1.2']:
+            predicates.append({'name': 'PodFitsHostPorts'})
+            predicates.append({'name': 'PodFitsResources'})
+
+        # applies to all known versions
+        predicates.append({'name': 'NoDiskConflict'})
+
+        # only 1.1 didn't include NoVolumeZoneConflict
+        if short_version != '1.1':
+            predicates.append({'name': 'NoVolumeZoneConflict'})
+
+        if short_version in ['1.1', '1.2']:
+            predicates.append({'name': 'MatchNodeSelector'})
+            predicates.append({'name': 'Hostname'})
+
+        if short_version != '1.1':
+            predicates.append({'name': 'MaxEBSVolumeCount'})
+            predicates.append({'name': 'MaxGCEPDVolumeCount'})
+
+        if short_version not in ['1.1', '1.2']:
+            predicates.append({'name': 'GeneralPredicates'})
+            predicates.append({'name': 'PodToleratesNodeTaints'})
+            predicates.append({'name': 'CheckNodeMemoryPressure'})
+
+        if short_version not in ['1.1', '1.2', '1.3']:
+            predicates.append({'name': 'CheckNodeDiskPressure'})
+            predicates.append({'name': 'MatchInterPodAffinity'})
+
+        if regions_enabled:
+            region_predicate = {
+                'name': 'Region',
+                'argument': {
+                    'serviceAffinity': {
+                        'labels': ['region']
                     }
                 }
-                predicates.append(region_predicate)
+            }
+            predicates.append(region_predicate)
 
-            return predicates
+        return predicates

roles/openshift_master_facts/lookup_plugins/openshift_master_facts_default_priorities.py

@@ -6,30 +6,28 @@ from ansible.plugins.lookup import LookupBase
 
 
 class LookupModule(LookupBase):
-    # pylint: disable=too-many-branches
-
-    def run(self, terms, variables=None, zones_enabled=True, **kwargs):
-        if 'openshift' not in variables:
-            raise AnsibleError("This lookup module requires openshift_facts to be run prior to use")
-        if 'master' not in variables['openshift']:
-            raise AnsibleError("This lookup module is meant to be run against an OpenShift master host only")
-
-        if 'openshift_master_scheduler_priorities' in variables:
-            return variables['openshift_master_scheduler_priorities']
-        elif 'scheduler_priorities' in variables['openshift']['master']:
-            return variables['openshift']['master']['scheduler_priorities']
-        else:
-            priorities = [
-                {'name': 'LeastRequestedPriority', 'weight': 1},
-                {'name': 'BalancedResourceAllocation', 'weight': 1},
-                {'name': 'SelectorSpreadPriority', 'weight': 1}
-            ]
+    # pylint: disable=too-many-branches,too-many-statements,too-many-arguments
+
+    def run(self, terms, variables=None, zones_enabled=True, short_version=None,
+            deployment_type=None, **kwargs):
+
+        priorities = [
+            {'name': 'LeastRequestedPriority', 'weight': 1},
+            {'name': 'BalancedResourceAllocation', 'weight': 1},
+            {'name': 'SelectorSpreadPriority', 'weight': 1}
+        ]
 
-            if 'deployment_type' not in variables['openshift']['common']:
+        if short_version is None or deployment_type is None:
+            if 'openshift' not in variables:
+                raise AnsibleError("This lookup module requires openshift_facts to be run prior to use")
+
+        if deployment_type is None:
+            if 'common' not in variables['openshift'] or 'deployment_type' not in variables['openshift']['common']:
                 raise AnsibleError("This lookup module requires that the deployment_type be set")
 
             deployment_type = variables['openshift']['common']['deployment_type']
 
+        if short_version is None:
             if 'short_version' in variables['openshift']['common']:
                 short_version = variables['openshift']['common']['short_version']
             elif 'openshift_release' in variables:
@@ -45,43 +43,42 @@ class LookupModule(LookupBase):
                 # pylint: disable=line-too-long
                 raise AnsibleError("Either OpenShift needs to be installed or openshift_release needs to be specified")
 
-            if deployment_type not in ['origin', 'openshift-enterprise']:
-                raise AnsibleError("Unknown deployment_type %s" % deployment_type)
-
-            if deployment_type == 'origin':
-                if short_version not in ['1.1', '1.2', '1.3', '1.4']:
-                    raise AnsibleError("Unknown short_version %s" % short_version)
-            elif deployment_type == 'openshift_enterprise':
-                if short_version not in ['3.1', '3.2', '3.3', '3.4']:
-                    raise AnsibleError("Unknown short_version %s" % short_version)
-
-            if deployment_type == 'openshift-enterprise':
-                # convert short_version to origin short_version
-                short_version = re.sub('^3.', '1.', short_version)
-
-            if short_version == '1.4':
-                priorities.append({'name': 'NodePreferAvoidPodsPriority', 'weight': 10000})
-
-            # only 1.1 didn't include NodeAffinityPriority
-            if short_version != '1.1':
-                priorities.append({'name': 'NodeAffinityPriority', 'weight': 1})
-
-            if short_version not in ['1.1', '1.2']:
-                priorities.append({'name': 'TaintTolerationPriority', 'weight': 1})
-
-            if short_version not in ['1.1', '1.2', '1.3']:
-                priorities.append({'name': 'InterPodAffinityPriority', 'weight': 1})
-
-            if zones_enabled:
-                zone_priority = {
-                    'name': 'Zone',
-                    'argument': {
-                        'serviceAntiAffinity': {
-                            'label': 'zone'
-                        }
-                    },
-                    'weight': 2
-                }
-                priorities.append(zone_priority)
-
-            return priorities
+        if deployment_type == 'origin':
+            if short_version not in ['1.1', '1.2', '1.3', '1.4']:
+                raise AnsibleError("Unknown short_version %s" % short_version)
+        elif deployment_type == 'openshift-enterprise':
+            if short_version not in ['3.1', '3.2', '3.3', '3.4']:
+                raise AnsibleError("Unknown short_version %s" % short_version)
+        else:
+            raise AnsibleError("Unknown deployment_type %s" % deployment_type)
+
+        if deployment_type == 'openshift-enterprise':
+            # convert short_version to origin short_version
+            short_version = re.sub('^3.', '1.', short_version)
+
+        if short_version == '1.4':
+            priorities.append({'name': 'NodePreferAvoidPodsPriority', 'weight': 10000})
+
+        # only 1.1 didn't include NodeAffinityPriority
+        if short_version != '1.1':
+            priorities.append({'name': 'NodeAffinityPriority', 'weight': 1})
+
+        if short_version not in ['1.1', '1.2']:
+            priorities.append({'name': 'TaintTolerationPriority', 'weight': 1})
+
+        if short_version not in ['1.1', '1.2', '1.3']:
+            priorities.append({'name': 'InterPodAffinityPriority', 'weight': 1})
+
+        if zones_enabled:
+            zone_priority = {
+                'name': 'Zone',
+                'argument': {
+                    'serviceAntiAffinity': {
+                        'label': 'zone'
+                    }
+                },
+                'weight': 2
+            }
+            priorities.append(zone_priority)
+
+        return priorities

roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py

@@ -59,13 +59,23 @@ REGION_PREDICATE = {
     }
 }
 
+TEST_VARS = [
+    ('1.1', 'origin', DEFAULT_PREDICATES_1_1),
+    ('3.1', 'openshift-enterprise', DEFAULT_PREDICATES_1_1),
+    ('1.2', 'origin', DEFAULT_PREDICATES_1_2),
+    ('3.2', 'openshift-enterprise', DEFAULT_PREDICATES_1_2),
+    ('1.3', 'origin', DEFAULT_PREDICATES_1_3),
+    ('3.3', 'openshift-enterprise', DEFAULT_PREDICATES_1_3),
+    ('1.4', 'origin', DEFAULT_PREDICATES_1_4),
+    ('3.4', 'openshift-enterprise', DEFAULT_PREDICATES_1_4)
+]
+
 
 class TestOpenShiftMasterFactsDefaultPredicates(object):
     def setUp(self):
         self.lookup = LookupModule()
         self.default_facts = {
             'openshift': {
-                'master': {},
                 'common': {}
             }
         }
@@ -76,13 +86,58 @@ class TestOpenShiftMasterFactsDefaultPredicates(object):
         facts['openshift']['common']['deployment_type'] = 'origin'
         self.lookup.run(None, variables=facts)
 
-    def check_defaults(self, release, deployment_type, default_predicates,
-                       regions_enabled, short_version):
+    def check_defaults_short_version(self, short_version, deployment_type, default_predicates,
+                                     regions_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['short_version'] = short_version
+        facts['openshift']['common']['deployment_type'] = deployment_type
+        results = self.lookup.run(None, variables=facts,
+                                  regions_enabled=regions_enabled)
+        if regions_enabled:
+            assert_equal(results, default_predicates + [REGION_PREDICATE])
+        else:
+            assert_equal(results, default_predicates)
+
+    def check_defaults_short_version_kwarg(self, short_version, deployment_type, default_predicates,
+                                           regions_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['deployment_type'] = deployment_type
+        results = self.lookup.run(None, variables=facts,
+                                  regions_enabled=regions_enabled,
+                                  short_version=short_version)
+        if regions_enabled:
+            assert_equal(results, default_predicates + [REGION_PREDICATE])
+        else:
+            assert_equal(results, default_predicates)
+
+    def check_defaults_deployment_type_kwarg(self, short_version, deployment_type,
+                                             default_predicates, regions_enabled):
         facts = copy.deepcopy(self.default_facts)
-        if short_version:
-            facts['openshift']['common']['short_version'] = release
+        facts['openshift']['common']['short_version'] = short_version
+        results = self.lookup.run(None, variables=facts,
+                                  regions_enabled=regions_enabled,
+                                  deployment_type=deployment_type)
+        if regions_enabled:
+            assert_equal(results, default_predicates + [REGION_PREDICATE])
+        else:
+            assert_equal(results, default_predicates)
+
+    def check_defaults_only_kwargs(self, short_version, deployment_type,
+                                   default_predicates, regions_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        results = self.lookup.run(None, variables=facts,
+                                  regions_enabled=regions_enabled,
+                                  short_version=short_version,
+                                  deployment_type=deployment_type)
+        if regions_enabled:
+            assert_equal(results, default_predicates + [REGION_PREDICATE])
         else:
-            facts['openshift_release'] = release
+            assert_equal(results, default_predicates)
+
+    def check_defaults_release(self, release, deployment_type, default_predicates,
+                               regions_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift_release'] = release
         facts['openshift']['common']['deployment_type'] = deployment_type
         results = self.lookup.run(None, variables=facts,
                                   regions_enabled=regions_enabled)
@@ -91,39 +146,70 @@ class TestOpenShiftMasterFactsDefaultPredicates(object):
         else:
             assert_equal(results, default_predicates)
 
-    def test_openshift_release_defaults(self):
-        test_vars = [
-            ('1.1', 'origin', DEFAULT_PREDICATES_1_1),
-            ('3.1', 'openshift-enterprise', DEFAULT_PREDICATES_1_1),
-            ('1.2', 'origin', DEFAULT_PREDICATES_1_2),
-            ('3.2', 'openshift-enterprise', DEFAULT_PREDICATES_1_2),
-            ('1.3', 'origin', DEFAULT_PREDICATES_1_3),
-            ('3.3', 'openshift-enterprise', DEFAULT_PREDICATES_1_3),
-            ('1.4', 'origin', DEFAULT_PREDICATES_1_4),
-            ('3.4', 'openshift-enterprise', DEFAULT_PREDICATES_1_4)
-        ]
+    def check_defaults_version(self, version, deployment_type, default_predicates,
+                               regions_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift_version'] = version
+        facts['openshift']['common']['deployment_type'] = deployment_type
+        results = self.lookup.run(None, variables=facts,
+                                  regions_enabled=regions_enabled)
+        if regions_enabled:
+            assert_equal(results, default_predicates + [REGION_PREDICATE])
+        else:
+            assert_equal(results, default_predicates)
+
+    def check_defaults_override_vars(self, release, deployment_type,
+                                     default_predicates, regions_enabled,
+                                     extra_facts=None):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['short_version'] = release
+        facts['openshift']['common']['deployment_type'] = deployment_type
+        if extra_facts is not None:
+            for fact in extra_facts:
+                facts[fact] = extra_facts[fact]
+        results = self.lookup.run(None, variables=facts,
+                                  regions_enabled=regions_enabled,
+                                  return_set_vars=False)
+        if regions_enabled:
+            assert_equal(results, default_predicates + [REGION_PREDICATE])
+        else:
+            assert_equal(results, default_predicates)
 
+    def test_openshift_version(self):
         for regions_enabled in (True, False):
-            for release, deployment_type, default_predicates in test_vars:
-                for prepend_v in (True, False):
-                    if prepend_v:
-                        release = 'v' + release
-                yield self.check_defaults, release, deployment_type, default_predicates, regions_enabled, False
+            for release, deployment_type, default_predicates in TEST_VARS:
+                release = release + '.1'
+                yield self.check_defaults_version, release, deployment_type, default_predicates, regions_enabled
+
+    def test_v_release_defaults(self):
+        for regions_enabled in (True, False):
+            for release, deployment_type, default_predicates in TEST_VARS:
+                yield self.check_defaults_release, 'v' + release, deployment_type, default_predicates, regions_enabled
+
+    def test_release_defaults(self):
+        for regions_enabled in (True, False):
+            for release, deployment_type, default_predicates in TEST_VARS:
+                yield self.check_defaults_release, release, deployment_type, default_predicates, regions_enabled
 
     def test_short_version_defaults(self):
-        test_vars = [
-            ('1.1', 'origin', DEFAULT_PREDICATES_1_1),
-            ('3.1', 'openshift-enterprise', DEFAULT_PREDICATES_1_1),
-            ('1.2', 'origin', DEFAULT_PREDICATES_1_2),
-            ('3.2', 'openshift-enterprise', DEFAULT_PREDICATES_1_2),
-            ('1.3', 'origin', DEFAULT_PREDICATES_1_3),
-            ('3.3', 'openshift-enterprise', DEFAULT_PREDICATES_1_3),
-            ('1.4', 'origin', DEFAULT_PREDICATES_1_4),
-            ('3.4', 'openshift-enterprise', DEFAULT_PREDICATES_1_4)
-        ]
         for regions_enabled in (True, False):
-            for short_version, deployment_type, default_predicates in test_vars:
-                yield self.check_defaults, short_version, deployment_type, default_predicates, regions_enabled, True
+            for release, deployment_type, default_predicates in TEST_VARS:
+                yield self.check_defaults_short_version, release, deployment_type, default_predicates, regions_enabled
+
+    def test_short_version_kwarg(self):
+        for regions_enabled in (True, False):
+            for release, deployment_type, default_predicates in TEST_VARS:
+                yield self.check_defaults_short_version_kwarg, release, deployment_type, default_predicates, regions_enabled
+
+    def test_only_kwargs(self):
+        for regions_enabled in (True, False):
+            for release, deployment_type, default_predicates in TEST_VARS:
+                yield self.check_defaults_only_kwargs, release, deployment_type, default_predicates, regions_enabled
+
+    def test_deployment_type_kwarg(self):
+        for regions_enabled in (True, False):
+            for release, deployment_type, default_predicates in TEST_VARS:
+                yield self.check_defaults_deployment_type_kwarg, release, deployment_type, default_predicates, regions_enabled
 
     @raises(AnsibleError)
     def test_unknown_deployment_types(self):
@@ -133,42 +219,26 @@ class TestOpenShiftMasterFactsDefaultPredicates(object):
         self.lookup.run(None, variables=facts)
 
     @raises(AnsibleError)
-    def test_missing_deployment_type(self):
+    def test_unknown_origin_version(self):
         facts = copy.deepcopy(self.default_facts)
-        facts['openshift']['common']['short_version'] = '10.10'
+        facts['openshift']['common']['short_version'] = '0.1'
+        facts['openshift']['common']['deployment_type'] = 'origin'
         self.lookup.run(None, variables=facts)
 
     @raises(AnsibleError)
-    def testMissingOpenShiftFacts(self):
-        facts = {}
+    def test_unknown_ocp_version(self):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['short_version'] = '0.1'
+        facts['openshift']['common']['deployment_type'] = 'openshift-enterprise'
         self.lookup.run(None, variables=facts)
 
     @raises(AnsibleError)
-    def testMissingMasterRole(self):
-        facts = {'openshift': {}}
+    def test_missing_deployment_type(self):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['short_version'] = '10.10'
         self.lookup.run(None, variables=facts)
 
-    def testPreExistingPredicates(self):
-        facts = {
-            'openshift': {
-                'master': {
-                    'scheduler_predicates': [
-                        {'name': 'pred_a'},
-                        {'name': 'pred_b'}
-                    ]
-                }
-            }
-        }
-        result = self.lookup.run(None, variables=facts)
-        assert_equal(result, facts['openshift']['master']['scheduler_predicates'])
-
-    def testDefinedPredicates(self):
-        facts = {
-            'openshift': {'master': {}},
-            'openshift_master_scheduler_predicates': [
-                {'name': 'pred_a'},
-                {'name': 'pred_b'}
-            ]
-        }
-        result = self.lookup.run(None, variables=facts)
-        assert_equal(result, facts['openshift_master_scheduler_predicates'])
+    @raises(AnsibleError)
+    def testMissingOpenShiftFacts(self):
+        facts = {}
+        self.lookup.run(None, variables=facts)

roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py

@@ -50,13 +50,23 @@ ZONE_PRIORITY = {
     'weight': 2
 }
 
+TEST_VARS = [
+    ('1.1', 'origin', DEFAULT_PRIORITIES_1_1),
+    ('3.1', 'openshift-enterprise', DEFAULT_PRIORITIES_1_1),
+    ('1.2', 'origin', DEFAULT_PRIORITIES_1_2),
+    ('3.2', 'openshift-enterprise', DEFAULT_PRIORITIES_1_2),
+    ('1.3', 'origin', DEFAULT_PRIORITIES_1_3),
+    ('3.3', 'openshift-enterprise', DEFAULT_PRIORITIES_1_3),
+    ('1.4', 'origin', DEFAULT_PRIORITIES_1_4),
+    ('3.4', 'openshift-enterprise', DEFAULT_PRIORITIES_1_4)
+]
+
 
 class TestOpenShiftMasterFactsDefaultPredicates(object):
     def setUp(self):
         self.lookup = LookupModule()
         self.default_facts = {
             'openshift': {
-                'master': {},
                 'common': {}
             }
         }
@@ -67,13 +77,57 @@ class TestOpenShiftMasterFactsDefaultPredicates(object):
         facts['openshift']['common']['deployment_type'] = 'origin'
         self.lookup.run(None, variables=facts)
 
-    def check_defaults(self, release, deployment_type, default_priorities,
-                       zones_enabled, short_version):
+    def check_defaults_short_version(self, release, deployment_type,
+                                     default_priorities, zones_enabled):
         facts = copy.deepcopy(self.default_facts)
-        if short_version:
-            facts['openshift']['common']['short_version'] = release
+        facts['openshift']['common']['short_version'] = release
+        facts['openshift']['common']['deployment_type'] = deployment_type
+        results = self.lookup.run(None, variables=facts, zones_enabled=zones_enabled)
+        if zones_enabled:
+            assert_equal(results, default_priorities + [ZONE_PRIORITY])
         else:
-            facts['openshift_release'] = release
+            assert_equal(results, default_priorities)
+
+    def check_defaults_short_version_kwarg(self, release, deployment_type,
+                                           default_priorities, zones_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['deployment_type'] = deployment_type
+        results = self.lookup.run(None, variables=facts,
+                                  zones_enabled=zones_enabled,
+                                  short_version=release)
+        if zones_enabled:
+            assert_equal(results, default_priorities + [ZONE_PRIORITY])
+        else:
+            assert_equal(results, default_priorities)
+
+    def check_defaults_deployment_type_kwarg(self, release, deployment_type,
+                                             default_priorities, zones_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['short_version'] = release
+        results = self.lookup.run(None, variables=facts,
+                                  zones_enabled=zones_enabled,
+                                  deployment_type=deployment_type)
+        if zones_enabled:
+            assert_equal(results, default_priorities + [ZONE_PRIORITY])
+        else:
+            assert_equal(results, default_priorities)
+
+    def check_defaults_only_kwargs(self, release, deployment_type,
+                                   default_priorities, zones_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        results = self.lookup.run(None, variables=facts,
+                                  zones_enabled=zones_enabled,
+                                  short_version=release,
+                                  deployment_type=deployment_type)
+        if zones_enabled:
+            assert_equal(results, default_priorities + [ZONE_PRIORITY])
+        else:
+            assert_equal(results, default_priorities)
+
+    def check_defaults_release(self, release, deployment_type, default_priorities,
+                               zones_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift_release'] = release
         facts['openshift']['common']['deployment_type'] = deployment_type
         results = self.lookup.run(None, variables=facts, zones_enabled=zones_enabled)
         if zones_enabled:
@@ -81,39 +135,84 @@ class TestOpenShiftMasterFactsDefaultPredicates(object):
         else:
             assert_equal(results, default_priorities)
 
-    def test_openshift_release_defaults(self):
-        test_vars = [
-            ('1.1', 'origin', DEFAULT_PRIORITIES_1_1),
-            ('3.1', 'openshift-enterprise', DEFAULT_PRIORITIES_1_1),
-            ('1.2', 'origin', DEFAULT_PRIORITIES_1_2),
-            ('3.2', 'openshift-enterprise', DEFAULT_PRIORITIES_1_2),
-            ('1.3', 'origin', DEFAULT_PRIORITIES_1_3),
-            ('3.3', 'openshift-enterprise', DEFAULT_PRIORITIES_1_3),
-            ('1.4', 'origin', DEFAULT_PRIORITIES_1_4),
-            ('3.4', 'openshift-enterprise', DEFAULT_PRIORITIES_1_4)
-        ]
+    def check_defaults_version(self, release, deployment_type, default_priorities,
+                               zones_enabled):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift_version'] = release
+        facts['openshift']['common']['deployment_type'] = deployment_type
+        results = self.lookup.run(None, variables=facts, zones_enabled=zones_enabled)
+        if zones_enabled:
+            assert_equal(results, default_priorities + [ZONE_PRIORITY])
+        else:
+            assert_equal(results, default_priorities)
 
+    def check_defaults_override_vars(self, release, deployment_type,
+                                     default_priorities, zones_enabled,
+                                     extra_facts=None):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['short_version'] = release
+        facts['openshift']['common']['deployment_type'] = deployment_type
+        if extra_facts is not None:
+            for fact in extra_facts:
+                facts[fact] = extra_facts[fact]
+        results = self.lookup.run(None, variables=facts,
+                                  zones_enabled=zones_enabled,
+                                  return_set_vars=False)
+        if zones_enabled:
+            assert_equal(results, default_priorities + [ZONE_PRIORITY])
+        else:
+            assert_equal(results, default_priorities)
+
+    def test_openshift_version(self):
+        for zones_enabled in (True, False):
+            for release, deployment_type, default_priorities in TEST_VARS:
+                release = release + '.1'
+                yield self.check_defaults_version, release, deployment_type, default_priorities, zones_enabled
+
+    def test_v_release_defaults(self):
         for zones_enabled in (True, False):
-            for release, deployment_type, default_priorities in test_vars:
-                for prepend_v in (True, False):
-                    if prepend_v:
-                        release = 'v' + release
-                yield self.check_defaults, release, deployment_type, default_priorities, zones_enabled, False
+            for release, deployment_type, default_priorities in TEST_VARS:
+                release = 'v' + release
+                yield self.check_defaults_release, release, deployment_type, default_priorities, zones_enabled
+
+    def test_release_defaults(self):
+        for zones_enabled in (True, False):
+            for release, deployment_type, default_priorities in TEST_VARS:
+                yield self.check_defaults_release, release, deployment_type, default_priorities, zones_enabled
 
     def test_short_version_defaults(self):
-        test_vars = [
-            ('1.1', 'origin', DEFAULT_PRIORITIES_1_1),
-            ('3.1', 'openshift-enterprise', DEFAULT_PRIORITIES_1_1),
-            ('1.2', 'origin', DEFAULT_PRIORITIES_1_2),
-            ('3.2', 'openshift-enterprise', DEFAULT_PRIORITIES_1_2),
-            ('1.3', 'origin', DEFAULT_PRIORITIES_1_3),
-            ('3.3', 'openshift-enterprise', DEFAULT_PRIORITIES_1_3),
-            ('1.4', 'origin', DEFAULT_PRIORITIES_1_4),
-            ('3.4', 'openshift-enterprise', DEFAULT_PRIORITIES_1_4)
-        ]
         for zones_enabled in (True, False):
-            for short_version, deployment_type, default_priorities in test_vars:
-                yield self.check_defaults, short_version, deployment_type, default_priorities, zones_enabled, True
+            for short_version, deployment_type, default_priorities in TEST_VARS:
+                yield self.check_defaults_short_version, short_version, deployment_type, default_priorities, zones_enabled
+
+    def test_only_kwargs(self):
+        for zones_enabled in (True, False):
+            for short_version, deployment_type, default_priorities in TEST_VARS:
+                yield self.check_defaults_only_kwargs, short_version, deployment_type, default_priorities, zones_enabled
+
+    def test_deployment_type_kwarg(self):
+        for zones_enabled in (True, False):
+            for short_version, deployment_type, default_priorities in TEST_VARS:
+                yield self.check_defaults_deployment_type_kwarg, short_version, deployment_type, default_priorities, zones_enabled
+
+    def test_release_kwarg(self):
+        for zones_enabled in (True, False):
+            for short_version, deployment_type, default_priorities in TEST_VARS:
+                yield self.check_defaults_short_version_kwarg, short_version, deployment_type, default_priorities, zones_enabled
+
+    @raises(AnsibleError)
+    def test_unknown_origin_version(self):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['short_version'] = '0.1'
+        facts['openshift']['common']['deployment_type'] = 'origin'
+        self.lookup.run(None, variables=facts)
+
+    @raises(AnsibleError)
+    def test_unknown_ocp_version(self):
+        facts = copy.deepcopy(self.default_facts)
+        facts['openshift']['common']['short_version'] = '0.1'
+        facts['openshift']['common']['deployment_type'] = 'openshift-enterprise'
+        self.lookup.run(None, variables=facts)
 
     @raises(AnsibleError)
     def test_unknown_deployment_types(self):
@@ -132,33 +231,3 @@ class TestOpenShiftMasterFactsDefaultPredicates(object):
     def test_missing_openshift_facts(self):
         facts = {}
         self.lookup.run(None, variables=facts)
-
-    @raises(AnsibleError)
-    def test_missing_master_role(self):
-        facts = {'openshift': {}}
-        self.lookup.run(None, variables=facts)
-
-    def test_pre_existing_priorities(self):
-        facts = {
-            'openshift': {
-                'master': {
-                    'scheduler_priorities': [
-                        {'name': 'pri_a', 'weight': 1},
-                        {'name': 'pri_b', 'weight': 1}
-                    ]
-                }
-            }
-        }
-        result = self.lookup.run(None, variables=facts)
-        assert_equal(result, facts['openshift']['master']['scheduler_priorities'])
-
-    def testDefinedPredicates(self):
-        facts = {
-            'openshift': {'master': {}},
-            'openshift_master_scheduler_priorities': [
-                {'name': 'pri_a', 'weight': 1},
-                {'name': 'pri_b', 'weight': 1}
-            ]
-        }
-        result = self.lookup.run(None, variables=facts)
-        assert_equal(result, facts['openshift_master_scheduler_priorities'])

utils/.coveragerc

@@ -0,0 +1,4 @@
+[run]
+omit=
+    */lib/python*/site-packages/*
+    /usr/*

utils/setup.cfg

@@ -7,10 +7,12 @@ universal=1
 [nosetests]
 tests=../,../roles/openshift_master_facts/test/,test/
 verbosity=2
-with_coverage=1
-cover_html=1
-cover_package=ooinstall
-cover_min_percentage=70
+with-coverage=1
+cover-html=1
+cover-inclusive=1
+cover-min-percentage=70
+detailed-errors=1
+cover-branches=1
 
 [flake8]
 max-line-length=120