diff options
49 files changed, 483 insertions, 494 deletions
diff --git a/.coveragerc b/.coveragerc index e1d918755..1e819e157 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,5 +1,18 @@ [run] -omit= +branch = True +omit = */lib/python*/site-packages/* */lib/python*/* /usr/* + setup.py + # TODO(rhcarvalho): this is used to ignore test files from coverage report. + # We can make this less generic when we stick with a single test pattern in + # the repo. + test_*.py + *_tests.py + +[report] +fail_under = 28 + +[html] +directory = cover diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dafa73bad..502ef6aa5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -152,3 +152,17 @@ are run on a different Jenkins host that is not publicly accessible, however the test results are posted to S3 buckets when complete. The test output of each job is also posted to the Pull Request as comments. + +--- + +## Appendix + +### Finding unused Python code + +If you are contributing with Python code, you can use the tool +[`vulture`](https://pypi.python.org/pypi/vulture) to verify that you are not +introducing unused code by accident. + +This tool is not used in an automated form in CI nor otherwise because it may +produce both false positives and false negatives. Still, it can be helpful to +detect dead code that escapes our eyes. diff --git a/conftest.py b/conftest.py new file mode 100644 index 000000000..ad03fab29 --- /dev/null +++ b/conftest.py @@ -0,0 +1,6 @@ +"""pytest configuration""" + + +def pytest_ignore_collect(path): + """Hook to ignore symlink files and directories.""" + return path.islink() diff --git a/openshift-ansible.spec b/openshift-ansible.spec index e49c54347..c03d4eb8f 100644 --- a/openshift-ansible.spec +++ b/openshift-ansible.spec @@ -73,6 +73,10 @@ find -L %{buildroot}%{_datadir}/ansible/%{name}/playbooks -name filter_plugins - # openshift-ansible-roles install cp -rp roles %{buildroot}%{_datadir}/ansible/%{name}/ +# openshift_master_facts symlinks filter_plugins/oo_filters.py from ansible_plugins/filter_plugins +pushd %{buildroot}%{_datadir}/ansible/%{name}/roles/openshift_master_facts/filter_plugins +ln -sf ../../../../../ansible_plugins/filter_plugins/oo_filters.py oo_filters.py +popd # openshift-ansible-filter-plugins install cp -rp filter_plugins %{buildroot}%{_datadir}/ansible_plugins/ diff --git a/playbooks/common/openshift-node/restart.yml b/playbooks/common/openshift-node/restart.yml index 6e9b1cca3..5e1df951c 100644 --- a/playbooks/common/openshift-node/restart.yml +++ b/playbooks/common/openshift-node/restart.yml @@ -2,16 +2,24 @@ - name: Restart nodes hosts: oo_nodes_to_config serial: "{{ openshift_restart_nodes_serial | default(1) }}" + + roles: + - lib_openshift + tasks: - name: Restart docker - service: name=docker state=restarted + service: + name: docker + state: restarted - name: Update docker facts openshift_facts: role: docker - name: Restart containerized services - service: name={{ item }} state=started + service: + name: "{{ item }}" + state: started with_items: - etcd_container - openvswitch @@ -36,12 +44,14 @@ state: restarted - name: Wait for node to be ready - command: > - {{ hostvars[groups.oo_first_master.0].openshift.common.client_binary }} get node {{ openshift.common.hostname | lower }} --no-headers + oc_obj: + state: list + kind: node + name: "{{ openshift.common.hostname | lower }}" register: node_output delegate_to: "{{ groups.oo_first_master.0 }}" when: inventory_hostname in groups.oo_nodes_to_config - until: "{{ node_output.stdout.split()[1].startswith('Ready')}}" + until: node_output.results.results[0].status.conditions | selectattr('type', 'equalto', 'Ready') | map(attribute='status') | join | bool == True # Give the node two minutes to come back online. retries: 24 delay: 5 diff --git a/requirements.txt b/requirements.txt index 8f47033f8..5a6a161cb 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,3 +2,4 @@ ansible>=2.2 six pyOpenSSL PyYAML +ruamel.yaml diff --git a/roles/lib_openshift/library/oc_route.py b/roles/lib_openshift/library/oc_route.py index 73600cb13..4b5c4460c 100644 --- a/roles/lib_openshift/library/oc_route.py +++ b/roles/lib_openshift/library/oc_route.py @@ -1548,7 +1548,7 @@ class OCRoute(OpenShiftCLI): def needs_update(self): ''' verify an update is needed ''' skip = [] - return not Utils.check_def_equal(self.config.data, self.route.yaml_dict, skip_keys=skip, debug=True) + return not Utils.check_def_equal(self.config.data, self.route.yaml_dict, skip_keys=skip, debug=self.verbose) @staticmethod def get_cert_data(path, content): diff --git a/roles/lib_openshift/library/oc_secret.py b/roles/lib_openshift/library/oc_secret.py index f3bb3c413..c423e9442 100644 --- a/roles/lib_openshift/library/oc_secret.py +++ b/roles/lib_openshift/library/oc_secret.py @@ -1429,7 +1429,7 @@ class Secret(Yedit): def update_secret(self, key, value): ''' update a secret''' # pylint: disable=no-member - if self.secrets.has_key(key): + if key in self.secrets: self.secrets[key] = value else: self.add_secret(key, value) @@ -1467,7 +1467,7 @@ class OCSecret(OpenShiftCLI): if results['returncode'] == 0 and results['results'][0]: results['exists'] = True if self.decode: - if results['results'][0].has_key('data'): + if 'data' in results['results'][0]: for sname, value in results['results'][0]['data'].items(): results['decoded'][sname] = base64.b64decode(value) diff --git a/roles/lib_openshift/src/class/oc_route.py b/roles/lib_openshift/src/class/oc_route.py index cb743e19d..448457292 100644 --- a/roles/lib_openshift/src/class/oc_route.py +++ b/roles/lib_openshift/src/class/oc_route.py @@ -61,7 +61,7 @@ class OCRoute(OpenShiftCLI): def needs_update(self): ''' verify an update is needed ''' skip = [] - return not Utils.check_def_equal(self.config.data, self.route.yaml_dict, skip_keys=skip, debug=True) + return not Utils.check_def_equal(self.config.data, self.route.yaml_dict, skip_keys=skip, debug=self.verbose) @staticmethod def get_cert_data(path, content): diff --git a/roles/lib_openshift/src/class/oc_secret.py b/roles/lib_openshift/src/class/oc_secret.py index 5eac27572..deb36a9fa 100644 --- a/roles/lib_openshift/src/class/oc_secret.py +++ b/roles/lib_openshift/src/class/oc_secret.py @@ -29,7 +29,7 @@ class OCSecret(OpenShiftCLI): if results['returncode'] == 0 and results['results'][0]: results['exists'] = True if self.decode: - if results['results'][0].has_key('data'): + if 'data' in results['results'][0]: for sname, value in results['results'][0]['data'].items(): results['decoded'][sname] = base64.b64decode(value) diff --git a/roles/lib_openshift/src/lib/secret.py b/roles/lib_openshift/src/lib/secret.py index 39bf3c33a..1ba78ddd5 100644 --- a/roles/lib_openshift/src/lib/secret.py +++ b/roles/lib_openshift/src/lib/secret.py @@ -91,7 +91,7 @@ class Secret(Yedit): def update_secret(self, key, value): ''' update a secret''' # pylint: disable=no-member - if self.secrets.has_key(key): + if key in self.secrets: self.secrets[key] = value else: self.add_secret(key, value) diff --git a/roles/lib_openshift/src/test/unit/oadm_manage_node.py b/roles/lib_openshift/src/test/unit/test_oadm_manage_node.py index b0786dfac..b0786dfac 100755..100644 --- a/roles/lib_openshift/src/test/unit/oadm_manage_node.py +++ b/roles/lib_openshift/src/test/unit/test_oadm_manage_node.py diff --git a/roles/lib_openshift/src/test/unit/oc_env.py b/roles/lib_openshift/src/test/unit/test_oc_env.py index 15bd7e464..15bd7e464 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_env.py +++ b/roles/lib_openshift/src/test/unit/test_oc_env.py diff --git a/roles/lib_openshift/src/test/unit/oc_label.py b/roles/lib_openshift/src/test/unit/test_oc_label.py index 3176987b0..3176987b0 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_label.py +++ b/roles/lib_openshift/src/test/unit/test_oc_label.py diff --git a/roles/lib_openshift/src/test/unit/oc_process.py b/roles/lib_openshift/src/test/unit/test_oc_process.py index 450ff7071..450ff7071 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_process.py +++ b/roles/lib_openshift/src/test/unit/test_oc_process.py diff --git a/roles/lib_openshift/src/test/unit/oc_route.py b/roles/lib_openshift/src/test/unit/test_oc_route.py index 361b61f4b..361b61f4b 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_route.py +++ b/roles/lib_openshift/src/test/unit/test_oc_route.py diff --git a/roles/lib_openshift/src/test/unit/oc_scale.py b/roles/lib_openshift/src/test/unit/test_oc_scale.py index f15eb164d..f15eb164d 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_scale.py +++ b/roles/lib_openshift/src/test/unit/test_oc_scale.py diff --git a/roles/lib_openshift/src/test/unit/oc_secret.py b/roles/lib_openshift/src/test/unit/test_oc_secret.py index 645aac82b..645aac82b 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_secret.py +++ b/roles/lib_openshift/src/test/unit/test_oc_secret.py diff --git a/roles/lib_openshift/src/test/unit/oc_service.py b/roles/lib_openshift/src/test/unit/test_oc_service.py index 4a845e9f3..4a845e9f3 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_service.py +++ b/roles/lib_openshift/src/test/unit/test_oc_service.py diff --git a/roles/lib_openshift/src/test/unit/oc_serviceaccount.py b/roles/lib_openshift/src/test/unit/test_oc_serviceaccount.py index 256b569eb..256b569eb 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_serviceaccount.py +++ b/roles/lib_openshift/src/test/unit/test_oc_serviceaccount.py diff --git a/roles/lib_openshift/src/test/unit/oc_serviceaccount_secret.py b/roles/lib_openshift/src/test/unit/test_oc_serviceaccount_secret.py index 213c581aa..213c581aa 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_serviceaccount_secret.py +++ b/roles/lib_openshift/src/test/unit/test_oc_serviceaccount_secret.py diff --git a/roles/lib_openshift/src/test/unit/oc_version.py b/roles/lib_openshift/src/test/unit/test_oc_version.py index 67dea415b..67dea415b 100755..100644 --- a/roles/lib_openshift/src/test/unit/oc_version.py +++ b/roles/lib_openshift/src/test/unit/test_oc_version.py diff --git a/roles/lib_utils/library/repoquery.py b/roles/lib_utils/library/repoquery.py index cc7aa04fd..f31c8911b 100644 --- a/roles/lib_utils/library/repoquery.py +++ b/roles/lib_utils/library/repoquery.py @@ -477,7 +477,7 @@ class Repoquery(RepoqueryCLI): ''' Gather and present the versions of each package ''' versions_dict = {} - versions_dict['available_versions_full'] = formatted_versions.keys() + versions_dict['available_versions_full'] = list(formatted_versions.keys()) # set the match version, if called if self.match_version: diff --git a/roles/lib_utils/src/class/repoquery.py b/roles/lib_utils/src/class/repoquery.py index 2447719e2..82adcada5 100644 --- a/roles/lib_utils/src/class/repoquery.py +++ b/roles/lib_utils/src/class/repoquery.py @@ -60,7 +60,7 @@ class Repoquery(RepoqueryCLI): ''' Gather and present the versions of each package ''' versions_dict = {} - versions_dict['available_versions_full'] = formatted_versions.keys() + versions_dict['available_versions_full'] = list(formatted_versions.keys()) # set the match version, if called if self.match_version: diff --git a/roles/lib_utils/src/test/unit/repoquery.py b/roles/lib_utils/src/test/unit/test_repoquery.py index c487ab254..c487ab254 100755..100644 --- a/roles/lib_utils/src/test/unit/repoquery.py +++ b/roles/lib_utils/src/test/unit/test_repoquery.py diff --git a/roles/lib_utils/src/test/unit/yedit_test.py b/roles/lib_utils/src/test/unit/test_yedit.py index 2793c5c1a..ed07ac96e 100755..100644 --- a/roles/lib_utils/src/test/unit/yedit_test.py +++ b/roles/lib_utils/src/test/unit/test_yedit.py @@ -256,7 +256,7 @@ class YeditTest(unittest.TestCase): def test_pop_list_item_2(self): '''test dict value with none value''' - z = range(10) + z = list(range(10)) yed = Yedit(content=z, separator=':') yed.pop('', 5) z.pop(5) diff --git a/roles/openshift_facts/tasks/main.yml b/roles/openshift_facts/tasks/main.yml index 9a1982076..0ec294bbc 100644 --- a/roles/openshift_facts/tasks/main.yml +++ b/roles/openshift_facts/tasks/main.yml @@ -14,6 +14,18 @@ l_is_master_system_container: "{{ (use_master_system_container | default(use_system_containers) | bool) }}" l_is_etcd_system_container: "{{ (use_etcd_system_container | default(use_system_containers) | bool) }}" +- name: Validate python version + fail: + msg: | + openshift-ansible requires Python 3 for {{ ansible_distribution }}; + For information on enabling Python 3 with Ansible, see https://docs.ansible.com/ansible/python_3_support.html + when: ansible_distribution == 'Fedora' and ansible_python['version']['major'] != 3 + +- name: Validate python version + fail: + msg: "openshift-ansible requires Python 2 for {{ ansible_distribution }}" + when: ansible_distribution != 'Fedora' and ansible_python['version']['major'] != 2 + - name: Ensure various deps are installed package: name={{ item }} state=present with_items: "{{ required_packages }}" diff --git a/roles/openshift_hosted/tasks/router/router.yml b/roles/openshift_hosted/tasks/router/router.yml index b944fa522..3d5713d6b 100644 --- a/roles/openshift_hosted/tasks/router/router.yml +++ b/roles/openshift_hosted/tasks/router/router.yml @@ -78,3 +78,27 @@ changed_when: "'service exists' not in openshift_hosted_router_results.stdout" failed_when: "openshift_hosted_router_results.rc != 0 and 'service exists' not in openshift_hosted_router_results.stdout and 'deployment_config' not in openshift_hosted_router_results.stderr and 'service' not in openshift_hosted_router_results.stderr" when: replicas | int > 0 + +- command: > + {{ openshift.common.client_binary }} + {% if openshift.hosted.router.name | default(none) is not none -%} + get dc/{{ openshift.hosted.router.name }} + {% else %} + get dc/router + {% endif%} + --template=\\{\\{.spec.replicas\\}\\} + --namespace={{ openshift.hosted.router.namespace | default('default') }} + register: current_replicas + when: replicas | int > 0 + +- name: Ensure router replica count matches desired + command: > + {{ openshift.common.client_binary }} + scale --replicas={{ replicas }} + {% if openshift.hosted.router.name | default(none) is not none -%} + dc/{{ openshift.hosted.router.name }} + {% else %} + dc/router + {% endif%} + --namespace={{ openshift.hosted.router.namespace | default('default') }} + when: replicas | int > 0 and replicas | int != current_replicas.stdout | int diff --git a/roles/openshift_logging/tasks/upgrade_logging.yaml b/roles/openshift_logging/tasks/upgrade_logging.yaml index 0421cdf58..83867d361 100644 --- a/roles/openshift_logging/tasks/upgrade_logging.yaml +++ b/roles/openshift_logging/tasks/upgrade_logging.yaml @@ -26,14 +26,14 @@ loop_control: loop_var: object -- name: Wait for pods to stop +- name: Wait for pods to start oc_obj: state: list - kind: dc + kind: pods selector: "component=es" namespace: "{{openshift_logging_namespace}}" register: running_pod - until: running_pod.results.results.items[?(@.status.phase == "Running")].metadata.name != '' + until: running_pod.results.results[0]['items'] | selectattr('status.phase', 'equalto', 'Running') | map(attribute='metadata.name') | list | length != 0 retries: 30 delay: 10 diff --git a/roles/openshift_logging/templates/curator.j2 b/roles/openshift_logging/templates/curator.j2 index b7bc15b62..55f4976ec 100644 --- a/roles/openshift_logging/templates/curator.j2 +++ b/roles/openshift_logging/templates/curator.j2 @@ -31,7 +31,7 @@ spec: {% if curator_node_selector is iterable and curator_node_selector | length > 0 %} nodeSelector: {% for key, value in curator_node_selector.iteritems() %} - {{key}}: {{value}} + {{key}}: "{{value}}" {% endfor %} {% endif %} containers: diff --git a/roles/openshift_logging/templates/es.j2 b/roles/openshift_logging/templates/es.j2 index ec84c6b76..81ae070be 100644 --- a/roles/openshift_logging/templates/es.j2 +++ b/roles/openshift_logging/templates/es.j2 @@ -33,7 +33,7 @@ spec: {% if es_node_selector is iterable and es_node_selector | length > 0 %} nodeSelector: {% for key, value in es_node_selector.iteritems() %} - {{key}}: {{value}} + {{key}}: "{{value}}" {% endfor %} {% endif %} containers: diff --git a/roles/openshift_logging/templates/kibana.j2 b/roles/openshift_logging/templates/kibana.j2 index be9b45ab4..e6ecf82ff 100644 --- a/roles/openshift_logging/templates/kibana.j2 +++ b/roles/openshift_logging/templates/kibana.j2 @@ -30,7 +30,7 @@ spec: {% if kibana_node_selector is iterable and kibana_node_selector | length > 0 %} nodeSelector: {% for key, value in kibana_node_selector.iteritems() %} - {{key}}: {{value}} + {{key}}: "{{value}}" {% endfor %} {% endif %} containers: diff --git a/roles/openshift_master_facts/test/conftest.py b/roles/openshift_master_facts/test/conftest.py new file mode 100644 index 000000000..e67d24f04 --- /dev/null +++ b/roles/openshift_master_facts/test/conftest.py @@ -0,0 +1,54 @@ +import os +import sys + +import pytest + +sys.path.insert(1, os.path.join(os.path.dirname(__file__), os.pardir, "lookup_plugins")) + +from openshift_master_facts_default_predicates import LookupModule as PredicatesLookupModule # noqa: E402 +from openshift_master_facts_default_priorities import LookupModule as PrioritiesLookupModule # noqa: E402 + + +@pytest.fixture() +def predicates_lookup(): + return PredicatesLookupModule() + + +@pytest.fixture() +def priorities_lookup(): + return PrioritiesLookupModule() + + +@pytest.fixture() +def facts(request): + return { + 'openshift': { + 'common': {} + } + } + + +@pytest.fixture(params=[True, False]) +def regions_enabled(request): + return request.param + + +@pytest.fixture(params=[True, False]) +def zones_enabled(request): + return request.param + + +def v_prefix(release): + """Prefix a release number with 'v'.""" + return "v" + release + + +def minor(release): + """Add a suffix to release, making 'X.Y' become 'X.Y.Z'.""" + return release + ".1" + + +@pytest.fixture(params=[str, v_prefix, minor]) +def release_mod(request): + """Modifies a release string to alternative valid values.""" + return request.param diff --git a/roles/openshift_master_facts/test/openshift_master_facts_bad_input_tests.py b/roles/openshift_master_facts/test/openshift_master_facts_bad_input_tests.py new file mode 100644 index 000000000..e8da1e04a --- /dev/null +++ b/roles/openshift_master_facts/test/openshift_master_facts_bad_input_tests.py @@ -0,0 +1,57 @@ +import copy +import os +import sys + +from ansible.errors import AnsibleError +import pytest + +sys.path.insert(1, os.path.join(os.path.dirname(__file__), os.pardir, "lookup_plugins")) + +from openshift_master_facts_default_predicates import LookupModule # noqa: E402 + + +class TestOpenShiftMasterFactsBadInput(object): + lookup = LookupModule() + default_facts = { + 'openshift': { + 'common': {} + } + } + + def test_missing_openshift_facts(self): + with pytest.raises(AnsibleError): + facts = {} + self.lookup.run(None, variables=facts) + + def test_missing_deployment_type(self): + with pytest.raises(AnsibleError): + facts = copy.deepcopy(self.default_facts) + facts['openshift']['common']['short_version'] = '10.10' + self.lookup.run(None, variables=facts) + + def test_missing_short_version_and_missing_openshift_release(self): + with pytest.raises(AnsibleError): + facts = copy.deepcopy(self.default_facts) + facts['openshift']['common']['deployment_type'] = 'origin' + self.lookup.run(None, variables=facts) + + def test_unknown_deployment_types(self): + with pytest.raises(AnsibleError): + facts = copy.deepcopy(self.default_facts) + facts['openshift']['common']['short_version'] = '1.1' + facts['openshift']['common']['deployment_type'] = 'bogus' + self.lookup.run(None, variables=facts) + + def test_unknown_origin_version(self): + with pytest.raises(AnsibleError): + 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) + + def test_unknown_ocp_version(self): + with pytest.raises(AnsibleError): + 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) diff --git a/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py b/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py index 68b6deb88..25294d91a 100644 --- a/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py +++ b/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py @@ -1,13 +1,5 @@ -import copy -import os -import sys +import pytest -from ansible.errors import AnsibleError -from nose.tools import raises, assert_equal - -sys.path = [os.path.abspath(os.path.dirname(__file__) + "/../lookup_plugins/")] + sys.path - -from openshift_master_facts_default_predicates import LookupModule # noqa: E402 # Predicates ordered according to OpenShift Origin source: # origin/vendor/k8s.io/kubernetes/plugin/pkg/scheduler/algorithmprovider/defaults/defaults.go @@ -88,179 +80,83 @@ TEST_VARS = [ ] -class TestOpenShiftMasterFactsDefaultPredicates(object): - def setUp(self): - self.lookup = LookupModule() - self.default_facts = { - 'openshift': { - 'common': {} - } - } +def assert_ok(predicates_lookup, default_predicates, regions_enabled, **kwargs): + results = predicates_lookup.run(None, regions_enabled=regions_enabled, **kwargs) + if regions_enabled: + assert results == default_predicates + [REGION_PREDICATE] + else: + assert results == default_predicates + + +def test_openshift_version(predicates_lookup, openshift_version_fixture, regions_enabled): + facts, default_predicates = openshift_version_fixture + assert_ok(predicates_lookup, default_predicates, variables=facts, regions_enabled=regions_enabled) + + +@pytest.fixture(params=TEST_VARS) +def openshift_version_fixture(request, facts): + version, deployment_type, default_predicates = request.param + version += '.1' + facts['openshift_version'] = version + facts['openshift']['common']['deployment_type'] = deployment_type + return facts, default_predicates + + +def test_openshift_release(predicates_lookup, openshift_release_fixture, regions_enabled): + facts, default_predicates = openshift_release_fixture + assert_ok(predicates_lookup, default_predicates, variables=facts, regions_enabled=regions_enabled) + + +@pytest.fixture(params=TEST_VARS) +def openshift_release_fixture(request, facts, release_mod): + release, deployment_type, default_predicates = request.param + facts['openshift_release'] = release_mod(release) + facts['openshift']['common']['deployment_type'] = deployment_type + return facts, default_predicates + + +def test_short_version(predicates_lookup, short_version_fixture, regions_enabled): + facts, default_predicates = short_version_fixture + assert_ok(predicates_lookup, default_predicates, variables=facts, regions_enabled=regions_enabled) + + +@pytest.fixture(params=TEST_VARS) +def short_version_fixture(request, facts): + short_version, deployment_type, default_predicates = request.param + facts['openshift']['common']['short_version'] = short_version + facts['openshift']['common']['deployment_type'] = deployment_type + return facts, default_predicates + + +def test_short_version_kwarg(predicates_lookup, short_version_kwarg_fixture, regions_enabled): + facts, short_version, default_predicates = short_version_kwarg_fixture + assert_ok(predicates_lookup, default_predicates, variables=facts, regions_enabled=regions_enabled, short_version=short_version) + + +@pytest.fixture(params=TEST_VARS) +def short_version_kwarg_fixture(request, facts): + short_version, deployment_type, default_predicates = request.param + facts['openshift']['common']['deployment_type'] = deployment_type + return facts, short_version, default_predicates + + +def test_deployment_type_kwarg(predicates_lookup, deployment_type_kwarg_fixture, regions_enabled): + facts, deployment_type, default_predicates = deployment_type_kwarg_fixture + assert_ok(predicates_lookup, default_predicates, variables=facts, regions_enabled=regions_enabled, deployment_type=deployment_type) + + +@pytest.fixture(params=TEST_VARS) +def deployment_type_kwarg_fixture(request, facts): + short_version, deployment_type, default_predicates = request.param + facts['openshift']['common']['short_version'] = short_version + return facts, deployment_type, default_predicates + + +def test_short_version_deployment_type_kwargs(predicates_lookup, short_version_deployment_type_kwargs_fixture, regions_enabled): + short_version, deployment_type, default_predicates = short_version_deployment_type_kwargs_fixture + assert_ok(predicates_lookup, default_predicates, regions_enabled=regions_enabled, short_version=short_version, deployment_type=deployment_type) + - @raises(AnsibleError) - def test_missing_short_version_and_missing_openshift_release(self): - facts = copy.deepcopy(self.default_facts) - facts['openshift']['common']['deployment_type'] = 'origin' - self.lookup.run(None, variables=facts) - - 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) - 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: - 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) - if regions_enabled: - assert_equal(results, default_predicates + [REGION_PREDICATE]) - else: - assert_equal(results, default_predicates) - - 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: - 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): - for regions_enabled in (True, False): - 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 - - def test_trunc_openshift_release(self): - for release, deployment_type, default_predicates in TEST_VARS: - release = release + '.1' - yield self.check_defaults_release, release, deployment_type, default_predicates, False - - @raises(AnsibleError) - def test_unknown_deployment_types(self): - facts = copy.deepcopy(self.default_facts) - facts['openshift']['common']['short_version'] = '1.1' - facts['openshift']['common']['deployment_type'] = 'bogus' - self.lookup.run(None, variables=facts) - - @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_missing_deployment_type(self): - facts = copy.deepcopy(self.default_facts) - facts['openshift']['common']['short_version'] = '10.10' - self.lookup.run(None, variables=facts) - - @raises(AnsibleError) - def testMissingOpenShiftFacts(self): - facts = {} - self.lookup.run(None, variables=facts) +@pytest.fixture(params=TEST_VARS) +def short_version_deployment_type_kwargs_fixture(request): + return request.param diff --git a/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py b/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py index 4e44a2b3d..cec44faa4 100644 --- a/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py +++ b/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py @@ -1,13 +1,5 @@ -import copy -import os -import sys +import pytest -from ansible.errors import AnsibleError -from nose.tools import raises, assert_equal - -sys.path = [os.path.abspath(os.path.dirname(__file__) + "/../lookup_plugins/")] + sys.path - -from openshift_master_facts_default_priorities import LookupModule # noqa: E402 DEFAULT_PRIORITIES_1_1 = [ {'name': 'LeastRequestedPriority', 'weight': 1}, @@ -76,177 +68,83 @@ TEST_VARS = [ ] -class TestOpenShiftMasterFactsDefaultPredicates(object): - def setUp(self): - self.lookup = LookupModule() - self.default_facts = { - 'openshift': { - 'common': {} - } - } +def assert_ok(priorities_lookup, default_priorities, zones_enabled, **kwargs): + results = priorities_lookup.run(None, zones_enabled=zones_enabled, **kwargs) + if zones_enabled: + assert results == default_priorities + [ZONE_PRIORITY] + else: + assert results == default_priorities + + +def test_openshift_version(priorities_lookup, openshift_version_fixture, zones_enabled): + facts, default_priorities = openshift_version_fixture + assert_ok(priorities_lookup, default_priorities, variables=facts, zones_enabled=zones_enabled) + + +@pytest.fixture(params=TEST_VARS) +def openshift_version_fixture(request, facts): + version, deployment_type, default_priorities = request.param + version += '.1' + facts['openshift_version'] = version + facts['openshift']['common']['deployment_type'] = deployment_type + return facts, default_priorities + + +def test_openshift_release(priorities_lookup, openshift_release_fixture, zones_enabled): + facts, default_priorities = openshift_release_fixture + assert_ok(priorities_lookup, default_priorities, variables=facts, zones_enabled=zones_enabled) + + +@pytest.fixture(params=TEST_VARS) +def openshift_release_fixture(request, facts, release_mod): + release, deployment_type, default_priorities = request.param + facts['openshift_release'] = release_mod(release) + facts['openshift']['common']['deployment_type'] = deployment_type + return facts, default_priorities + + +def test_short_version(priorities_lookup, short_version_fixture, zones_enabled): + facts, default_priorities = short_version_fixture + assert_ok(priorities_lookup, default_priorities, variables=facts, zones_enabled=zones_enabled) + + +@pytest.fixture(params=TEST_VARS) +def short_version_fixture(request, facts): + short_version, deployment_type, default_priorities = request.param + facts['openshift']['common']['short_version'] = short_version + facts['openshift']['common']['deployment_type'] = deployment_type + return facts, default_priorities + + +def test_short_version_kwarg(priorities_lookup, short_version_kwarg_fixture, zones_enabled): + facts, short_version, default_priorities = short_version_kwarg_fixture + assert_ok(priorities_lookup, default_priorities, variables=facts, zones_enabled=zones_enabled, short_version=short_version) + + +@pytest.fixture(params=TEST_VARS) +def short_version_kwarg_fixture(request, facts): + short_version, deployment_type, default_priorities = request.param + facts['openshift']['common']['deployment_type'] = deployment_type + return facts, short_version, default_priorities + + +def test_deployment_type_kwarg(priorities_lookup, deployment_type_kwarg_fixture, zones_enabled): + facts, deployment_type, default_priorities = deployment_type_kwarg_fixture + assert_ok(priorities_lookup, default_priorities, variables=facts, zones_enabled=zones_enabled, deployment_type=deployment_type) + + +@pytest.fixture(params=TEST_VARS) +def deployment_type_kwarg_fixture(request, facts): + short_version, deployment_type, default_priorities = request.param + facts['openshift']['common']['short_version'] = short_version + return facts, deployment_type, default_priorities + + +def test_short_version_deployment_type_kwargs(priorities_lookup, short_version_deployment_type_kwargs_fixture, zones_enabled): + short_version, deployment_type, default_priorities = short_version_deployment_type_kwargs_fixture + assert_ok(priorities_lookup, default_priorities, zones_enabled=zones_enabled, short_version=short_version, deployment_type=deployment_type) + - @raises(AnsibleError) - def test_missing_short_version_and_missing_openshift_release(self): - facts = copy.deepcopy(self.default_facts) - facts['openshift']['common']['deployment_type'] = 'origin' - self.lookup.run(None, variables=facts) - - def check_defaults_short_version(self, release, deployment_type, - default_priorities, zones_enabled): - facts = copy.deepcopy(self.default_facts) - 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: - 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: - assert_equal(results, default_priorities + [ZONE_PRIORITY]) - else: - assert_equal(results, default_priorities) - - 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: - 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): - for zones_enabled in (True, False): - 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 - - def test_trunc_openshift_release(self): - for release, deployment_type, default_priorities in TEST_VARS: - release = release + '.1' - yield self.check_defaults_release, release, deployment_type, default_priorities, False - - @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): - facts = copy.deepcopy(self.default_facts) - facts['openshift']['common']['short_version'] = '1.1' - facts['openshift']['common']['deployment_type'] = 'bogus' - self.lookup.run(None, variables=facts) - - @raises(AnsibleError) - 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) - - @raises(AnsibleError) - def test_missing_openshift_facts(self): - facts = {} - self.lookup.run(None, variables=facts) +@pytest.fixture(params=TEST_VARS) +def short_version_deployment_type_kwargs_fixture(request): + return request.param diff --git a/roles/openshift_node_upgrade/tasks/main.yml b/roles/openshift_node_upgrade/tasks/main.yml index 609ca2a6e..96e296a3b 100644 --- a/roles/openshift_node_upgrade/tasks/main.yml +++ b/roles/openshift_node_upgrade/tasks/main.yml @@ -13,7 +13,10 @@ vars: # We will restart Docker ourselves after everything is ready: skip_docker_restart: True - when: l_docker_upgrade is defined and l_docker_upgrade | bool and not openshift.common.is_atomic | bool + when: + - l_docker_upgrade is defined + - l_docker_upgrade | bool + - not openshift.common.is_containerized | bool - include: "{{ node_config_hook }}" when: node_config_hook is defined @@ -25,14 +28,19 @@ when: not openshift.common.is_containerized | bool - name: Remove obsolete docker-sdn-ovs.conf - file: path=/etc/systemd/system/docker.service.d/docker-sdn-ovs.conf state=absent - when: (deployment_type == 'openshift-enterprise' and openshift_release | version_compare('3.4', '>=')) or (deployment_type == 'origin' and openshift_release | version_compare('1.4', '>=')) + file: + path: "/etc/systemd/system/docker.service.d/docker-sdn-ovs.conf" + state: absent + when: (deployment_type == 'openshift-enterprise' and openshift_release | version_compare('3.4', '>=')) + or (deployment_type == 'origin' and openshift_release | version_compare('1.4', '>=')) - include: containerized_node_upgrade.yml when: openshift.common.is_containerized | bool - name: Ensure containerized services stopped before Docker restart - service: name={{ item }} state=stopped + service: + name: "{{ item }}" + state: stopped with_items: - etcd_container - openvswitch @@ -62,22 +70,19 @@ - include: docker/restart.yml - name: Restart rpm node service - service: name="{{ openshift.common.service_type }}-node" state=restarted + service: + name: "{{ openshift.common.service_type }}-node" + state: restarted when: not openshift.common.is_containerized | bool - name: Wait for node to be ready - command: > - {{ hostvars[groups.oo_first_master.0].openshift.common.client_binary }} get node {{ openshift.common.hostname | lower }} --no-headers + oc_obj: + state: list + kind: node + name: "{{ openshift.common.hostname | lower }}" register: node_output delegate_to: "{{ groups.oo_first_master.0 }}" - until: "{{ node_output.stdout.split()[1].startswith('Ready')}}" - # Give the node two minutes to come back online. Note that we pre-pull images now - # so containerized services should restart quickly as well. + until: node_output.results.results[0].status.conditions | selectattr('type', 'equalto', 'Ready') | map(attribute='status') | join | bool == True + # Give the node two minutes to come back online. retries: 24 delay: 5 - # AUDIT:changed_when: `false` because we are only inspecting the - # state of the node, we aren't changing anything (we changed node - # service state in the previous task). You could say we shouldn't - # override this because something will be changing (the state of a - # service), but that should be part of the last task. - changed_when: false @@ -4,17 +4,6 @@ # will need to generate wheels for each Python version that you support. universal=1 -[nosetests] -tests=roles/openshift_master_facts/test/, test/ -verbosity=2 -with-coverage=1 -cover-html=1 -cover-inclusive=1 -cover-min-percentage=70 -cover-erase=1 -detailed-errors=1 -cover-branches=1 - [yamllint] excludes=.tox,utils,files @@ -25,3 +14,22 @@ lint_disable=fixme,locally-disabled,file-ignored,duplicate-code exclude=.tox/*,utils/*,inventory/* max_line_length = 120 ignore = E501,T003 + +[tool:pytest] +norecursedirs = + .* + __pycache__ + cover + docs + # utils have its own config + utils +python_files = + # TODO(rhcarvalho): rename test files to follow a single pattern. "test*.py" + # is Python unittest's default, while pytest discovers both "test_*.py" and + # "*_test.py" by default. + test_*.py + *_tests.py +addopts = + --cov=. + --cov-report=term + --cov-report=html @@ -13,6 +13,7 @@ import yaml from setuptools import setup, Command from setuptools_lint.setuptools_command import PylintCommand from six import string_types +from six.moves import reload_module from yamllint.config import YamlLintConfig from yamllint.cli import Format from yamllint import linter @@ -185,6 +186,7 @@ class OpenShiftAnsibleGenerateValidation(Command): # the python path. # pylint: disable=import-error import generate + reload_module(generate) generate.verify() except generate.GenerateAnsibleException as gae: print(gae.args) diff --git a/test-requirements.txt b/test-requirements.txt index 2ee1e657d..9bb6e058c 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -7,5 +7,7 @@ pylint setuptools-lint PyYAML yamllint -nose coverage +mock +pytest +pytest-cov @@ -12,8 +12,10 @@ deps = ansible22: ansible~=2.2 commands = - flake8: flake8 + unit: pytest {posargs} + flake8: flake8 {posargs} pylint: python setup.py lint yamllint: python setup.py yamllint - unit: nosetests generate_validation: python setup.py generate_validation + + diff --git a/utils/.coveragerc b/utils/.coveragerc index e1d918755..551e13192 100644 --- a/utils/.coveragerc +++ b/utils/.coveragerc @@ -1,5 +1,18 @@ [run] -omit= +branch = True +omit = */lib/python*/site-packages/* */lib/python*/* /usr/* + setup.py + # TODO(rhcarvalho): this is used to ignore test files from coverage report. + # We can make this less generic when we stick with a single test pattern in + # the repo. + test_*.py + *_tests.py + +[report] +fail_under = 73 + +[html] +directory = cover diff --git a/utils/README.md b/utils/README.md index c37ab41e6..7aa045ae4 100644 --- a/utils/README.md +++ b/utils/README.md @@ -1,56 +1,61 @@ -# Running Tests (NEW) +# Running Tests Run the command: make ci -to run an array of unittests locally. +to run tests and linting tools. Underneath the covers, we use [tox](http://readthedocs.org/docs/tox/) to manage virtualenvs and run tests. Alternatively, tests can be run using [detox](https://pypi.python.org/pypi/detox/) which allows -for running tests in parallel - +for running tests in parallel. ``` pip install tox detox ``` List the test environments available: + ``` tox -l ``` Run all of the tests with: + ``` tox ``` Run all of the tests in parallel with detox: + ``` detox ``` -Running a particular test environment (python 2.7 flake8 tests in this case): +Run a particular test environment: + ``` -tox -e py27-ansible22-flake8 +tox -e py27-flake8 ``` -Running a particular test environment in a clean virtualenv (python 3.5 pylint -tests in this case): +Run a particular test environment in a clean virtualenv: + ``` -tox -r -e py35-ansible22-pylint +tox -r -e py35-pylint ``` If you want to enter the virtualenv created by tox to do additional -testing/debugging (py27-flake8 env in this case): +testing/debugging: + ``` -source .tox/py27-ansible22-flake8/bin/activate +source .tox/py27-flake8/bin/activate ``` You will get errors if the log files already exist and can not be written to by the current user (`/tmp/ansible.log` and `/tmp/installer.txt`). *We're working on it.* + # Running From Source You will need to setup a **virtualenv** to run from source: @@ -66,17 +71,3 @@ The virtualenv `bin` directory should now be at the start of your You can exit the virtualenv with: $ deactivate - -# Testing (OLD) - -*This section is deprecated, but still works* - -First, run the **virtualenv setup steps** described above. - -Install some testing libraries: (we cannot do this via setuptools due to the version virtualenv bundles) - -$ pip install mock nose - -Then run the tests with: - -$ oo-install/bin/nosetests diff --git a/utils/setup.cfg b/utils/setup.cfg index 862dffd7b..d730cd3b4 100644 --- a/utils/setup.cfg +++ b/utils/setup.cfg @@ -4,15 +4,8 @@ # will need to generate wheels for each Python version that you support. universal=1 -[nosetests] -verbosity=2 -with-coverage=1 -cover-html=1 -cover-inclusive=1 -cover-min-percentage=70 -cover-erase=1 -detailed-errors=1 -cover-branches=1 +[aliases] +test=pytest [flake8] max-line-length=120 @@ -21,3 +14,19 @@ ignore=E501 [lint] lint_disable=fixme,locally-disabled,file-ignored,duplicate-code + +[tool:pytest] +testpaths = test +norecursedirs = + .* + __pycache__ +python_files = + # TODO(rhcarvalho): rename test files to follow a single pattern. "test*.py" + # is Python unittest's default, while pytest discovers both "test_*.py" and + # "*_test.py" by default. + test_*.py + *_tests.py +addopts = + --cov=. + --cov-report=term + --cov-report=html diff --git a/utils/setup.py b/utils/setup.py index 3518581e7..20e3d81dc 100644 --- a/utils/setup.py +++ b/utils/setup.py @@ -40,7 +40,7 @@ setup( # simple. Or you can use find_packages(). #packages=find_packages(exclude=['contrib', 'docs', 'tests*']), packages=['ooinstall'], - package_dir={'ooinstall': 'src/ooinstall'}, + package_dir={'': 'src'}, # List run-time dependencies here. These will be installed by pip when @@ -65,9 +65,8 @@ setup( 'ooinstall': ['ansible.cfg', 'ansible-quiet.cfg', 'ansible_plugins/*'], }, - tests_require=['nose'], - - test_suite='nose.collector', + setup_requires=['pytest-runner'], + tests_require=['pytest'], # To provide executable scripts, use entry points in preference to the # "scripts" keyword. Entry points provide cross-platform support and allow diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py index b787741d7..a6d784dea 100644 --- a/utils/src/ooinstall/cli_installer.py +++ b/utils/src/ooinstall/cli_installer.py @@ -72,12 +72,6 @@ You might want to override the default subdomain used for exposed routes. If you return click.prompt('New default subdomain (ENTER for none)', default='') -def list_hosts(hosts): - hosts_idx = range(len(hosts)) - for idx in hosts_idx: - click.echo(' {}: {}'.format(idx, hosts[idx])) - - def collect_hosts(oo_cfg, existing_env=False, masters_set=False, print_summary=True): """ Collect host information from user. This will later be filled in using @@ -656,20 +650,6 @@ https://docs.openshift.com/enterprise/latest/admin_guide/install/prerequisites.h return oo_cfg -def get_role_variable(oo_cfg, role_name, variable_name): - try: - target_role = next(role for role in oo_cfg.deployment.roles if role.name is role_name) - target_variable = target_role.variables[variable_name] - return target_variable - except (StopIteration, KeyError): - return None - - -def set_role_variable(oo_cfg, role_name, variable_name, variable_value): - target_role = next(role for role in oo_cfg.deployment.roles if role.name is role_name) - target_role[variable_name] = variable_value - - def collect_new_nodes(oo_cfg): click.clear() click.echo('*** New Node Configuration ***') diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py index cf14105af..c3501c018 100644 --- a/utils/src/ooinstall/oo_config.py +++ b/utils/src/ooinstall/oo_config.py @@ -126,15 +126,6 @@ class Host(object): """ Does this host have the etcd role """ return 'etcd' in self.roles - def is_etcd_member(self, all_hosts): - """ Will this host be a member of a standalone etcd cluster. """ - if not self.is_master(): - return False - masters = [host for host in all_hosts if host.is_master()] - if len(masters) > 1: - return True - return False - def is_dedicated_node(self): """ Will this host be a dedicated node. (not a master) """ return self.is_node() and not self.is_master() @@ -185,7 +176,7 @@ class Deployment(object): class OOConfig(object): default_dir = os.path.normpath( os.environ.get('XDG_CONFIG_HOME', - os.environ['HOME'] + '/.config/') + '/openshift/') + os.environ.get('HOME', '') + '/.config/') + '/openshift/') default_file = '/installer.cfg.yml' def __init__(self, config_path): @@ -436,12 +427,6 @@ class OOConfig(object): def __str__(self): return self.yaml() - def get_host(self, name): - for host in self.deployment.hosts: - if host.connect_to == name: - return host - return None - def get_host_roles_set(self): roles_set = set() for host in self.deployment.hosts: diff --git a/utils/test-requirements.txt b/utils/test-requirements.txt index 699afc26a..b26e22a7e 100644 --- a/utils/test-requirements.txt +++ b/utils/test-requirements.txt @@ -2,7 +2,6 @@ ansible # flake8 moved to before setuptools-lint to satisfy mccabe dependency issue flake8 setuptools-lint -nose coverage mock PyYAML @@ -12,3 +11,5 @@ pyOpenSSL yamllint tox detox +pytest +pytest-cov diff --git a/utils/tox.ini b/utils/tox.ini index 1308f7505..2524923cb 100644 --- a/utils/tox.ini +++ b/utils/tox.ini @@ -11,6 +11,9 @@ deps = -rtest-requirements.txt py35-flake8: flake8-bugbear commands = + # Needed to make detox work, since it ignores usedevelop + # https://github.com/tox-dev/tox/issues/180 + unit: pip install -e . + unit: pytest {posargs} flake8: python setup.py flake8 - unit: python setup.py nosetests pylint: python setup.py lint |