From c98c17a236f7db1aabd2b67fad8fff772667ab39 Mon Sep 17 00:00:00 2001 From: Gris Ge 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 ` 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 --- 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