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