mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 19:26:48 +00:00
fix(networks): fixed addresses, failed creates and stopped networks behave
- A network created with fixed addresses gets one per address of its DHCP range for any subnet; the old prefix cut assumed a gateway ending in .1 and built addresses such as 10.0.0.1.130. Fixed addresses need a /24 or smaller subnet. - A new network that does not start (its subnet is in use) is undefined again, so the name is free for another try. - Fixed addresses are changed and deleted on a stopped network too: the running network is changed only while it runs. A host with only a name is deleted as defined, not with an empty MAC, shows no "None", and has no edit button it could not use. - The create dialog disables the fields it hides, so an IPv6 subnet typed and then unchecked is not created, and switching from MACVTAP back to BRIDGE restores the bridge label and the Open vSwitch option. - Isolated networks show QoS; QoS save buttons no longer raise a script error; saving an unchanged fixed address says so; two messages lose a stray backslash and spaces.
This commit is contained in:
parent
1262559793
commit
09d7b0c08a
7 changed files with 119 additions and 64 deletions
|
|
@ -61,6 +61,14 @@ class AddNetPool(forms.Form):
|
|||
def clean_subnet(self):
|
||||
return checked_subnet(self.cleaned_data["subnet"], 4)
|
||||
|
||||
def clean(self):
|
||||
cleaned = super().clean()
|
||||
# one fixed address per address of the DHCP range
|
||||
subnet = cleaned.get("subnet")
|
||||
if cleaned.get("fixed") and subnet and ipaddress.ip_network(subnet, strict=False).prefixlen < 24:
|
||||
self.add_error("fixed", _("Fixed addresses need an IPv4 subnet of /24 or smaller"))
|
||||
return cleaned
|
||||
|
||||
def clean_subnet6(self):
|
||||
return checked_subnet(self.cleaned_data["subnet6"], 6)
|
||||
|
||||
|
|
|
|||
|
|
@ -60,7 +60,7 @@
|
|||
<div class="row ipv6_group">
|
||||
<label for="net_subnet6" class="col-sm-4 col-form-label">{% trans "IPv6 Subnet pool" %}</label>
|
||||
<div class="col-sm-6">
|
||||
<input id="net_subnet6" type="text" class="form-control" name="subnet6" value="" placeholder="fd00:dead:baba:1::/64" required pattern="[0-9\/\.]+">
|
||||
<input id="net_subnet6" type="text" class="form-control" name="subnet6" value="" placeholder="fd00:dead:baba:1::/64" required pattern="[0-9a-fA-F:\/]+">
|
||||
</div>
|
||||
</div>
|
||||
<div class="row ipv6_group">
|
||||
|
|
|
|||
|
|
@ -183,19 +183,21 @@
|
|||
{% for fix4 in ipv4_fixed_address %}
|
||||
<tr>
|
||||
<form method="post" role="form" aria-label="IPv4 edit/delete form">{% csrf_token %}
|
||||
<td><input class="form-control" value="{{ fix4.mac }}" name="mac" readonly/></td>
|
||||
<td><input class="form-control" value="{{ fix4.mac|default_if_none:"" }}" name="mac" readonly/></td>
|
||||
<td><input class="form-control" value="{{ fix4.ip }}" name="address" /></td>
|
||||
<td><input class="form-control" value="{{ fix4.name }}" name="name" /></td>
|
||||
<td class="text-nowrap">
|
||||
<input hidden name="family" value="ipv4"/>
|
||||
{% if fix4.mac %}{# an entry is changed by its MAC #}
|
||||
<button type="submit" class="btn btn-sm btn-primary"
|
||||
name="modify_fixed_address"
|
||||
title="{% trans "Edit entry" %}">
|
||||
{% bs_icon 'save' %}
|
||||
</button>
|
||||
{% endif %}
|
||||
<button type="submit" class="btn btn-sm btn-danger"
|
||||
name="delete_fixed_address"
|
||||
title="{% trans "Delete entry" %}" data-confirm="{% blocktrans with mac=fix4.mac %}Delete the fixed address of {{ mac }}? The VM may get another address when its lease ends.{% endblocktrans %}" onclick="return confirm(this.dataset.confirm)">
|
||||
title="{% trans "Delete entry" %}" data-confirm="{% blocktrans with mac=fix4.mac|default:fix4.ip %}Delete the fixed address of {{ mac }}? The VM may get another address when its lease ends.{% endblocktrans %}" onclick="return confirm(this.dataset.confirm)">
|
||||
{% bs_icon 'trash' %}
|
||||
</button>
|
||||
</td>
|
||||
|
|
@ -294,19 +296,21 @@
|
|||
{% for fix6 in ipv6_fixed_address %}
|
||||
<tr>
|
||||
<form method="post" role="form" aria-label="IP v6 edit/delete form">{% csrf_token %}
|
||||
<td><input class="form-control" value="{{ fix6.id }}" name="id" readonly/></td>
|
||||
<td><input class="form-control" value="{{ fix6.id|default_if_none:"" }}" name="id" readonly/></td>
|
||||
<td><input class="form-control" value="{{ fix6.ip }}" name="address" /></td>
|
||||
<td><input class="form-control" value="{{ fix6.name }}" name="name" /></td>
|
||||
<td class="text-nowrap">
|
||||
<input hidden name="family" value="ipv6"/>
|
||||
{% if fix6.id %}{# an entry is changed by its DUID #}
|
||||
<button type="submit" class="btn btn-sm btn-primary"
|
||||
name="modify_fixed_address"
|
||||
title="{% trans "Edit entry" %}">
|
||||
<span>{% bs_icon 'save' %} </span>
|
||||
</button>
|
||||
{% endif %}
|
||||
<button type="submit" class="btn btn-sm btn-danger"
|
||||
name="delete_fixed_address"
|
||||
title="{% trans "Delete entry" %}" data-confirm="{% blocktrans with id=fix6.id %}Delete the fixed address of {{ id }}? The VM may get another address when its lease ends.{% endblocktrans %}" onclick="return confirm(this.dataset.confirm)">
|
||||
title="{% trans "Delete entry" %}" data-confirm="{% blocktrans with id=fix6.id|default:fix6.ip %}Delete the fixed address of {{ id }}? The VM may get another address when its lease ends.{% endblocktrans %}" onclick="return confirm(this.dataset.confirm)">
|
||||
<span>{% bs_icon 'trash' %} </span>
|
||||
</button>
|
||||
</td>
|
||||
|
|
@ -320,7 +324,7 @@
|
|||
</div>
|
||||
{% endif %}
|
||||
|
||||
{% if net_forward.0 == 'route' or net_forward.0 == 'nat' or net_forward.0 == 'isolated' %}
|
||||
{% if net_forward.0 == 'route' or net_forward.0 == 'nat' or not net_forward.0 %}{# an isolated network has no forward #}
|
||||
{% if state == 0 and qos.items|length != 2%}
|
||||
<form method="post" name="set_qos" role="form" aria-label="Create network qos form">{% csrf_token %}
|
||||
{% include 'add_network_qos.html' %}
|
||||
|
|
@ -357,7 +361,7 @@
|
|||
<td class="text-nowrap">
|
||||
<input name="qos_direction" value="{{ q }}" hidden/>
|
||||
<button type="submit" class="btn btn-sm btn-primary"
|
||||
name="set_qos" data-bs-toggle="modal"
|
||||
name="set_qos"
|
||||
title="{% trans "Edit QoS" %}">
|
||||
{% bs_icon 'save' %}
|
||||
</button>
|
||||
|
|
|
|||
|
|
@ -71,29 +71,22 @@
|
|||
{% block script %}
|
||||
<script>
|
||||
$(document).ready(function () {
|
||||
$('#forward_select').change(function (eventObject) {
|
||||
if ($(this).val() == 'bridge') {
|
||||
$('.bridge_name_form_group').show();
|
||||
$('.bridge_name_form_group_dhcp').hide();
|
||||
} else if ($(this).val() == 'macvtap') {
|
||||
$('#bridge_label').text("Dev Name");
|
||||
$('#bridge_name').attr("placeholder", "eth0");
|
||||
$('.bridge_name_form_group').show();
|
||||
$('.bridge_name_form_group_dhcp').hide();
|
||||
$('.openvswitch').hide();
|
||||
} else {
|
||||
$('.bridge_name_form_group').hide();
|
||||
$('.bridge_name_form_group_dhcp').show();
|
||||
}
|
||||
}).change();
|
||||
|
||||
$('#enable_ipv6').change(function (eventObject) {
|
||||
if ($(this).is(':checked')) {
|
||||
$('.ipv6_group').show();
|
||||
} else {
|
||||
$('.ipv6_group').hide();
|
||||
}
|
||||
}).change();
|
||||
// a hidden field is disabled: the browser neither checks nor sends it
|
||||
function show(rows, on) {
|
||||
$(rows).toggle(on).find(':input').prop('disabled', !on);
|
||||
}
|
||||
function update() {
|
||||
var forward = $('#forward_select').val();
|
||||
var bridged = forward == 'bridge' || forward == 'macvtap';
|
||||
show('.bridge_name_form_group', bridged);
|
||||
show('.openvswitch', forward == 'bridge');
|
||||
show('.bridge_name_form_group_dhcp', !bridged);
|
||||
show('.ipv6_group', !bridged && $('#enable_ipv6').is(':checked'));
|
||||
$('#bridge_label').text(forward == 'macvtap' ? "{% trans 'Dev Name'|escapejs %}" : "{% trans 'Bridge Name'|escapejs %}");
|
||||
$('#bridge_name').attr('placeholder', forward == 'macvtap' ? 'eth0' : 'br0');
|
||||
}
|
||||
$('#forward_select, #enable_ipv6').change(update);
|
||||
update();
|
||||
});
|
||||
</script>
|
||||
{% endblock %}
|
||||
57
networks/tests/test_network_changes.py
Normal file
57
networks/tests/test_network_changes.py
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
"""A new network with fixed addresses gets one per address of its DHCP range,
|
||||
for any /24 or smaller subnet; a network that does not start is not left
|
||||
defined; fixed addresses change on a stopped network and may have no MAC."""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from django.test import SimpleTestCase
|
||||
from libvirt import VIR_NETWORK_UPDATE_AFFECT_CONFIG, VIR_NETWORK_UPDATE_AFFECT_LIVE, libvirtError
|
||||
from lxml import etree
|
||||
from networks.forms import AddNetPool
|
||||
from vrtManager.network import network_size, wvmNetwork, wvmNetworks
|
||||
|
||||
NET = ("<network><name>n1</name><ip address='192.0.2.1' netmask='255.255.255.0'><dhcp>"
|
||||
"<range start='192.0.2.2' end='192.0.2.254'/><host name='paul' ip='192.0.2.20'/></dhcp></ip></network>")
|
||||
|
||||
|
||||
class NetworkChangeTests(SimpleTestCase):
|
||||
def setUp(self):
|
||||
patcher = patch("vrtManager.connection.connection_manager.get_connection")
|
||||
self.wvm = patcher.start().return_value
|
||||
self.addCleanup(patcher.stop)
|
||||
|
||||
def create(self, subnet):
|
||||
gateway, mask, dhcp = network_size(subnet, True)
|
||||
wvmNetworks("host", "login", "pw", 1).create_network(
|
||||
"n1", "nat", True, gateway, mask, dhcp, False, None, None, None, "", False, fixed=True)
|
||||
return etree.fromstring(self.wvm.networkDefineXML.call_args.args[0])
|
||||
|
||||
def test_fixed_addresses_cover_the_dhcp_range(self):
|
||||
ip = self.create("192.0.2.128/25").find("ip")
|
||||
# the gateway is the subnet's first address, not one ending in .1
|
||||
self.assertEqual((ip.get("address"), ip.get("netmask")), ("192.0.2.129", "255.255.255.128"))
|
||||
hosts = ip.xpath("dhcp/host/@ip")
|
||||
self.assertEqual((hosts[0], hosts[-1], len(hosts)), ("192.0.2.130", "192.0.2.254", 125))
|
||||
|
||||
def test_fixed_addresses_need_a_small_subnet(self):
|
||||
form = AddNetPool({"name": "n1", "forward": "nat", "subnet": "198.51.0.0/16", "dhcp4": True, "fixed": True})
|
||||
self.assertIn("fixed", form.errors)
|
||||
|
||||
def test_a_network_that_does_not_start_is_undefined(self):
|
||||
net = self.wvm.networkDefineXML.return_value
|
||||
net.create.side_effect = libvirtError("network is already in use by interface virbr0")
|
||||
with self.assertRaises(libvirtError):
|
||||
self.create("192.0.2.0/24")
|
||||
net.undefine.assert_called_once_with()
|
||||
net.setAutostart.assert_not_called()
|
||||
|
||||
def test_a_stopped_network_changes_its_configuration_only(self):
|
||||
net = self.wvm.networkLookupByName.return_value
|
||||
net.XMLDesc.return_value = NET
|
||||
for active, flags in ((False, VIR_NETWORK_UPDATE_AFFECT_CONFIG),
|
||||
(True, VIR_NETWORK_UPDATE_AFFECT_CONFIG | VIR_NETWORK_UPDATE_AFFECT_LIVE)):
|
||||
with self.subTest(active=active):
|
||||
net.isActive.return_value = active
|
||||
wvmNetwork("host", "login", "pw", 1, "n1").delete_fixed_address("192.0.2.20")
|
||||
# a host with only a name is named as it is, without an empty MAC
|
||||
self.assertEqual(net.update.call_args.args[3:], ('<host name="paul" ip="192.0.2.20"/>', flags))
|
||||
|
|
@ -188,12 +188,15 @@ def network(request, compute_id, pool):
|
|||
|
||||
try:
|
||||
with audit(request, compute, subject, f"set fixed address {address}"):
|
||||
conn.modify_fixed_address(name, address, mac_duid, family)
|
||||
messages.success(
|
||||
request,
|
||||
_("Fixed address operation completed for %(family)s")
|
||||
% {"family": family.upper()},
|
||||
)
|
||||
changed = conn.modify_fixed_address(name, address, mac_duid, family)
|
||||
if changed is False:
|
||||
messages.info(request, _("The fixed address is unchanged"))
|
||||
else:
|
||||
messages.success(
|
||||
request,
|
||||
_("Fixed address operation completed for %(family)s")
|
||||
% {"family": family.upper()},
|
||||
)
|
||||
return HttpResponseRedirect(request.get_full_path())
|
||||
except libvirtError as lib_err:
|
||||
messages.error(request, lib_err)
|
||||
|
|
@ -232,7 +235,7 @@ def network(request, compute_id, pool):
|
|||
messages.success(
|
||||
request,
|
||||
_(
|
||||
"Network XML is changed. \\"
|
||||
"Network XML is changed. "
|
||||
"Stop and start network to activate new config."
|
||||
),
|
||||
)
|
||||
|
|
@ -277,8 +280,8 @@ def network(request, compute_id, pool):
|
|||
messages.success(
|
||||
request,
|
||||
_(
|
||||
"%(qos_dir)s QoS is deleted. Network XML is changed. \
|
||||
Stop and start network to activate new config"
|
||||
"%(qos_dir)s QoS is deleted. Network XML is changed. "
|
||||
"Stop and start network to activate new config"
|
||||
)
|
||||
% {"qos_dir": qos_dir.capitalize()},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -50,9 +50,6 @@ class wvmNetworks(wvmConnect):
|
|||
|
||||
return networks
|
||||
|
||||
def define_network(self, xml):
|
||||
self.wvm.networkDefineXML(xml)
|
||||
|
||||
def create_network(
|
||||
self,
|
||||
name,
|
||||
|
|
@ -104,11 +101,9 @@ class wvmNetworks(wvmConnect):
|
|||
xml += f"""<dhcp>
|
||||
<range start='{dhcp4_0}' end='{dhcp4_1}' />"""
|
||||
if fixed:
|
||||
fist_oct = int(dhcp4[0].strip().split(".")[3])
|
||||
last_oct = int(dhcp4[1].strip().split(".")[3])
|
||||
gw_prefix = util.xml_escape(str(gateway)[:-2])
|
||||
for ip in range(fist_oct, last_oct + 1):
|
||||
xml += f"""<host mac='{util.randomMAC()}' ip='{gw_prefix}.{ip}' />"""
|
||||
first, last = (int(ipaddress.IPv4Address(ip)) for ip in dhcp4)
|
||||
for ip in range(first, last + 1):
|
||||
xml += f"""<host mac='{util.randomMAC()}' ip='{ipaddress.IPv4Address(ip)}' />"""
|
||||
xml += """</dhcp>"""
|
||||
xml += """</ip>"""
|
||||
if ipv6:
|
||||
|
|
@ -121,9 +116,12 @@ class wvmNetworks(wvmConnect):
|
|||
xml += """</dhcp>"""
|
||||
xml += """</ip>"""
|
||||
xml += """</network>"""
|
||||
self.define_network(xml)
|
||||
net = self.get_network(name)
|
||||
net.create()
|
||||
net = self.wvm.networkDefineXML(xml)
|
||||
try:
|
||||
net.create()
|
||||
except libvirtError: # e.g. the subnet is in use: the name stays free for another try
|
||||
net.undefine()
|
||||
raise
|
||||
net.setAutostart(1)
|
||||
|
||||
|
||||
|
|
@ -182,7 +180,10 @@ class wvmNetwork(wvmConnect):
|
|||
"start; remove their network cards first")
|
||||
self.net.undefine()
|
||||
|
||||
def update(self, command, section, xml, parentIndex, flags=0):
|
||||
def update(self, command, section, xml, parentIndex):
|
||||
"""Changes the network's configuration and, while it runs, the running
|
||||
network too (libvirt refuses a live change on a stopped one)."""
|
||||
flags = VIR_NETWORK_UPDATE_AFFECT_CONFIG | (VIR_NETWORK_UPDATE_AFFECT_LIVE if self.is_active() else 0)
|
||||
return self.net.update(command, section, parentIndex, xml, flags)
|
||||
|
||||
def get_ip_networks(self):
|
||||
|
|
@ -289,22 +290,13 @@ class wvmNetwork(wvmConnect):
|
|||
parent_index = self.parent_count - 1
|
||||
for h in hosts:
|
||||
if h.get("ip") == ip:
|
||||
h_name = util.xml_escape(h.get("name") or "")
|
||||
name_attr = f' name="{h_name}"' if h.get("name") else ""
|
||||
ip_esc = util.xml_escape(ip)
|
||||
if family == "ipv4":
|
||||
h_mac = util.xml_escape(h.get("mac") or "")
|
||||
new_xml = f'<host mac="{h_mac}"{name_attr} ip="{ip_esc}"/>'
|
||||
if family == "ipv6":
|
||||
h_id = util.xml_escape(h.get("id") or "")
|
||||
new_xml = f'<host id="{h_id}"{name_attr} ip="{ip_esc}"/>'
|
||||
|
||||
# the host as it is: a host may have only a name and an address
|
||||
new_xml = etree.tostring(h, with_tail=False).decode()
|
||||
self.update(
|
||||
VIR_NETWORK_UPDATE_COMMAND_DELETE,
|
||||
VIR_NETWORK_SECTION_IP_DHCP_HOST,
|
||||
new_xml,
|
||||
parent_index,
|
||||
VIR_NETWORK_UPDATE_AFFECT_LIVE | VIR_NETWORK_UPDATE_AFFECT_CONFIG,
|
||||
)
|
||||
break
|
||||
|
||||
|
|
@ -332,7 +324,6 @@ class wvmNetwork(wvmConnect):
|
|||
VIR_NETWORK_SECTION_IP_DHCP_HOST,
|
||||
new_xml,
|
||||
parent_index,
|
||||
VIR_NETWORK_UPDATE_AFFECT_LIVE | VIR_NETWORK_UPDATE_AFFECT_CONFIG,
|
||||
)
|
||||
else:
|
||||
# change the host
|
||||
|
|
@ -344,7 +335,6 @@ class wvmNetwork(wvmConnect):
|
|||
VIR_NETWORK_SECTION_IP_DHCP_HOST,
|
||||
new_xml,
|
||||
parent_index,
|
||||
VIR_NETWORK_UPDATE_AFFECT_LIVE | VIR_NETWORK_UPDATE_AFFECT_CONFIG,
|
||||
)
|
||||
|
||||
def get_qos(self):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue