From c98c17a236f7db1aabd2b67fad8fff772667ab39 Mon Sep 17 00:00:00 2001

From: Gris Ge <fge@redhat.com>

Date: Fri, 21 Jan 2022 18:24:54 +0800

Subject: [PATCH 1/2] Fix problem when switch provider from initscript to nm

Problem:

After `tests_bridge_initscripts.yml` passed, the `tests_bridge_nm.yml`

will fail with NetworkManager 1.18.

Root cause:

 1. The `absent` and `down` action of initscript provider will not

    remove the bridge interface which fail the assertion in

    `tests_bridge_nm.yml`.

 2. In initscript mode, network role will create ifcfg file with

    `NM_CONTROLLED=no` instructing NetworkManager to mark the bridge as

    unmanaged. The follow up `down` and `absent` action of initscript

    provider will not change the NetworkManager's understanding on

    unmanaged state of this interface.

Fixes:

 1. We cannot change existing behaviour of initscript on not deleting

    interface in `down` and `absent` action. So we change the test

    function `tests/playbooks/down_profile.yml` to delete the interface

    manually via `ip link del <ifname>` command.

 2. Use `NM.Client.reload_connections_async()` to reload the

    configuration for nm provider on NetworkManager 1.18.

Previous test infrastructure is running each test file in a brand new VM

or container which cause this problem not been found before.

Dedicate test case `tests/tests_switch_provider.yml` included.

Signed-off-by: Gris Ge <fge@redhat.com>

---

 library/network_connections.py                | 10 +++

 module_utils/network_lsr/nm/provider.py       | 28 ++++++++

 tests/ensure_provider_tests.py                |  1 +

 .../down_profile+delete_interface.yml         |  7 ++

 tests/playbooks/tests_switch_provider.yml     | 66 +++++++++++++++++++

 tests/tests_switch_provider.yml               | 10 +++

 7 files changed, 126 insertions(+)

 create mode 100644 tests/playbooks/down_profile+delete_interface.yml

 create mode 100644 tests/playbooks/tests_switch_provider.yml

 create mode 100644 tests/tests_switch_provider.yml

diff --git a/library/network_connections.py b/library/network_connections.py

index c810b4b..706b198 100644

--- a/library/network_connections.py

+++ b/library/network_connections.py

@@ -2055,6 +2055,16 @@ class Cmd_nm(Cmd):

                 len(self.connections) * DEFAULT_ACTIVATION_TIMEOUT

             )

+        # On NetworkManger 1.18, If user switch from initscripts provider where

+        # NM_CONTROLLED=no defined in ifcfg-ethX file, NetworkManager daemon will treat

+        # that interface as strictly unmanaged, even the follow up deletion of

+        # ifcfg-ethX file cannot change the NetworManager's unmanaged state of this

+        # interface. This will prevent any follow up "nm" provider action on this

+        # interface.  To solve that, we instruct NetworkManager to reload the

+        # configuration.

+        if self._nm_provider.get_client_version().startswith("1.18."):

+            self._nm_provider.reload_configuration()

+

     def rollback_transaction(self, idx, action, error):

         Cmd.rollback_transaction(self, idx, action, error)

         self.on_failure()

diff --git a/module_utils/network_lsr/nm/provider.py b/module_utils/network_lsr/nm/provider.py

index 567c9d1..9d3d491 100644

--- a/module_utils/network_lsr/nm/provider.py

+++ b/module_utils/network_lsr/nm/provider.py

@@ -60,3 +60,31 @@ class NetworkManagerProvider:

     def get_connections(self):

         nm_client = client.get_client()

         return nm_client.get_connections()

+

+    def get_client_version(self):

+        nm_client = client.get_client()

+        return nm_client.get_version()

+

+    def reload_configuration(self):

+        timeout = 10

+        nm_client = client.get_client()

+        main_loop = client.get_mainloop(timeout)

+        logging.debug("Reloading configuration with timeout %s", timeout)

+        nm_client.reload_connections_async(

+            main_loop.cancellable, _reload_config_callback, main_loop

+        )

+        main_loop.run()

+

+

+def _reload_config_callback(nm_client, result, main_loop):

+    try:

+        success = nm_client.reload_connections_finish(result)

+    except client.GLib.Error as e:

+        logging.warn("Failed to reload configuration: %s", e)

+        main_loop.quit()

+        return

+    if success:

