Add configurable interface route metric#6447
Conversation
Signed-off-by: David Rapan <david@rapan.cz>
Signed-off-by: David Rapan <david@rapan.cz>
Signed-off-by: David Rapan <david@rapan.cz>
Signed-off-by: David Rapan <david@rapan.cz>
Signed-off-by: David Rapan <david@rapan.cz>
davidrapan
force-pushed
the
dev-route_metric
branch
from
92b230f to
ed3618d
Compare
mdegat01
left a comment
mdegat01
left a comment
There was a problem hiding this comment.
Approved although would like to know why the one test was removed before merging. Generally feature request PRs should minimize changes to existing tests so we can confirm it does not break existing functionality.
Also can you provide some more info on how you're leveraging this from HA? Does this option only make sense if you also use ha network vlan to create multiple interfaces or is there some value in setting it for someone only using the primary interface?
| async def test_api_network_interface_info_default(api_client: TestClient): | ||
| """Test network manager default api.""" | ||
| resp = await api_client.get("/network/interface/default/info") | ||
| result = await resp.json() | ||
| assert result["data"]["ipv4"]["address"][-1] == "192.168.2.148/24" | ||
| assert result["data"]["ipv4"]["gateway"] == "192.168.2.1" | ||
| assert result["data"]["ipv4"]["nameservers"] == ["192.168.2.2"] | ||
| assert result["data"]["ipv4"]["ready"] is True | ||
| assert ( | ||
| result["data"]["ipv6"]["address"][0] == "2a03:169:3df5:0:6be9:2588:b26a:a679/64" | ||
| ) | ||
| assert result["data"]["ipv6"]["address"][1] == "2a03:169:3df5::2f1/128" | ||
| assert result["data"]["ipv6"]["gateway"] == "fe80::da58:d7ff:fe00:9c69" | ||
| assert result["data"]["ipv6"]["nameservers"] == [ | ||
| "2001:1620:2777:1::10", | ||
| "2001:1620:2777:2::20", | ||
| ] | ||
| assert result["data"]["ipv6"]["ready"] is True | ||
| assert result["data"]["interface"] == TEST_INTERFACE_ETH_NAME | ||
| assert result["data"]["mdns"] == "announce" | ||
| assert result["data"]["llmnr"] == "announce" | ||
|
|
||
|
|
There was a problem hiding this comment.
Why was this test dropped?
There was a problem hiding this comment.
Because it's just a special case of:
supervisor/tests/api/test_network.py
Line 72 in a8e5c4f
so I though of optimizing it by dropping it and extending params to also check default interface.
The coverage remains exactly the same. It just reuses existing code.
Edit: I can do it in separate PR, but I think it should be done. :)
Personally, I only use it in the scope of VLANs, yes, but in general I don't see why HA shouldn't work with multiple physical interfaces, where route metrics would also have a say. I just didn't tried that one yet. Edit (some more info about my prod env): I'm also multihoming and the HA has direct access to both ISPs (using VLANs) as one is used only for incoming connections and the other for outgoing (+ the other one also serves as the fallback and vice versa). Edit2: "Is there some value in setting it for someone only using the primary interface?": I don't think so. + I'm not sure if the default value should be |
I was wondering the same in other places a couple weeks back. The optional NetworkManager settings can simply not be set. So I'd make Python |
mdegat01
added
the
needs-client-library
|
PR has needs-client-library, but just like w/ |
mdegat01
removed
the
needs-client-library
3 participants
Proposed change
Add route-metric configuration option, as I recently encountered the need to fine-tune IPv4 route metric in my multi-VLAN environment (lan, iot, public), because I limit internet access in iot network, and had to resort to using
nmcliagain. :)I will follow up w/ documentation and cli PRs after initial review. 😉
Type of change
Additional information
--ipvx-route-metricoption for ha network cli#629Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed: