linux-system-roles / network

An ansible role to configure networking
https://linux-system-roles.github.io/network/
BSD 3-Clause "New" or "Revised" License
238 stars 106 forks source link

feat: Support `wait_ip` property #741

Closed liangwen12year closed 1 month ago

liangwen12year commented 1 month ago

Enhancement:

Add support for the wait_ip property, the system will consider connection activated only when specific IP stack is configured. This enables flexibility in scenarios such as IPv6-only networks, where the overall network configuration can still succeed when IPv4 configuration fails but IPv6 completes successfully.

The wait_ip can be configured with the following possible values:

Reason:

Result:

Issue Tracker Tickets (Jira or BZ if any):

Resolves: https://issues.redhat.com/browse/RHEL-63026

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.

Project coverage is 43.21%. Comparing base (89d7148) to head (624bca5). Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
library/network_connections.py 0.00% 4 Missing :warning:
module_utils/network_lsr/argument_validator.py 33.33% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #741 +/- ## =========================================== + Coverage 20.40% 43.21% +22.81% =========================================== Files 10 12 +2 Lines 1485 3117 +1632 Branches 436 0 -436 =========================================== + Hits 303 1347 +1044 - Misses 1181 1770 +589 + Partials 1 0 -1 ``` | [Flag](https://app.codecov.io/gh/linux-system-roles/network/pull/741/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles) | Coverage Δ | | |---|---|---| | [sanity](https://app.codecov.io/gh/linux-system-roles/network/pull/741/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

liangwen12year commented 1 month ago

[citest]

liangwen12year commented 1 month ago

[citest]

spetrosi commented 1 month ago

[citest]

spetrosi commented 1 month ago

[citest_bad]

spetrosi commented 1 month ago

[citest]

tyll commented 1 month ago

It seems more user friendly to use the Nmstate syntax here with wait_ip: any|ipv4|ipv6|ipv4+ipv6 - https://nmstate.io/devel/yaml_api.html

liangwen12year commented 1 month ago

It seems more user friendly to use the Nmstate syntax here with wait_ip: any|ipv4|ipv6|ipv4+ipv6 - https://nmstate.io/devel/yaml_api.html

I changed it to align with nmstate schema.

liangwen12year commented 1 month ago

[citest]

richm commented 1 month ago

Added support for the may_fail4 and may_fail6 properties, allowing the network configuration to proceed even if the corresponding IP configuration times out. This enables flexibility in scenarios such as IPv6-only networks, where the overall network configuration can still succeed when IPv4 configuration fails but IPv6 completes successfully.

This should be changed to describe wait_ip

liangwen12year commented 1 month ago

Added support for the may_fail4 and may_fail6 properties, allowing the network configuration to proceed even if the corresponding IP configuration times out. This enables flexibility in scenarios such as IPv6-only networks, where the overall network configuration can still succeed when IPv4 configuration fails but IPv6 completes successfully.

This should be changed to describe wait_ip

I updated it, thanks.

liangwen12year commented 1 month ago

[citest]

richm commented 1 month ago

@liangwen12year ready to merge?

liangwen12year commented 1 month ago

Approved, through I don't like we are expanding network_connections, especially we are introducing nmstate stuff into this NM specific schema. Use who request this should use nmstate schema instead of request RFE to network_connections.

I actually requested the issue reporter to use the network_state variable, but the reporter has not tried it yet, https://github.com/linux-system-roles/network/issues/735#issuecomment-2394097149. I also updated the ReadMe to reflect that the future of the network role is promoting using network_state variable instead of the network_connections, hopefully, we will have fewer feature requests in the network_connections variable when the feature is available in the network_state .