Add network editing with VLAN, firewall, IPv4/IPv6 and undo
Press n in the info screen to edit one interface: bridge, VLAN tag, firewall, IPv4 (DHCP, static or none) and IPv6 (SLAAC, DHCP, static or none). The netN string is parsed, the chosen fields are changed and the string is written back, so hwaddr, model=MAC, mtu, rate, trunks and type are kept. For VMs, IP settings go to cloud-init ipconfigN and only when the VM has a cloud-init drive. A confirm shows the old and new raw values before anything is sent. Press u to restore the values from before the last change to that guest in this session. Review fixes: - Undo entries are cleared on a context switch, because VMIDs repeat across clusters. - has_cloudinit only looks at disks. - CIDR needs a decimal prefix: no netmask form and no /0. - Fields you did not touch are not rewritten. Live test on CT 104: firewall, VLAN 99, IPv6 DHCP and static IPv4 were each applied and undone, and net0 ended identical to the baseline. Going from static back to DHCP left the CT without IPv4 until a reboot, so the confirm now warns that IP changes on a running container may need one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit was merged in pull request #4.
This commit is contained in:
DuckbootandClaude Opus 5.5 committed
1 parent
b874996b1d
commit
5038148686
9 files changed
+1422
-11
No files matched your search
@@ -7,6 +7,8 @@ import json
|
||||
import subprocess
|
||||
|
||||
import pytest
|
||||
from hypothesis import given
|
||||
from hypothesis import strategies as st
|
||||
|
||||
from cv4pve_tui import cli
|
||||
|
||||
@@ -683,3 +685,303 @@ def test_interfaces_ct_lists_all_addresses_incl_global_ipv6(fake):
|
||||
found = cli.interfaces({"vmid": 104, "node": "mainframe", "type": "lxc"})
|
||||
assert found == [{"name": "eth0", "mac": "bc:24:11:36:de:78", "ipv4": ["172.16.41.200/23"],
|
||||
"ipv6": ["2a02:fe1:e77e:6004:be24:11ff:fe36:de78/64", "fe80::be24:11ff:fe36:de78/64"]}] # fmt: skip
|
||||
|
||||
|
||||
# --- delivery B: network editing ----------------------------------------------
|
||||
# spec: docs/plans/guest-tui-mvp.md ("Network editing - delivery B" > cli.py)
|
||||
|
||||
CT_NET = "name=eth0,bridge=vmbr0,hwaddr=BC:24:11:36:DE:78,ip=dhcp,ip6=auto,type=veth"
|
||||
VM_NET_LINK = "virtio=02:24:A6:49:A8:25,bridge=vmbr0"
|
||||
|
||||
|
||||
def test_bridges_filters_bridge_types_sorted_and_uses_node_network_argv(fake):
|
||||
fake.result = (
|
||||
json.dumps(
|
||||
[
|
||||
{"iface": "vmbr1", "type": "bridge", "active": 1},
|
||||
{"iface": "eno1", "type": "eth", "active": 1},
|
||||
{"iface": "ovs0", "type": "OVSBridge", "active": 1},
|
||||
{"iface": "bond0", "type": "bond", "active": 1},
|
||||
{"iface": "vmbr0", "type": "bridge", "active": 1},
|
||||
]
|
||||
),
|
||||
"",
|
||||
0,
|
||||
)
|
||||
assert cli.bridges(VM) == ["ovs0", "vmbr0", "vmbr1"]
|
||||
assert fake.argv == [
|
||||
"cv4pve-cli",
|
||||
"get",
|
||||
"node",
|
||||
"network",
|
||||
"mainframe",
|
||||
"-o",
|
||||
"json",
|
||||
]
|
||||
|
||||
|
||||
def test_bridges_none_present_gives_empty_list(fake):
|
||||
fake.result = (json.dumps([{"iface": "eno1", "type": "eth", "active": 1}]), "", 0)
|
||||
assert cli.bridges(CT) == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("sample", [CT_NET, VM_NET_LINK, "ip=dhcp"])
|
||||
def test_format_net_round_trips_real_samples(sample):
|
||||
assert cli.format_net(cli.parse_net(sample)) == sample
|
||||
|
||||
|
||||
_token = st.text(
|
||||
alphabet=st.characters(
|
||||
blacklist_characters=",=", blacklist_categories=("Cs", "Cc")
|
||||
),
|
||||
min_size=1,
|
||||
)
|
||||
|
||||
|
||||
@given(
|
||||
st.lists(
|
||||
st.tuples(_token, _token), min_size=1, max_size=8, unique_by=lambda kv: kv[0]
|
||||
)
|
||||
)
|
||||
def test_format_net_parse_net_round_trip_property(pairs):
|
||||
s = ",".join(f"{k}={v}" for k, v in pairs)
|
||||
assert cli.format_net(cli.parse_net(s)) == s
|
||||
|
||||
|
||||
def test_format_net_keeps_dict_order():
|
||||
assert cli.format_net({"b": "1", "a": "2"}) == "b=1,a=2"
|
||||
|
||||
|
||||
def test_has_cloudinit_true_when_a_disk_value_mentions_cloudinit():
|
||||
assert cli.has_cloudinit(
|
||||
{"ide2": "local-lvm:vm-108-cloudinit,media=cdrom", "cores": 2}
|
||||
)
|
||||
|
||||
|
||||
def test_has_cloudinit_false_without_cloudinit_drive():
|
||||
assert not cli.has_cloudinit(
|
||||
{"scsi0": "local-lvm:vm-108-disk-0,size=10G", "cores": 2}
|
||||
)
|
||||
|
||||
|
||||
# apply_link
|
||||
|
||||
|
||||
def test_apply_link_sets_bridge_and_returns_a_copy():
|
||||
net = cli.parse_net(CT_NET)
|
||||
out = cli.apply_link(net, "vmbr1", None, False)
|
||||
assert out["bridge"] == "vmbr1"
|
||||
assert net == cli.parse_net(CT_NET) # input untouched
|
||||
|
||||
|
||||
def test_apply_link_new_tag_and_firewall_are_appended_after_existing_keys():
|
||||
out = cli.apply_link(cli.parse_net(CT_NET), "vmbr0", 99, True)
|
||||
assert list(out)[-2:] == ["tag", "firewall"] or list(out)[-2:] == [
|
||||
"firewall",
|
||||
"tag",
|
||||
]
|
||||
assert out["tag"] == "99" and out["firewall"] == "1"
|
||||
assert list(out)[:6] == ["name", "bridge", "hwaddr", "ip", "ip6", "type"]
|
||||
|
||||
|
||||
def test_apply_link_none_tag_and_false_firewall_remove_the_keys():
|
||||
net = cli.parse_net("virtio=02:24:A6:49:A8:25,bridge=vmbr0,tag=20,firewall=1")
|
||||
out = cli.apply_link(net, "vmbr0", None, False)
|
||||
assert out == {"virtio": "02:24:A6:49:A8:25", "bridge": "vmbr0"}
|
||||
|
||||
|
||||
def test_apply_link_false_firewall_removes_firewall_zero_too():
|
||||
out = cli.apply_link(cli.parse_net(CT_NET + ",firewall=0"), "vmbr0", None, False)
|
||||
assert "firewall" not in out
|
||||
|
||||
|
||||
def test_apply_link_existing_keys_keep_position_and_unrelated_keys_untouched():
|
||||
src = "e1000=02:24:A6:49:A8:25,bridge=vmbr0,tag=20,mtu=1400,rate=10,trunks=10;20,firewall=1"
|
||||
out = cli.apply_link(cli.parse_net(src), "vmbr2", 30, True)
|
||||
assert cli.format_net(out) == (
|
||||
"e1000=02:24:A6:49:A8:25,bridge=vmbr2,tag=30,mtu=1400,rate=10,trunks=10;20,firewall=1"
|
||||
)
|
||||
|
||||
|
||||
def test_apply_link_keeps_lxc_hwaddr_and_ip_keys():
|
||||
out = cli.apply_link(cli.parse_net(CT_NET), "vmbr1", 5, False)
|
||||
for key in ("hwaddr", "ip", "ip6", "name", "type"):
|
||||
assert out[key] == cli.parse_net(CT_NET)[key]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tag", [1, 4094])
|
||||
def test_apply_link_accepts_tag_boundaries(tag):
|
||||
assert cli.apply_link({"bridge": "vmbr0"}, "vmbr0", tag, False)["tag"] == str(tag)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tag", [0, -1, 4095, 5000])
|
||||
def test_apply_link_rejects_tag_outside_1_to_4094(tag):
|
||||
with pytest.raises(ValueError):
|
||||
cli.apply_link({"bridge": "vmbr0"}, "vmbr0", tag, False)
|
||||
|
||||
|
||||
# ip_mode
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("cfg", "family", "expected"),
|
||||
[
|
||||
({"ip": "dhcp", "ip6": "auto"}, 4, ("dhcp", "", "")),
|
||||
({"ip": "dhcp", "ip6": "auto"}, 6, ("auto", "", "")),
|
||||
(
|
||||
{"ip": "10.0.0.5/24", "gw": "10.0.0.1"},
|
||||
4,
|
||||
("static", "10.0.0.5/24", "10.0.0.1"),
|
||||
),
|
||||
({"ip": "10.0.0.5/24"}, 4, ("static", "10.0.0.5/24", "")),
|
||||
(
|
||||
{"ip6": "2001:db8::5/64", "gw6": "2001:db8::1"},
|
||||
6,
|
||||
("static", "2001:db8::5/64", "2001:db8::1"),
|
||||
),
|
||||
({"ip6": "dhcp"}, 6, ("dhcp", "", "")),
|
||||
({"ip": "manual", "ip6": "manual"}, 4, ("none", "", "")),
|
||||
({"ip": "manual", "ip6": "manual"}, 6, ("none", "", "")),
|
||||
({"ip": "dhcp"}, 6, ("none", "", "")), # key absent reads as none
|
||||
({"ip6": "auto"}, 4, ("none", "", "")), # families are independent
|
||||
({"ip": "10.0.0.5/24", "gw": "10.0.0.1"}, 6, ("none", "", "")),
|
||||
],
|
||||
)
|
||||
def test_ip_mode_reads_mode_cidr_and_gateway(cfg, family, expected):
|
||||
assert cli.ip_mode(cfg, family) == expected
|
||||
|
||||
|
||||
# apply_ip
|
||||
|
||||
|
||||
def test_apply_ip_dhcp_sets_dhcp_and_removes_gateway_keeping_other_keys():
|
||||
cfg = {"name": "eth0", "ip": "10.0.0.5/24", "gw": "10.0.0.1", "ip6": "auto"}
|
||||
out = cli.apply_ip(cfg, 4, "dhcp")
|
||||
assert out == {"name": "eth0", "ip": "dhcp", "ip6": "auto"}
|
||||
assert cfg["ip"] == "10.0.0.5/24" # input untouched
|
||||
|
||||
|
||||
def test_apply_ip_auto_for_family_6_removes_gw6():
|
||||
out = cli.apply_ip(
|
||||
{"ip": "dhcp", "ip6": "2001:db8::5/64", "gw6": "2001:db8::1"}, 6, "auto"
|
||||
)
|
||||
assert out == {"ip": "dhcp", "ip6": "auto"}
|
||||
|
||||
|
||||
def test_apply_ip_dhcp_for_family_6():
|
||||
assert cli.apply_ip({"ip6": "auto"}, 6, "dhcp") == {"ip6": "dhcp"}
|
||||
|
||||
|
||||
def test_apply_ip_static_v4_with_gateway():
|
||||
out = cli.apply_ip(
|
||||
{"ip": "dhcp", "ip6": "auto"}, 4, "static", "172.16.41.200/23", "172.16.40.1"
|
||||
)
|
||||
assert out == {"ip": "172.16.41.200/23", "gw": "172.16.40.1", "ip6": "auto"}
|
||||
|
||||
|
||||
def test_apply_ip_static_v4_without_gateway_removes_old_gateway():
|
||||
out = cli.apply_ip(
|
||||
{"ip": "10.0.0.5/24", "gw": "10.0.0.1"}, 4, "static", "10.0.0.9/24"
|
||||
)
|
||||
assert out == {"ip": "10.0.0.9/24"}
|
||||
|
||||
|
||||
def test_apply_ip_static_v6_with_and_without_gateway():
|
||||
with_gw = cli.apply_ip(
|
||||
{"ip6": "auto"}, 6, "static", "2001:db8::5/64", "2001:db8::1"
|
||||
)
|
||||
assert with_gw == {"ip6": "2001:db8::5/64", "gw6": "2001:db8::1"}
|
||||
no_gw = cli.apply_ip(with_gw, 6, "static", "2001:db8::6/64")
|
||||
assert no_gw == {"ip6": "2001:db8::6/64"}
|
||||
|
||||
|
||||
def test_apply_ip_none_removes_ip_and_gateway_of_that_family_only():
|
||||
cfg = {
|
||||
"ip": "10.0.0.5/24",
|
||||
"gw": "10.0.0.1",
|
||||
"ip6": "2001:db8::5/64",
|
||||
"gw6": "2001:db8::1",
|
||||
}
|
||||
assert cli.apply_ip(cfg, 4, "none") == {
|
||||
"ip6": "2001:db8::5/64",
|
||||
"gw6": "2001:db8::1",
|
||||
}
|
||||
assert cli.apply_ip(cfg, 6, "none") == {"ip": "10.0.0.5/24", "gw": "10.0.0.1"}
|
||||
|
||||
|
||||
def test_apply_ip_none_on_vm_ipconfig_can_leave_empty_dict():
|
||||
assert cli.apply_ip({"ip": "dhcp"}, 4, "none") == {}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("family", "mode", "cidr", "gw"),
|
||||
[
|
||||
(4, "static", "2001:db8::5/64", ""), # wrong family
|
||||
(6, "static", "10.0.0.5/24", ""), # wrong family
|
||||
(4, "static", "10.0.0.5", ""), # missing prefix
|
||||
(6, "static", "2001:db8::5", ""), # missing prefix
|
||||
(4, "static", "", ""), # missing CIDR
|
||||
(4, "static", "not-an-ip/24", ""),
|
||||
(4, "static", "10.0.0.5/24", "nonsense"), # bad gateway
|
||||
(4, "static", "10.0.0.5/24", "2001:db8::1"), # gateway family mismatch
|
||||
(6, "static", "2001:db8::5/64", "10.0.0.1"), # gateway family mismatch
|
||||
(4, "auto", "", ""), # SLAAC exists only for IPv6
|
||||
],
|
||||
)
|
||||
def test_apply_ip_validation_errors(family, mode, cidr, gw):
|
||||
with pytest.raises(ValueError):
|
||||
cli.apply_ip({"ip": "dhcp"}, family, mode, cidr, gw)
|
||||
|
||||
|
||||
# set_net
|
||||
|
||||
|
||||
def test_set_net_sets_only(fake):
|
||||
cli.set_net(CT, {"net0": CT_NET + ",tag=99"})
|
||||
assert fake.argv == ["cv4pve-cli", "set", "ct", "config", "mainframe", "100",
|
||||
"--net0", CT_NET + ",tag=99", "--wait"] # fmt: skip
|
||||
|
||||
|
||||
def test_set_net_deletes_only(fake):
|
||||
cli.set_net(VM, {"ipconfig0": None})
|
||||
assert fake.argv == ["cv4pve-cli", "set", "vm", "config", "mainframe", "108",
|
||||
"--delete", "ipconfig0", "--wait"] # fmt: skip
|
||||
|
||||
|
||||
def test_set_net_sets_then_deletes_joined_with_comma_and_wait_last(fake):
|
||||
cli.set_net(
|
||||
VM, {"net0": VM_NET_LINK + ",tag=5", "ipconfig0": None, "ipconfig1": None}
|
||||
)
|
||||
assert fake.argv == ["cv4pve-cli", "set", "vm", "config", "mainframe", "108",
|
||||
"--net0", VM_NET_LINK + ",tag=5", "--delete", "ipconfig0,ipconfig1", "--wait"] # fmt: skip
|
||||
|
||||
|
||||
def test_set_net_empty_changes_is_value_error_without_call(fake):
|
||||
with pytest.raises(ValueError):
|
||||
cli.set_net(VM, {})
|
||||
assert fake.calls == []
|
||||
|
||||
|
||||
def test_set_net_failure_raises_clierror(monkeypatch):
|
||||
FakeRun(monkeypatch, "Error: boom", returncode=1)
|
||||
with pytest.raises(cli.CliError):
|
||||
cli.set_net(CT, {"net0": CT_NET})
|
||||
|
||||
|
||||
# --- review findings: network editing -----------------------------------------
|
||||
|
||||
|
||||
def test_has_cloudinit_only_looks_at_disks():
|
||||
assert cli.has_cloudinit({"ide2": "local-lvm:vm-108-cloudinit,media=cdrom"})
|
||||
assert not cli.has_cloudinit(
|
||||
{"name": "ubuntu-cloudinit-tpl", "description": "cloudinit base"}
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"cidr", ["10.0.0.5/255.255.255.0", "10.0.0.5/0", "2001:db8::5/0"]
|
||||
)
|
||||
def test_apply_ip_rejects_netmask_form_and_zero_prefix(cidr):
|
||||
family = 6 if ":" in cidr else 4
|
||||
with pytest.raises(ValueError):
|
||||
cli.apply_ip({}, family, "static", cidr)
|
||||
Reference in new issue
Block a user