Merge pull request #74 from tyll/autodetect_provider

Use network manager only when it is running

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
 

defaults/main.yml

@@ -1,8 +1,15 @@
 # SPDX-License-Identifier: BSD-3-Clause
-network_provider:     "{{ network_provider_default }}"
 network_connections:  []
 
-# The python-gobject-base packag depends on the python version and
+# 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 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
@@ -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.
 #

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 }}"

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())

tests/tests_bridge_initscripts.yml

@@ -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'

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'

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'

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

tests/tests_ethernet_initscripts.yml

@@ -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'

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'

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