From 5d73511d6439a6a986c6ad2937ab120427145b07 Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Thu, 3 Oct 2024 22:19:59 -0400 Subject: [PATCH] feat: Support `autoconnect_retries` There is no fine-grained control over the number of retries for automatically reconnecting a network connection in the role. This limitation can be problematic for certain use cases where extending the retry process is critical, particularly in environments with unstable networks. Introduce support for the `autoconnect_retries` property in nm provider of `network_connections` variable. This feature allows users to configure how many times NetworkManager will attempt to reconnect a connection after a autoconnect failure, providing more control over network stability and performance. Resolves: https://issues.redhat.com/browse/RHEL-61599 Signed-off-by: Wen Liang --- README.md | 9 +++++ examples/eth_simple_auto.yml | 1 + library/network_connections.py | 3 ++ .../network_lsr/argument_validator.py | 6 ++++ tests/playbooks/tests_dummy.yml | 2 ++ tests/tasks/assert_autoconnect_retries.yml | 16 +++++++++ tests/tasks/create_dummy_profile.yml | 1 + tests/unit/test_network_connections.py | 36 +++++++++++++++++++ 8 files changed, 74 insertions(+) create mode 100644 tests/tasks/assert_autoconnect_retries.yml diff --git a/README.md b/README.md index 0c690046..506aa325 100644 --- a/README.md +++ b/README.md @@ -378,6 +378,15 @@ By default, profiles are created with autoconnect enabled. - For `initscripts`, this corresponds to the `ONBOOT` property. +### `autoconnect_retries` + +The number of times a connection should be tried when autoactivating before giving up. +Zero means forever, -1 means the global default in NetworkManager (4 times if not +overridden). Setting this to 1 means to try activation only once before blocking +autoconnect. Note that after a timeout, NetworkManager will try to autoconnect again. + +- For `NetworkManager`, this corresponds to the `connection.autoconnect-retries` property. + ### `mac` The `mac` address is optional and restricts the profile to be usable only on diff --git a/examples/eth_simple_auto.yml b/examples/eth_simple_auto.yml index 5c7e9eaf..2de85a4e 100644 --- a/examples/eth_simple_auto.yml +++ b/examples/eth_simple_auto.yml @@ -11,6 +11,7 @@ state: up type: ethernet autoconnect: true + autoconnect_retries: 2 mac: "{{ network_mac1 }}" mtu: 1450 diff --git a/library/network_connections.py b/library/network_connections.py index fa7cf7b1..4627bd19 100644 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -906,6 +906,9 @@ def connection_create(self, connections, idx, connection_current=None): s_con.set_property(NM.SETTING_CONNECTION_ID, connection["name"]) s_con.set_property(NM.SETTING_CONNECTION_UUID, connection["nm.uuid"]) s_con.set_property(NM.SETTING_CONNECTION_AUTOCONNECT, connection["autoconnect"]) + s_con.set_property( + NM.SETTING_CONNECTION_AUTOCONNECT_RETRIES, connection["autoconnect_retries"] + ) s_con.set_property( NM.SETTING_CONNECTION_INTERFACE_NAME, connection["interface_name"] ) diff --git a/module_utils/network_lsr/argument_validator.py b/module_utils/network_lsr/argument_validator.py index 8209b76e..4baa321f 100644 --- a/module_utils/network_lsr/argument_validator.py +++ b/module_utils/network_lsr/argument_validator.py @@ -1866,6 +1866,12 @@ def __init__(self): "type", enum_values=ArgValidator_DictConnection.VALID_TYPES ), ArgValidatorBool("autoconnect", default_value=True), + ArgValidatorNum( + "autoconnect_retries", + val_min=0, + val_max=UINT32_MAX, + default_value=-1, + ), ArgValidatorStr( "port_type", enum_values=ArgValidator_DictConnection.VALID_PORT_TYPES, diff --git a/tests/playbooks/tests_dummy.yml b/tests/playbooks/tests_dummy.yml index 518795d3..e4848568 100644 --- a/tests/playbooks/tests_dummy.yml +++ b/tests/playbooks/tests_dummy.yml @@ -3,6 +3,7 @@ - name: Play for testing dummy connection hosts: all vars: + autocon_retries: 2 interface: dummy0 profile: "{{ interface }}" lsr_fail_debug: @@ -30,6 +31,7 @@ lsr_assert: - tasks/assert_profile_present.yml - tasks/assert_device_present.yml + - tasks/assert_autoconnect_retries.yml lsr_cleanup: - tasks/cleanup_profile+device.yml - tasks/check_network_dns.yml diff --git a/tests/tasks/assert_autoconnect_retries.yml b/tests/tasks/assert_autoconnect_retries.yml new file mode 100644 index 00000000..8ebea27a --- /dev/null +++ b/tests/tasks/assert_autoconnect_retries.yml @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- name: Get autoconnect retries + command: > + nmcli -f connection.autoconnect-retries connection show {{ profile }} + register: autoconnect_retries + ignore_errors: true + changed_when: false +- name: "Assert that autoconnect-retries is configured as specified" + assert: + that: + - autoconnect_retries.stdout.split(":")[1] | trim + == autocon_retries | string + msg: "autoconnect-retries is configured as + {{ autoconnect_retries.stdout.split(':')[1] | trim }} + but specified as {{ autocon_retries }}" diff --git a/tests/tasks/create_dummy_profile.yml b/tests/tasks/create_dummy_profile.yml index a1b9655b..9b914eb7 100644 --- a/tests/tasks/create_dummy_profile.yml +++ b/tests/tasks/create_dummy_profile.yml @@ -6,6 +6,7 @@ vars: network_connections: - name: "{{ interface }}" + autoconnect_retries: "{{ autocon_retries }}" state: up type: dummy ip: diff --git a/tests/unit/test_network_connections.py b/tests/unit/test_network_connections.py index cfdc01f4..16fd046b 100644 --- a/tests/unit/test_network_connections.py +++ b/tests/unit/test_network_connections.py @@ -177,6 +177,7 @@ def setUp(self): # default values when "type" is specified and state is not self.default_connection_settings = { "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -591,6 +592,7 @@ def test_ethernet_two_defaults(self): { "actions": ["present"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -651,6 +653,7 @@ def test_up_ethernet(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -706,6 +709,7 @@ def test_up_ethernet_no_autoconnect(self): { "actions": ["present", "up"], "autoconnect": False, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -798,6 +802,7 @@ def test_up_ethernet_mac_mtu_static_ip(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -870,6 +875,7 @@ def test_up_single_v4_dns(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -939,6 +945,7 @@ def test_ipv6_static(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1047,6 +1054,7 @@ def test_routes(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1105,6 +1113,7 @@ def test_routes(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1209,6 +1218,7 @@ def test_auto_gateway_true(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1290,6 +1300,7 @@ def test_auto_gateway_false(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1389,6 +1400,7 @@ def test_vlan(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1447,6 +1459,7 @@ def test_vlan(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1551,6 +1564,7 @@ def test_macvlan(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1604,6 +1618,7 @@ def test_macvlan(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethtool": ETHTOOL_DEFAULTS, "force_state_change": None, @@ -1668,6 +1683,7 @@ def test_macvlan(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethtool": ETHTOOL_DEFAULTS, "force_state_change": None, @@ -1781,6 +1797,7 @@ def test_bridge_no_dhcp4_auto6(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1828,6 +1845,7 @@ def test_bridge_no_dhcp4_auto6(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -1898,6 +1916,7 @@ def test_bond(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "bond": { "mode": "balance-rr", "ad_actor_sys_prio": None, @@ -1981,6 +2000,7 @@ def test_bond_active_backup(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "bond": { "mode": "active-backup", "ad_actor_sys_prio": None, @@ -2076,6 +2096,7 @@ def test_ethernet_mac_address(self): { "actions": ["present"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -2129,6 +2150,7 @@ def test_ethernet_speed_settings(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": {"autoneg": False, "duplex": "half", "speed": 400}, "ethtool": ETHTOOL_DEFAULTS, @@ -2211,6 +2233,7 @@ def test_bridge2(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -2258,6 +2281,7 @@ def test_bridge2(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -2321,6 +2345,7 @@ def test_infiniband(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethtool": ETHTOOL_DEFAULTS, "force_state_change": None, @@ -2401,6 +2426,7 @@ def test_infiniband2(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethtool": ETHTOOL_DEFAULTS, "force_state_change": None, @@ -2488,6 +2514,7 @@ def test_infiniband3(self): { "actions": ["present"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethtool": ETHTOOL_DEFAULTS, "ignore_errors": None, @@ -2533,6 +2560,7 @@ def test_infiniband3(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethtool": ETHTOOL_DEFAULTS, "force_state_change": None, @@ -2644,6 +2672,7 @@ def test_route_metric_prefix(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -2752,6 +2781,7 @@ def test_route_v6(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -2911,6 +2941,7 @@ def test_route_without_interface_name(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -3071,6 +3102,7 @@ def test_802_1x_1(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -3155,6 +3187,7 @@ def test_802_1x_2(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -3239,6 +3272,7 @@ def test_802_1x_3(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethernet": ETHERNET_DEFAULTS, "ethtool": ETHTOOL_DEFAULTS, @@ -3322,6 +3356,7 @@ def test_wireless_psk(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethtool": ETHTOOL_DEFAULTS, "force_state_change": None, @@ -3394,6 +3429,7 @@ def test_wireless_eap(self): { "actions": ["present", "up"], "autoconnect": True, + "autoconnect_retries": -1, "check_iface_exists": True, "ethtool": ETHTOOL_DEFAULTS, "force_state_change": None,