From 4b9c91763bff4c377d9562c01bf81617e32bcccc Mon Sep 17 00:00:00 2001 From: Till Maas Date: Fri, 20 Jul 2018 15:09:31 +0200 Subject: [PATCH 1/2] Use nm provider by default only when NetworkManager is running - Use initscripts as provider except when NetworkManager is running - Rename network_provider_default to network_provider_os_default, since it contains the default based on the OS --- README.md | 7 +++++-- defaults/main.yml | 11 +++++++++-- tasks/main.yml | 19 ++++++++----------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 998c4b1..c16fee6 100644 --- a/README.md +++ b/README.md @@ -462,8 +462,11 @@ role. ### `network_provider` -Whether to use `nm` or `initscripts` is detected based on the distribution. -It can be however be explicitly set via `network_.provider` variable. +When Network Manager is running on the target system, the role will use the +`nm` provider and `initscripts` otherwise. The variable `network_provider` +allows to specify a specific provider. Setting it to +`network_provider_os_default` will choose the provider depening on the +operating system. This is usually `nm` except for RHEL 6 or CentOS 6 systems. #### Example diff --git a/defaults/main.yml b/defaults/main.yml index e7a3aff..d3a21d7 100644 --- a/defaults/main.yml +++ b/defaults/main.yml @@ -1,7 +1,14 @@ # SPDX-License-Identifier: BSD-3-Clause -network_provider: "{{ network_provider_default }}" network_connections: [] +# Use initscripts for RHEL/CentOS < 7, nm otherwise +network_provider_os_default: "{{ 'initscripts' if ansible_distribution in ['RedHat', 'CentOS'] and ansible_distribution_major_version |version_compare('7', '<') else 'nm' }}" +# If NetworkManager.service is running, assume that 'nm' is currently in-use, +# otherwise initscripts +network_provider_current: "{{ 'nm' if 'NetworkManager.service' in ansible_facts.services and ansible_facts.services['NetworkManager.service']['state'] == 'running' else 'initscripts' }}" +# Default to the auto-detected value +network_provider: "{{ network_provider_current }}" + # The python-gobject-base packag depends on the python version and # distribution: # - python-gobject-base on RHEL7 (no python2-gobject-base :-/) @@ -27,7 +34,7 @@ network_packages_default_initscripts: "{{ ['ethtool'] + _network_packages_defaul # # Usually, the user only wants to select the "network_provider" # (or not set it at all and let it be autodetected via the -# internal variable "{{ network_provider_default }}". Hence, +# internal variable "{{ network_provider_current }}". Hence, # depending on the "network_provider", a different set of # service-name and packages is chosen. # diff --git a/tasks/main.yml b/tasks/main.yml index ad65506..8d162b3 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -1,16 +1,9 @@ # SPDX-License-Identifier: BSD-3-Clause -- name: Define default provider as nm - set_fact: - network_provider_default: nm +# get service facts, used in defaults/main.yml +- name: Check which services are running + service_facts: -- name: Override default provider to initscipts for older systems - set_fact: - network_provider_default: initscripts - when: - - ansible_distribution_major_version == "6" - - ansible_distribution == "CentOS" or ansible_distribution == "RedHat" - -# needed for ansible_facst.packages +# needed for ansible_facts.packages - name: Check which packages are installed package_facts: @@ -30,6 +23,10 @@ state: started enabled: yes +- name: Print network provider + debug: + msg: "Using network provider: {{ network_provider }}" + - name: Configure networking connection profiles network_connections: provider: "{{ network_provider | mandatory }}" From c279f0afc547bacfb912d738d1fb0b4a8db0a1b6 Mon Sep 17 00:00:00 2001 From: Till Maas Date: Wed, 25 Jul 2018 14:16:33 +0200 Subject: [PATCH 2/2] Run integration tests with other provider, too - Amend and add files to run integration tests against the provider that was not autodetected, too. - Add check to ensure that all integration tests run against both providers - Run black check for all python scripts --- defaults/main.yml | 2 +- tests/ensure_non_running_provider.py | 97 +++++++++++++++++++++++++ tests/tests_bridge_initscripts.yml | 10 --- tests/tests_bridge_other_provider.yml | 13 ++++ tests/tests_default_other_provider.yml | 13 ++++ tests/tests_ethernet.yml | 13 ++++ tests/tests_ethernet_initscripts.yml | 10 --- tests/tests_ethernet_other_provider.yml | 13 ++++ tox.ini | 15 ++-- 9 files changed, 160 insertions(+), 26 deletions(-) create mode 100755 tests/ensure_non_running_provider.py delete mode 100644 tests/tests_bridge_initscripts.yml create mode 100644 tests/tests_bridge_other_provider.yml create mode 100644 tests/tests_default_other_provider.yml delete mode 100644 tests/tests_ethernet_initscripts.yml create mode 100644 tests/tests_ethernet_other_provider.yml diff --git a/defaults/main.yml b/defaults/main.yml index d3a21d7..214bf13 100644 --- a/defaults/main.yml +++ b/defaults/main.yml @@ -9,7 +9,7 @@ network_provider_current: "{{ 'nm' if 'NetworkManager.service' in ansible_facts. # Default to the auto-detected value network_provider: "{{ network_provider_current }}" -# The python-gobject-base packag depends on the python version and +# The python-gobject-base package depends on the python version and # distribution: # - python-gobject-base on RHEL7 (no python2-gobject-base :-/) # - python-gobject-base or python2-gobject-base on Fedora 27 diff --git a/tests/ensure_non_running_provider.py b/tests/ensure_non_running_provider.py new file mode 100755 index 0000000..6038ab3 --- /dev/null +++ b/tests/ensure_non_running_provider.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: BSD-3-Clause +""" Check that there is a playbook to run all role tests with the non-default +provider as well """ +# vim: fileencoding=utf8 + +import glob +import os +import sys + + +import yaml + +OTHER_PROVIDER_SUFFIX = "_other_provider.yml" + +IGNORE = ["tests_unit.yml", "tests_helpers-and-asserts.yml"] + +OTHER_PLAYBOOK = """ +# SPDX-License-Identifier: BSD-3-Clause +--- +- hosts: all + vars: + network_provider_current: + tasks: + # required for the code to set network_provider_current + - service_facts: + - set_fact: + network_provider: "{{{{ 'initscripts' if network_provider_current == 'nm' else 'nm' }}}}" + +- import_playbook: "{tests_playbook}" + when: + - ansible_distribution_major_version != '6' +""" # noqa: E501 # ignore that the line is too long + + +def get_current_provider_code(): + with open("../defaults/main.yml") as defaults: + yaml_defaults = yaml.safe_load(defaults) + current_provider = yaml_defaults["network_provider_current"] + return current_provider + + +def generate_nominal_other_playbook(tests_playbook): + nominal_other_testfile_data = OTHER_PLAYBOOK.format(tests_playbook=tests_playbook) + nominal = yaml.safe_load(nominal_other_testfile_data) + nominal[0]["vars"]["network_provider_current"] = get_current_provider_code() + return yaml.dump(nominal, default_flow_style=False) + + +def main(): + testsfiles = glob.glob("tests_*.yml") + missing = [] + returncode = 0 + + # Generate files when specified + generate = bool(len(sys.argv) > 1 and sys.argv[1] == "generate") + + if not testsfiles: + print("ERROR: No tests found") + returncode = 1 + + for filename in testsfiles: + if filename.endswith(OTHER_PROVIDER_SUFFIX): + continue + + if filename in IGNORE: + continue + + fileroot = os.path.splitext(filename)[0] + other_testfile = fileroot + OTHER_PROVIDER_SUFFIX + nominal_other_testfile_data = generate_nominal_other_playbook(filename) + + if generate: + with open(other_testfile, "w") as ofile: + ofile.write(nominal_other_testfile_data) + + if other_testfile not in testsfiles: + missing.append(filename) + else: + with open(other_testfile) as ifile: + testdata = ifile.read() + if testdata != nominal_other_testfile_data: + print( + "ERROR: Playbook does not match nominal value " + other_testfile + ) + returncode = 1 + + if missing: + print("ERROR: No tests for other provider found for:\n" + ", \n".join(missing)) + print("Try to generate them with '{} generate'".format(sys.argv[0])) + returncode = 1 + + return returncode + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/tests_bridge_initscripts.yml b/tests/tests_bridge_initscripts.yml deleted file mode 100644 index bbab822..0000000 --- a/tests/tests_bridge_initscripts.yml +++ /dev/null @@ -1,10 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause ---- -# empty play to gather facts -- hosts: all - -- import_playbook: tests_bridge.yml - vars: - network_provider: initscripts - when: - - ansible_distribution_major_version != '6' diff --git a/tests/tests_bridge_other_provider.yml b/tests/tests_bridge_other_provider.yml new file mode 100644 index 0000000..e907b0f --- /dev/null +++ b/tests/tests_bridge_other_provider.yml @@ -0,0 +1,13 @@ +- hosts: all + tasks: + - service_facts: null + - set_fact: + network_provider: '{{ ''initscripts'' if network_provider_current == ''nm'' + else ''nm'' }}' + vars: + network_provider_current: '{{ ''nm'' if ''NetworkManager.service'' in ansible_facts.services + and ansible_facts.services[''NetworkManager.service''][''state''] == ''running'' + else ''initscripts'' }}' +- import_playbook: tests_bridge.yml + when: + - ansible_distribution_major_version != '6' diff --git a/tests/tests_default_other_provider.yml b/tests/tests_default_other_provider.yml new file mode 100644 index 0000000..34498d2 --- /dev/null +++ b/tests/tests_default_other_provider.yml @@ -0,0 +1,13 @@ +- hosts: all + tasks: + - service_facts: null + - set_fact: + network_provider: '{{ ''initscripts'' if network_provider_current == ''nm'' + else ''nm'' }}' + vars: + network_provider_current: '{{ ''nm'' if ''NetworkManager.service'' in ansible_facts.services + and ansible_facts.services[''NetworkManager.service''][''state''] == ''running'' + else ''initscripts'' }}' +- import_playbook: tests_default.yml + when: + - ansible_distribution_major_version != '6' diff --git a/tests/tests_ethernet.yml b/tests/tests_ethernet.yml index b4ea31a..cb4459f 100644 --- a/tests/tests_ethernet.yml +++ b/tests/tests_ethernet.yml @@ -1,5 +1,12 @@ # SPDX-License-Identifier: BSD-3-Clause --- +- hosts: all + tasks: + - debug: + msg: Inside ethernet tests + - debug: + var: network_provider + - name: Test configuring ethernet devices hosts: all vars: @@ -28,6 +35,12 @@ address: 192.0.2.1/24 roles: - linux-system-roles.network + +- hosts: all + tasks: + - debug: + var: network_provider + # FIXME: assert profile present # FIXME: assert profile/device up + IP address - import_playbook: down-profile.yml diff --git a/tests/tests_ethernet_initscripts.yml b/tests/tests_ethernet_initscripts.yml deleted file mode 100644 index 8254b7d..0000000 --- a/tests/tests_ethernet_initscripts.yml +++ /dev/null @@ -1,10 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause ---- -# empty play to gather facts -- hosts: all - -- import_playbook: tests_ethernet.yml - vars: - network_provider: initscripts - when: - - ansible_distribution_major_version != '6' diff --git a/tests/tests_ethernet_other_provider.yml b/tests/tests_ethernet_other_provider.yml new file mode 100644 index 0000000..779f993 --- /dev/null +++ b/tests/tests_ethernet_other_provider.yml @@ -0,0 +1,13 @@ +- hosts: all + tasks: + - service_facts: null + - set_fact: + network_provider: '{{ ''initscripts'' if network_provider_current == ''nm'' + else ''nm'' }}' + vars: + network_provider_current: '{{ ''nm'' if ''NetworkManager.service'' in ansible_facts.services + and ansible_facts.services[''NetworkManager.service''][''state''] == ''running'' + else ''initscripts'' }}' +- import_playbook: tests_ethernet.yml + when: + - ansible_distribution_major_version != '6' diff --git a/tox.ini b/tox.ini index dbe1873..369f890 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = black, flake8, pylint, py{26,27,36,37} +envlist = black, flake8, pylint, py{26,27,36,37}, ensure_non_running_provider skipsdist = true skip_missing_interpreters = True @@ -17,15 +17,13 @@ setenv = LC_ALL = C changedir = {toxinidir}/tests covtarget = network_connections -scriptfiles = library/network_connections.py tests/test_network_connections.py pytesttarget = . [testenv:black] basepython = python3.6 deps = black -commands = black --check --diff \ - {[base]scriptfiles} +commands = black --check --diff --include "^[^.].*\.py$" . [testenv:py26] passenv = {[base]passenv} @@ -113,6 +111,13 @@ changedir = {[base]changedir} commands = coveralls +[testenv:ensure_non_running_provider] +basepython = python3.6 +deps = + PyYAML +changedir = {toxinidir}/tests +commands = {toxinidir}/tests/ensure_non_running_provider.py + [pytest] addopts = -rxs @@ -132,5 +137,5 @@ max-line-length = 88 python = 2.6: py26 2.7: py27,coveralls,flake8,pylint - 3.6: py36,black + 3.6: py36,black,ensure_non_running_provider 3.7: py37