+        logging.debug("Reloading configuration finished")

+    else:

+        logging.warn("Failed to reload configuration, no error message")

+    main_loop.quit()

diff --git a/tests/ensure_provider_tests.py b/tests/ensure_provider_tests.py

index e989eed..02844e9 100755

--- a/tests/ensure_provider_tests.py

+++ b/tests/ensure_provider_tests.py

@@ -132,6 +132,7 @@ NM_CONDITIONAL_TESTS = {

 IGNORE = [

     # checked by tests_regression_nm.yml

     "playbooks/tests_checkpoint_cleanup.yml",

+    "playbooks/tests_switch_provider.yml",

 ]

 RUN_PLAYBOOK_WITH_INITSCRIPTS = """# SPDX-License-Identifier: BSD-3-Clause

diff --git a/tests/playbooks/down_profile+delete_interface.yml b/tests/playbooks/down_profile+delete_interface.yml

new file mode 100644

index 0000000..1f5b5d3

--- /dev/null

+++ b/tests/playbooks/down_profile+delete_interface.yml

@@ -0,0 +1,7 @@

+# SPDX-License-Identifier: BSD-3-Clause

+---

+- import_playbook: down_profile.yml

+- name: Delete the interface when the network provider is initscripts

+  hosts: all

+  tasks:

+    - include_tasks: tasks/delete_interface.yml

diff --git a/tests/playbooks/tests_switch_provider.yml b/tests/playbooks/tests_switch_provider.yml

new file mode 100644

index 0000000..f2d165c

--- /dev/null

+++ b/tests/playbooks/tests_switch_provider.yml

@@ -0,0 +1,66 @@

+# SPDX-License-Identifier: BSD-3-Clause

+# This file was generated by ensure_provider_tests.py

+---

+# set network provider and gather facts

+- hosts: all

+  name: Switch initscripts provider to nm

+  vars:

+    interface: LSR-TST-br34

+  tasks:

+    - name: set fact to use initscripts network_provider

+      set_fact:

+        network_provider: initscripts

+      tags:

+        - always

+    - name: "Create test bridge {{ interface }} via initscripts provider"

+      include_role:

+        name: linux-system-roles.network

+      vars:

+        network_connections:

+          - name: "{{ interface }}"

+            state: up

+            type: bridge

+            ip:

+              dhcp4: false

+              auto6: false

+    - include_tasks: tasks/assert_device_present.yml

+    - name: "Take the profile {{ interface }} down via initscripts provider"

+      include_role:

+        name: linux-system-roles.network

+      vars:

+        network_connections:

+          - name: "{{ interface }}"

+            persistent_state: absent

+            state: down

+            type: bridge

+    # The initscripts should not remove the interface for down/absent

+    - include_tasks: tasks/assert_device_present.yml

+    - name: set fact to use nm network_provider

+      set_fact:

+        network_provider: nm

+      tags:

+        - always

+    - name: "Create test bridge {{ interface }} via nm provider"

+      include_role:

+        name: linux-system-roles.network

+      vars:

+        network_connections:

+          - name: "{{ interface }}"

+            state: up

+            type: bridge

+            ip:

+              dhcp4: false

+              auto6: false

+    - include_tasks: tasks/assert_device_present.yml

+    - name: "Remove bridge {{ interface }} via nm provider"

+      include_role:

+        name: linux-system-roles.network

+      vars:

+        network_connections:

+          - name: "{{ interface }}"

+            state: down

+            type: bridge

+    # NetworkManager should not remove pre-exist interface for down/absent

+    - include_tasks: tasks/assert_device_present.yml

+    - include_tasks: tasks/delete_interface.yml

+    - include_tasks: tasks/assert_device_absent.yml

diff --git a/tests/tests_switch_provider.yml b/tests/tests_switch_provider.yml

new file mode 100644

index 0000000..562fbf2

--- /dev/null

+++ b/tests/tests_switch_provider.yml

@@ -0,0 +1,10 @@

+# SPDX-License-Identifier: BSD-3-Clause

+---

+- hosts: all

+  name: Run playbook 'playbooks/tests_switch_provider.yml'

+

+- import_playbook: playbooks/tests_switch_provider.yml

+  when:

+    # The test requires or should run with NetworkManager, therefore it cannot

+    # run on RHEL/CentOS 6

+    - ansible_distribution_major_version != '6'

-- 

2.34.1