From 75809bc827bc99659a446b58ab4c76eccdda4904 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. 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 0c690046d..506aa3255 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 5c7e9eaf3..2de85a4e0 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 fa7cf7b11..4627bd19f 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 8209b76e8..4baa321f2 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 518795d31..e4848568e 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 000000000..8ebea27a0 --- /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 a1b9655bc..9b914eb78 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 cfdc01f42..16fd046bd 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,