diff --git a/networks/forms.py b/networks/forms.py index b3cc8e4f..691dd8a1 100644 --- a/networks/forms.py +++ b/networks/forms.py @@ -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) diff --git a/networks/templates/create_net_block.html b/networks/templates/create_net_block.html index 0b79c182..4caf6db6 100644 --- a/networks/templates/create_net_block.html +++ b/networks/templates/create_net_block.html @@ -60,7 +60,7 @@
- +
diff --git a/networks/templates/network.html b/networks/templates/network.html index b608d4bd..e67f3ea5 100644 --- a/networks/templates/network.html +++ b/networks/templates/network.html @@ -183,19 +183,21 @@ {% for fix4 in ipv4_fixed_address %}
{% csrf_token %} - + + {% if fix4.mac %}{# an entry is changed by its MAC #} + {% endif %} @@ -294,19 +296,21 @@ {% for fix6 in ipv6_fixed_address %} {% csrf_token %} - + + {% if fix6.id %}{# an entry is changed by its DUID #} + {% endif %} @@ -320,7 +324,7 @@
{% 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%} {% csrf_token %} {% include 'add_network_qos.html' %} @@ -357,7 +361,7 @@ diff --git a/networks/templates/networks.html b/networks/templates/networks.html index 64bbf813..6d3abb9d 100644 --- a/networks/templates/networks.html +++ b/networks/templates/networks.html @@ -71,29 +71,22 @@ {% block script %} {% endblock %} \ No newline at end of file diff --git a/networks/tests/test_network_changes.py b/networks/tests/test_network_changes.py new file mode 100644 index 00000000..c81b0f06 --- /dev/null +++ b/networks/tests/test_network_changes.py @@ -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 = ("n1" + "") + + +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:], ('', flags)) diff --git a/networks/views.py b/networks/views.py index f194f26e..70d25bbc 100644 --- a/networks/views.py +++ b/networks/views.py @@ -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()}, ) diff --git a/vrtManager/network.py b/vrtManager/network.py index 0044b7c1..a12b9c02 100644 --- a/vrtManager/network.py +++ b/vrtManager/network.py @@ -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""" """ 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"""""" + first, last = (int(ipaddress.IPv4Address(ip)) for ip in dhcp4) + for ip in range(first, last + 1): + xml += f"""""" xml += """""" xml += """""" if ipv6: @@ -121,9 +116,12 @@ class wvmNetworks(wvmConnect): xml += """""" xml += """""" xml += """""" - 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'' - if family == "ipv6": - h_id = util.xml_escape(h.get("id") or "") - new_xml = f'' - + # 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):