[PATCH] [Kimchi 0/2] Add support for editing virtual networks
Lucio Correia (2): Add backend support for editing virtual networks Add network rename tests API.json | 31 ++++++++++++++++++++++++++++ control/networks.py | 1 + docs/API.md | 8 ++++++++ i18n.py | 9 +++++--- model/networks.py | 50 ++++++++++++++++++++++++++++++++++++++++++++- tests/test_model_network.py | 13 +++++++++++- 6 files changed, 107 insertions(+), 5 deletions(-) -- 1.9.1
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- API.json | 31 +++++++++++++++++++++++++++++++ control/networks.py | 1 + docs/API.md | 8 ++++++++ i18n.py | 9 ++++++--- model/networks.py | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 95 insertions(+), 4 deletions(-) diff --git a/API.json b/API.json index 6f0b484..2205be6 100644 --- a/API.json +++ b/API.json @@ -395,6 +395,37 @@ } } }, + "network_update": { + "type": "object", + "error": "KCHAPI0001E", + "properties": { + "name": { + "description": "The new name of the network", + "type": "string", + "minLength": 1, + "pattern": "^[^/\"]*$", + "required": true, + "error": "KCHNET0011E" + }, + "subnet": { + "description": "Network segment in slash-separated format with ip address and prefix or netmask", + "type": "string", + "error": "KCHNET0013E" + }, + "interfaces": { + "description": "An array of network interfaces of the host", + "type": "array", + "error": "KCHNET0014E" + }, + "vlan_id": { + "description": "Network's VLAN ID", + "type": "integer", + "maximum": 4094, + "minimum": 1, + "error": "KCHNET0015E" + } + } + }, "vmifaces_create": { "type": "object", "error": "KCHVMIF0007E", diff --git a/control/networks.py b/control/networks.py index 8ba2206..c87b5a6 100644 --- a/control/networks.py +++ b/control/networks.py @@ -27,6 +27,7 @@ NETWORKS_REQUESTS = { NETWORK_REQUESTS = { 'DELETE': {'default': "Remove virtual network '%(ident)s'"}, + 'PUT': {'default': "Update virtual network '%(ident)s'"}, 'POST': { 'activate': "Activate virtual network '%(ident)s'", 'deactivate': "Deactivate virtual network '%(ident)s'", diff --git a/docs/API.md b/docs/API.md index f1a13b4..491e887 100644 --- a/docs/API.md +++ b/docs/API.md @@ -736,6 +736,14 @@ A interface represents available interface on host. * **DELETE**: Remove the Network * **POST**: *See Network Actions* +* **PUT**: Update the Network. The network type cannot be changed. + * name *(optional)*: The new name of the Network + * subnet *(optional)*: Network segment in slash-separated format with ip address and prefix. + Applies only to NAT and isolated networks + * interfaces *(optional)*: An array of network interfaces that belongs to this network. + Applies only to bridge, macvtap and VEPA networks. + * vlan_id *(optional)*: VLAN tagging ID for the bridge network. + **Actions (POST):** * activate: Activate an inactive Network diff --git a/i18n.py b/i18n.py index 6214687..b413dcd 100644 --- a/i18n.py +++ b/i18n.py @@ -255,10 +255,10 @@ messages = { "KCHNET0002E": _("Network %(name)s does not exist"), "KCHNET0003E": _("Subnet %(subnet)s specified for network %(network)s is not valid."), "KCHNET0004E": _("Specify a network interface to create bridged or macvtap networks."), - "KCHNET0005E": _("Unable to delete active network %(name)s"), + "KCHNET0005E": _("Unable to delete or update active network %(name)s"), "KCHNET0006E": _("Interface %(iface)s specified for network %(network)s is already in use"), "KCHNET0007E": _("Interface should be bare NIC, bonding or bridge device."), - "KCHNET0008E": _("Unable to create network %(name)s. Details: %(err)s"), + "KCHNET0008E": _("Unable to create or update network %(name)s. Details: %(err)s"), "KCHNET0009E": _("Unable to find a free IP address for network '%(name)s'"), "KCHNET0010E": _("The interface %(iface)s already exists."), "KCHNET0011E": _("Network name must be a string without slashes (/) or quotes (\")"), @@ -267,7 +267,7 @@ messages = { "KCHNET0014E": _("Network interfaces must be an array."), "KCHNET0015E": _("Network VLAN ID must be an integer between 1 and 4094"), "KCHNET0016E": _("Specify name and type to create a Network"), - "KCHNET0017E": _("Unable to delete network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), + "KCHNET0017E": _("Unable to delete or update network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), "KCHNET0018E": _("Unable to deactivate network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), "KCHNET0019E": _("Bridge device %(name)s can not be the trunk device of a VLAN."), "KCHNET0020E": _("Failed to activate interface %(iface)s: %(err)s."), @@ -279,6 +279,9 @@ messages = { "KCHNET0028E": _("Interface should be bare NIC or bonding."), "KCHNET0029E": _("Network interfaces parameter must contain at least one interface."), "KCHNET0030E": _("Only one interface is allowed for 'bridge' and 'macvtap' networks."), + "KCHNET0031E": _("Invalid parameter 'connection'. Virtual network type cannot be changed."), + "KCHNET0032E": _("Subnet is not a valid parameter for this type of virtual network."), + "KCHNET0033E": _("VLAN ID and interfaces are not valid parameters for this type of virtual network."), "KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"), diff --git a/model/networks.py b/model/networks.py index 2664af3..fdf102a 100644 --- a/model/networks.py +++ b/model/networks.py @@ -17,6 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import copy import ipaddr import libvirt import sys @@ -33,6 +34,8 @@ from wok.plugins.kimchi import netinfo from wok.plugins.kimchi import network as knetwork from wok.plugins.kimchi.config import kimchiPaths from wok.plugins.kimchi.model.config import CapabilitiesModel +from wok.plugins.kimchi.netinfo import get_vlan_device, is_bridge, is_vlan +from wok.plugins.kimchi.netinfo import ports from wok.plugins.kimchi.osinfo import defaults as tmpl_defaults from wok.plugins.kimchi.xmlutils.interface import get_iface_xml from wok.plugins.kimchi.xmlutils.network import create_linux_bridge_xml @@ -109,7 +112,7 @@ class NetworksModel(object): try: network = conn.networkDefineXML(xml.encode("utf-8")) - network.setAutostart(True) + network.setAutostart(params.get('autostart', True)) except libvirt.libvirtError as e: raise OperationFailed("KCHNET0008E", {'name': name, 'err': e.get_error_message()}) @@ -339,6 +342,7 @@ class NetworkModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] self.objstore = kargs['objstore'] + self.collection = NetworksModel(**kargs) def lookup(self, name): network = self.get_network(self.conn.get(), name) @@ -499,3 +503,47 @@ class NetworkModel(object): iface = conn.interfaceLookupByName(bridge) iface.isActive() and iface.destroy(0) iface.undefine() + + def update(self, name, params): + info = self.lookup(name) + original = copy.deepcopy(info) + original['name'] = name + + # validate parameters + if 'connection' in params: + raise InvalidParameter("KCHNET0031E") + + connection = info['connection'] + if connection in ['bridge', 'macvtap', 'vepa']: + if params.get('subnet'): + raise InvalidParameter("KCHNET0032E") + elif connection in ['nat', 'isolated']: + if params.get('vlan_id') or params.get('interfaces'): + raise InvalidParameter("KCHNET0033E") + + # get target device if bridge was created by Kimchi + if connection == 'bridge': + iface = info['interfaces'][0] + if is_bridge(iface) and iface.startswith(KIMCHI_BRIDGE_PREFIX): + port = ports(iface)[0] + if is_vlan(port): + info['interfaces'] = [get_vlan_device(port)] + # nic + else: + info['interfaces'] = [port] + + # merge parameters + info.update(params) + + # delete original network + self.delete(name) + + try: + # create new network + network = self.collection.create(info) + except: + # restore original network + self.collection.create(original) + raise + + return network -- 1.9.1
On 04/12/2016 11:47 AM, Lucio Correia wrote:
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- API.json | 31 +++++++++++++++++++++++++++++++ control/networks.py | 1 + docs/API.md | 8 ++++++++ i18n.py | 9 ++++++--- model/networks.py | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/API.json b/API.json index 6f0b484..2205be6 100644 --- a/API.json +++ b/API.json @@ -395,6 +395,37 @@ } } }, + "network_update": { + "type": "object", + "error": "KCHAPI0001E", + "properties": { + "name": { + "description": "The new name of the network", + "type": "string", + "minLength": 1, + "pattern": "^[^/\"]*$",
+ "required": true,
I should be able to update all the other fields without changing the name. So there is no need to set is as required.
+ "error": "KCHNET0011E" + }, + "subnet": { + "description": "Network segment in slash-separated format with ip address and prefix or netmask", + "type": "string", + "error": "KCHNET0013E" + },
+ "interfaces": { + "description": "An array of network interfaces of the host", + "type": "array", + "error": "KCHNET0014E" + },
Set the minLength as 1 - so we make sure to have at least one interface.
+ "vlan_id": { + "description": "Network's VLAN ID", + "type": "integer", + "maximum": 4094, + "minimum": 1, + "error": "KCHNET0015E" + } + } + }, "vmifaces_create": { "type": "object", "error": "KCHVMIF0007E", diff --git a/control/networks.py b/control/networks.py index 8ba2206..c87b5a6 100644 --- a/control/networks.py +++ b/control/networks.py @@ -27,6 +27,7 @@ NETWORKS_REQUESTS = {
NETWORK_REQUESTS = { 'DELETE': {'default': "Remove virtual network '%(ident)s'"}, + 'PUT': {'default': "Update virtual network '%(ident)s'"}, 'POST': { 'activate': "Activate virtual network '%(ident)s'", 'deactivate': "Deactivate virtual network '%(ident)s'", diff --git a/docs/API.md b/docs/API.md index f1a13b4..491e887 100644 --- a/docs/API.md +++ b/docs/API.md @@ -736,6 +736,14 @@ A interface represents available interface on host. * **DELETE**: Remove the Network * **POST**: *See Network Actions*
+* **PUT**: Update the Network. The network type cannot be changed. + * name *(optional)*: The new name of the Network + * subnet *(optional)*: Network segment in slash-separated format with ip address and prefix. + Applies only to NAT and isolated networks + * interfaces *(optional)*: An array of network interfaces that belongs to this network. + Applies only to bridge, macvtap and VEPA networks.
'For bridge and macvtap, only one interface is allowed. For VEPA networks, you can specify multiple interfaces'
+ * vlan_id *(optional)*: VLAN tagging ID for the bridge network. + **Actions (POST):**
* activate: Activate an inactive Network diff --git a/i18n.py b/i18n.py index 6214687..b413dcd 100644 --- a/i18n.py +++ b/i18n.py @@ -255,10 +255,10 @@ messages = { "KCHNET0002E": _("Network %(name)s does not exist"), "KCHNET0003E": _("Subnet %(subnet)s specified for network %(network)s is not valid."), "KCHNET0004E": _("Specify a network interface to create bridged or macvtap networks."),
- "KCHNET0005E": _("Unable to delete active network %(name)s"), + "KCHNET0005E": _("Unable to delete or update active network %(name)s"), "KCHNET0006E": _("Interface %(iface)s specified for network %(network)s is already in use"), "KCHNET0007E": _("Interface should be bare NIC, bonding or bridge device."), - "KCHNET0008E": _("Unable to create network %(name)s. Details: %(err)s"), + "KCHNET0008E": _("Unable to create or update network %(name)s. Details: %(err)s"),
It'd be better to have separated error message for update instead of trying to reuse an existing one.
"KCHNET0009E": _("Unable to find a free IP address for network '%(name)s'"), "KCHNET0010E": _("The interface %(iface)s already exists."), "KCHNET0011E": _("Network name must be a string without slashes (/) or quotes (\")"), @@ -267,7 +267,7 @@ messages = { "KCHNET0014E": _("Network interfaces must be an array."), "KCHNET0015E": _("Network VLAN ID must be an integer between 1 and 4094"), "KCHNET0016E": _("Specify name and type to create a Network"),
- "KCHNET0017E": _("Unable to delete network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), + "KCHNET0017E": _("Unable to delete or update network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."),
Is there a problem to edit a network in use if the user does not change the name? Maybe, we only need to block the name in this case (and proper add an information about it on UI)
"KCHNET0018E": _("Unable to deactivate network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), "KCHNET0019E": _("Bridge device %(name)s can not be the trunk device of a VLAN."), "KCHNET0020E": _("Failed to activate interface %(iface)s: %(err)s."), @@ -279,6 +279,9 @@ messages = { "KCHNET0028E": _("Interface should be bare NIC or bonding."), "KCHNET0029E": _("Network interfaces parameter must contain at least one interface."), "KCHNET0030E": _("Only one interface is allowed for 'bridge' and 'macvtap' networks."),
+ "KCHNET0031E": _("Invalid parameter 'connection'. Virtual network type cannot be changed."),
To avoid the need to manually check it, you can add "additionalProperties: false' to API.json That way, only the parameters specified there will be allowed. For example: "additionalProperties": false, "error": "<error code>" The error message will be something like: "To update a virtual network only the following parameters are allowed: name, subnet, .."
+ "KCHNET0032E": _("Subnet is not a valid parameter for this type of virtual network."), + "KCHNET0033E": _("VLAN ID and interfaces are not valid parameters for this type of virtual network."),
"KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"),
diff --git a/model/networks.py b/model/networks.py index 2664af3..fdf102a 100644 --- a/model/networks.py +++ b/model/networks.py @@ -17,6 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import copy import ipaddr import libvirt import sys @@ -33,6 +34,8 @@ from wok.plugins.kimchi import netinfo from wok.plugins.kimchi import network as knetwork from wok.plugins.kimchi.config import kimchiPaths from wok.plugins.kimchi.model.config import CapabilitiesModel +from wok.plugins.kimchi.netinfo import get_vlan_device, is_bridge, is_vlan +from wok.plugins.kimchi.netinfo import ports from wok.plugins.kimchi.osinfo import defaults as tmpl_defaults from wok.plugins.kimchi.xmlutils.interface import get_iface_xml from wok.plugins.kimchi.xmlutils.network import create_linux_bridge_xml @@ -109,7 +112,7 @@ class NetworksModel(object):
try: network = conn.networkDefineXML(xml.encode("utf-8")) - network.setAutostart(True) + network.setAutostart(params.get('autostart', True)) except libvirt.libvirtError as e: raise OperationFailed("KCHNET0008E", {'name': name, 'err': e.get_error_message()}) @@ -339,6 +342,7 @@ class NetworkModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] self.objstore = kargs['objstore'] + self.collection = NetworksModel(**kargs)
def lookup(self, name): network = self.get_network(self.conn.get(), name) @@ -499,3 +503,47 @@ class NetworkModel(object): iface = conn.interfaceLookupByName(bridge) iface.isActive() and iface.destroy(0) iface.undefine() + + def update(self, name, params): + info = self.lookup(name) + original = copy.deepcopy(info) + original['name'] = name +
+ # validate parameters + if 'connection' in params: + raise InvalidParameter("KCHNET0031E") +
If you change API.json as I commented above, this verification is not needed.
+ connection = info['connection'] + if connection in ['bridge', 'macvtap', 'vepa']: + if params.get('subnet'): + raise InvalidParameter("KCHNET0032E") + elif connection in ['nat', 'isolated']: + if params.get('vlan_id') or params.get('interfaces'): + raise InvalidParameter("KCHNET0033E") + + # get target device if bridge was created by Kimchi + if connection == 'bridge':
+ iface = info['interfaces'][0]
Verify only ONE interface is passed for bridge and macvtap and raise an error otherwise. Multiples interfaces are only allowed for VEPA networks.
+ if is_bridge(iface) and iface.startswith(KIMCHI_BRIDGE_PREFIX): + port = ports(iface)[0] + if is_vlan(port): + info['interfaces'] = [get_vlan_device(port)] + # nic + else: + info['interfaces'] = [port] + + # merge parameters + info.update(params) + + # delete original network + self.delete(name) + + try: + # create new network + network = self.collection.create(info) + except: + # restore original network + self.collection.create(original) + raise + + return network
On 13-04-2016 14:22, Aline Manera wrote:
On 04/12/2016 11:47 AM, Lucio Correia wrote:
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- API.json | 31 +++++++++++++++++++++++++++++++ control/networks.py | 1 + docs/API.md | 8 ++++++++ i18n.py | 9 ++++++--- model/networks.py | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/API.json b/API.json index 6f0b484..2205be6 100644 --- a/API.json +++ b/API.json @@ -395,6 +395,37 @@ } } }, + "network_update": { + "type": "object", + "error": "KCHAPI0001E", + "properties": { + "name": { + "description": "The new name of the network", + "type": "string", + "minLength": 1, + "pattern": "^[^/\"]*$",
+ "required": true,
I should be able to update all the other fields without changing the name. So there is no need to set is as required.
OK
+ "error": "KCHNET0011E" + }, + "subnet": { + "description": "Network segment in slash-separated format with ip address and prefix or netmask", + "type": "string", + "error": "KCHNET0013E" + },
+ "interfaces": { + "description": "An array of network interfaces of the host", + "type": "array", + "error": "KCHNET0014E" + },
Set the minLength as 1 - so we make sure to have at least one interface.
OK
+ "vlan_id": { + "description": "Network's VLAN ID", + "type": "integer", + "maximum": 4094, + "minimum": 1, + "error": "KCHNET0015E" + } + } + }, "vmifaces_create": { "type": "object", "error": "KCHVMIF0007E", diff --git a/control/networks.py b/control/networks.py index 8ba2206..c87b5a6 100644 --- a/control/networks.py +++ b/control/networks.py @@ -27,6 +27,7 @@ NETWORKS_REQUESTS = {
NETWORK_REQUESTS = { 'DELETE': {'default': "Remove virtual network '%(ident)s'"}, + 'PUT': {'default': "Update virtual network '%(ident)s'"}, 'POST': { 'activate': "Activate virtual network '%(ident)s'", 'deactivate': "Deactivate virtual network '%(ident)s'", diff --git a/docs/API.md b/docs/API.md index f1a13b4..491e887 100644 --- a/docs/API.md +++ b/docs/API.md @@ -736,6 +736,14 @@ A interface represents available interface on host. * **DELETE**: Remove the Network * **POST**: *See Network Actions*
+* **PUT**: Update the Network. The network type cannot be changed. + * name *(optional)*: The new name of the Network + * subnet *(optional)*: Network segment in slash-separated format with ip address and prefix. + Applies only to NAT and isolated networks + * interfaces *(optional)*: An array of network interfaces that belongs to this network. + Applies only to bridge, macvtap and VEPA networks.
'For bridge and macvtap, only one interface is allowed. For VEPA networks, you can specify multiple interfaces'
OK
+ * vlan_id *(optional)*: VLAN tagging ID for the bridge network. + **Actions (POST):**
* activate: Activate an inactive Network diff --git a/i18n.py b/i18n.py index 6214687..b413dcd 100644 --- a/i18n.py +++ b/i18n.py @@ -255,10 +255,10 @@ messages = { "KCHNET0002E": _("Network %(name)s does not exist"), "KCHNET0003E": _("Subnet %(subnet)s specified for network %(network)s is not valid."), "KCHNET0004E": _("Specify a network interface to create bridged or macvtap networks."),
- "KCHNET0005E": _("Unable to delete active network %(name)s"), + "KCHNET0005E": _("Unable to delete or update active network %(name)s"), "KCHNET0006E": _("Interface %(iface)s specified for network %(network)s is already in use"), "KCHNET0007E": _("Interface should be bare NIC, bonding or bridge device."), - "KCHNET0008E": _("Unable to create network %(name)s. Details: %(err)s"), + "KCHNET0008E": _("Unable to create or update network %(name)s. Details: %(err)s"),
It'd be better to have separated error message for update instead of trying to reuse an existing one.
The same line of code generates each message, since the update is implemented as delete + create.
"KCHNET0009E": _("Unable to find a free IP address for network '%(name)s'"), "KCHNET0010E": _("The interface %(iface)s already exists."), "KCHNET0011E": _("Network name must be a string without slashes (/) or quotes (\")"), @@ -267,7 +267,7 @@ messages = { "KCHNET0014E": _("Network interfaces must be an array."), "KCHNET0015E": _("Network VLAN ID must be an integer between 1 and 4094"), "KCHNET0016E": _("Specify name and type to create a Network"),
- "KCHNET0017E": _("Unable to delete network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), + "KCHNET0017E": _("Unable to delete or update network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."),
Is there a problem to edit a network in use if the user does not change the name? Maybe, we only need to block the name in this case (and proper add an information about it on UI)
That comes from delete method, since update is implemented as delete + create.
"KCHNET0018E": _("Unable to deactivate network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), "KCHNET0019E": _("Bridge device %(name)s can not be the trunk device of a VLAN."), "KCHNET0020E": _("Failed to activate interface %(iface)s: %(err)s."), @@ -279,6 +279,9 @@ messages = { "KCHNET0028E": _("Interface should be bare NIC or bonding."), "KCHNET0029E": _("Network interfaces parameter must contain at least one interface."), "KCHNET0030E": _("Only one interface is allowed for 'bridge' and 'macvtap' networks."),
+ "KCHNET0031E": _("Invalid parameter 'connection'. Virtual network type cannot be changed."),
To avoid the need to manually check it, you can add "additionalProperties: false' to API.json That way, only the parameters specified there will be allowed.
For example:
"additionalProperties": false, "error": "<error code>"
The error message will be something like: "To update a virtual network only the following parameters are allowed: name, subnet, .."
Great, I didn't know that field. Thanks!
+ "KCHNET0032E": _("Subnet is not a valid parameter for this type of virtual network."), + "KCHNET0033E": _("VLAN ID and interfaces are not valid parameters for this type of virtual network."),
"KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"),
diff --git a/model/networks.py b/model/networks.py index 2664af3..fdf102a 100644 --- a/model/networks.py +++ b/model/networks.py @@ -17,6 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import copy import ipaddr import libvirt import sys @@ -33,6 +34,8 @@ from wok.plugins.kimchi import netinfo from wok.plugins.kimchi import network as knetwork from wok.plugins.kimchi.config import kimchiPaths from wok.plugins.kimchi.model.config import CapabilitiesModel +from wok.plugins.kimchi.netinfo import get_vlan_device, is_bridge, is_vlan +from wok.plugins.kimchi.netinfo import ports from wok.plugins.kimchi.osinfo import defaults as tmpl_defaults from wok.plugins.kimchi.xmlutils.interface import get_iface_xml from wok.plugins.kimchi.xmlutils.network import create_linux_bridge_xml @@ -109,7 +112,7 @@ class NetworksModel(object):
try: network = conn.networkDefineXML(xml.encode("utf-8")) - network.setAutostart(True) + network.setAutostart(params.get('autostart', True)) except libvirt.libvirtError as e: raise OperationFailed("KCHNET0008E", {'name': name, 'err': e.get_error_message()}) @@ -339,6 +342,7 @@ class NetworkModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] self.objstore = kargs['objstore'] + self.collection = NetworksModel(**kargs)
def lookup(self, name): network = self.get_network(self.conn.get(), name) @@ -499,3 +503,47 @@ class NetworkModel(object): iface = conn.interfaceLookupByName(bridge) iface.isActive() and iface.destroy(0) iface.undefine() + + def update(self, name, params): + info = self.lookup(name) + original = copy.deepcopy(info) + original['name'] = name +
+ # validate parameters + if 'connection' in params: + raise InvalidParameter("KCHNET0031E") +
If you change API.json as I commented above, this verification is not needed.
OK
+ connection = info['connection'] + if connection in ['bridge', 'macvtap', 'vepa']: + if params.get('subnet'): + raise InvalidParameter("KCHNET0032E") + elif connection in ['nat', 'isolated']: + if params.get('vlan_id') or params.get('interfaces'): + raise InvalidParameter("KCHNET0033E") + + # get target device if bridge was created by Kimchi + if connection == 'bridge':
+ iface = info['interfaces'][0]
Verify only ONE interface is passed for bridge and macvtap and raise an error otherwise. Multiples interfaces are only allowed for VEPA networks.
This is just discovering the interface that is the port of the bridge used by the already existent network that will be updated.
+ if is_bridge(iface) and iface.startswith(KIMCHI_BRIDGE_PREFIX): + port = ports(iface)[0] + if is_vlan(port): + info['interfaces'] = [get_vlan_device(port)] + # nic + else: + info['interfaces'] = [port] + + # merge parameters + info.update(params) + + # delete original network + self.delete(name) + + try: + # create new network + network = self.collection.create(info) + except: + # restore original network + self.collection.create(original) + raise + + return network
-- Lucio Correia Software Engineer IBM LTC Brazil
On 04/13/2016 04:06 PM, Lucio Correia wrote:
On 13-04-2016 14:22, Aline Manera wrote:
On 04/12/2016 11:47 AM, Lucio Correia wrote:
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- API.json | 31 +++++++++++++++++++++++++++++++ control/networks.py | 1 + docs/API.md | 8 ++++++++ i18n.py | 9 ++++++--- model/networks.py | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/API.json b/API.json index 6f0b484..2205be6 100644 --- a/API.json +++ b/API.json @@ -395,6 +395,37 @@ } } }, + "network_update": { + "type": "object", + "error": "KCHAPI0001E", + "properties": { + "name": { + "description": "The new name of the network", + "type": "string", + "minLength": 1, + "pattern": "^[^/\"]*$",
+ "required": true,
I should be able to update all the other fields without changing the name. So there is no need to set is as required.
OK
+ "error": "KCHNET0011E" + }, + "subnet": { + "description": "Network segment in slash-separated format with ip address and prefix or netmask", + "type": "string", + "error": "KCHNET0013E" + },
+ "interfaces": { + "description": "An array of network interfaces of the host", + "type": "array", + "error": "KCHNET0014E" + },
Set the minLength as 1 - so we make sure to have at least one interface.
OK
+ "vlan_id": { + "description": "Network's VLAN ID", + "type": "integer", + "maximum": 4094, + "minimum": 1, + "error": "KCHNET0015E" + } + } + }, "vmifaces_create": { "type": "object", "error": "KCHVMIF0007E", diff --git a/control/networks.py b/control/networks.py index 8ba2206..c87b5a6 100644 --- a/control/networks.py +++ b/control/networks.py @@ -27,6 +27,7 @@ NETWORKS_REQUESTS = {
NETWORK_REQUESTS = { 'DELETE': {'default': "Remove virtual network '%(ident)s'"}, + 'PUT': {'default': "Update virtual network '%(ident)s'"}, 'POST': { 'activate': "Activate virtual network '%(ident)s'", 'deactivate': "Deactivate virtual network '%(ident)s'", diff --git a/docs/API.md b/docs/API.md index f1a13b4..491e887 100644 --- a/docs/API.md +++ b/docs/API.md @@ -736,6 +736,14 @@ A interface represents available interface on host. * **DELETE**: Remove the Network * **POST**: *See Network Actions*
+* **PUT**: Update the Network. The network type cannot be changed. + * name *(optional)*: The new name of the Network + * subnet *(optional)*: Network segment in slash-separated format with ip address and prefix. + Applies only to NAT and isolated networks + * interfaces *(optional)*: An array of network interfaces that belongs to this network. + Applies only to bridge, macvtap and VEPA networks.
'For bridge and macvtap, only one interface is allowed. For VEPA networks, you can specify multiple interfaces'
OK
+ * vlan_id *(optional)*: VLAN tagging ID for the bridge network. + **Actions (POST):**
* activate: Activate an inactive Network diff --git a/i18n.py b/i18n.py index 6214687..b413dcd 100644 --- a/i18n.py +++ b/i18n.py @@ -255,10 +255,10 @@ messages = { "KCHNET0002E": _("Network %(name)s does not exist"), "KCHNET0003E": _("Subnet %(subnet)s specified for network %(network)s is not valid."), "KCHNET0004E": _("Specify a network interface to create bridged or macvtap networks."),
- "KCHNET0005E": _("Unable to delete active network %(name)s"), + "KCHNET0005E": _("Unable to delete or update active network %(name)s"), "KCHNET0006E": _("Interface %(iface)s specified for network %(network)s is already in use"), "KCHNET0007E": _("Interface should be bare NIC, bonding or bridge device."), - "KCHNET0008E": _("Unable to create network %(name)s. Details: %(err)s"), + "KCHNET0008E": _("Unable to create or update network %(name)s. Details: %(err)s"),
It'd be better to have separated error message for update instead of trying to reuse an existing one.
The same line of code generates each message, since the update is implemented as delete + create.
OK
"KCHNET0009E": _("Unable to find a free IP address for network '%(name)s'"), "KCHNET0010E": _("The interface %(iface)s already exists."), "KCHNET0011E": _("Network name must be a string without slashes (/) or quotes (\")"), @@ -267,7 +267,7 @@ messages = { "KCHNET0014E": _("Network interfaces must be an array."), "KCHNET0015E": _("Network VLAN ID must be an integer between 1 and 4094"), "KCHNET0016E": _("Specify name and type to create a Network"),
- "KCHNET0017E": _("Unable to delete network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), + "KCHNET0017E": _("Unable to delete or update network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."),
Is there a problem to edit a network in use if the user does not change the name? Maybe, we only need to block the name in this case (and proper add an information about it on UI)
That comes from delete method, since update is implemented as delete + create.
OK
"KCHNET0018E": _("Unable to deactivate network %(name)s. There are some virtual machines %(vms)s and/or templates linked to this network."), "KCHNET0019E": _("Bridge device %(name)s can not be the trunk device of a VLAN."), "KCHNET0020E": _("Failed to activate interface %(iface)s: %(err)s."), @@ -279,6 +279,9 @@ messages = { "KCHNET0028E": _("Interface should be bare NIC or bonding."), "KCHNET0029E": _("Network interfaces parameter must contain at least one interface."), "KCHNET0030E": _("Only one interface is allowed for 'bridge' and 'macvtap' networks."),
+ "KCHNET0031E": _("Invalid parameter 'connection'. Virtual network type cannot be changed."),
To avoid the need to manually check it, you can add "additionalProperties: false' to API.json That way, only the parameters specified there will be allowed.
For example:
"additionalProperties": false, "error": "<error code>"
The error message will be something like: "To update a virtual network only the following parameters are allowed: name, subnet, .."
Great, I didn't know that field. Thanks!
+ "KCHNET0032E": _("Subnet is not a valid parameter for this type of virtual network."), + "KCHNET0033E": _("VLAN ID and interfaces are not valid parameters for this type of virtual network."),
"KCHSR0001E": _("Storage server %(server)s was not used by Kimchi"),
diff --git a/model/networks.py b/model/networks.py index 2664af3..fdf102a 100644 --- a/model/networks.py +++ b/model/networks.py @@ -17,6 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import copy import ipaddr import libvirt import sys @@ -33,6 +34,8 @@ from wok.plugins.kimchi import netinfo from wok.plugins.kimchi import network as knetwork from wok.plugins.kimchi.config import kimchiPaths from wok.plugins.kimchi.model.config import CapabilitiesModel +from wok.plugins.kimchi.netinfo import get_vlan_device, is_bridge, is_vlan +from wok.plugins.kimchi.netinfo import ports from wok.plugins.kimchi.osinfo import defaults as tmpl_defaults from wok.plugins.kimchi.xmlutils.interface import get_iface_xml from wok.plugins.kimchi.xmlutils.network import create_linux_bridge_xml @@ -109,7 +112,7 @@ class NetworksModel(object):
try: network = conn.networkDefineXML(xml.encode("utf-8")) - network.setAutostart(True) + network.setAutostart(params.get('autostart', True)) except libvirt.libvirtError as e: raise OperationFailed("KCHNET0008E", {'name': name, 'err': e.get_error_message()}) @@ -339,6 +342,7 @@ class NetworkModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] self.objstore = kargs['objstore'] + self.collection = NetworksModel(**kargs)
def lookup(self, name): network = self.get_network(self.conn.get(), name) @@ -499,3 +503,47 @@ class NetworkModel(object): iface = conn.interfaceLookupByName(bridge) iface.isActive() and iface.destroy(0) iface.undefine() + + def update(self, name, params): + info = self.lookup(name) + original = copy.deepcopy(info) + original['name'] = name +
+ # validate parameters + if 'connection' in params: + raise InvalidParameter("KCHNET0031E") +
If you change API.json as I commented above, this verification is not needed.
OK
+ connection = info['connection'] + if connection in ['bridge', 'macvtap', 'vepa']: + if params.get('subnet'): + raise InvalidParameter("KCHNET0032E") + elif connection in ['nat', 'isolated']: + if params.get('vlan_id') or params.get('interfaces'): + raise InvalidParameter("KCHNET0033E") + + # get target device if bridge was created by Kimchi + if connection == 'bridge':
+ iface = info['interfaces'][0]
Verify only ONE interface is passed for bridge and macvtap and raise an error otherwise. Multiples interfaces are only allowed for VEPA networks.
This is just discovering the interface that is the port of the bridge used by the already existent network that will be updated.
OK
+ if is_bridge(iface) and iface.startswith(KIMCHI_BRIDGE_PREFIX): + port = ports(iface)[0] + if is_vlan(port): + info['interfaces'] = [get_vlan_device(port)] + # nic + else: + info['interfaces'] = [port] + + # merge parameters + info.update(params) + + # delete original network + self.delete(name) + + try: + # create new network + network = self.collection.create(info) + except: + # restore original network + self.collection.create(original) + raise + + return network
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- tests/test_model_network.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_model_network.py b/tests/test_model_network.py index e27036d..ccb2c4d 100644 --- a/tests/test_model_network.py +++ b/tests/test_model_network.py @@ -92,8 +92,19 @@ def _do_network_test(self, model, params): network = json.loads(resp.read()) self.assertEquals('inactive', network['state']) + # Edit (rename) the network + params['name'] += u'renamed' + params.pop('connection') + req = json.dumps(params) + resp = self.request(uri, req, 'PUT') + self.assertEquals(303, resp.status) + + # Assert old name does not exist anymore + resp = self.request(uri, '{}', 'GET') + self.assertEquals(404, resp.status) + # Delete the network - resp = self.request(uri, '{}', 'DELETE') + resp = self.request(uri + 'renamed'.encode('utf-8'), '{}', 'DELETE') self.assertEquals(204, resp.status) -- 1.9.1
On 04/12/2016 11:47 AM, Lucio Correia wrote:
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- tests/test_model_network.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/tests/test_model_network.py b/tests/test_model_network.py index e27036d..ccb2c4d 100644 --- a/tests/test_model_network.py +++ b/tests/test_model_network.py @@ -92,8 +92,19 @@ def _do_network_test(self, model, params): network = json.loads(resp.read()) self.assertEquals('inactive', network['state'])
+ # Edit (rename) the network + params['name'] += u'renamed' + params.pop('connection')
What is in params already? It is better (at least to review) to create a new group of parameters so we can easily identify which parameters you are trying to update.
+ req = json.dumps(params) + resp = self.request(uri, req, 'PUT') + self.assertEquals(303, resp.status) + + # Assert old name does not exist anymore + resp = self.request(uri, '{}', 'GET') + self.assertEquals(404, resp.status) + # Delete the network - resp = self.request(uri, '{}', 'DELETE') + resp = self.request(uri + 'renamed'.encode('utf-8'), '{}', 'DELETE') self.assertEquals(204, resp.status)
Please, also add tests to edit the other parameters: subnet, iface, dhcp, etc.
On 13-04-2016 14:30, Aline Manera wrote:
On 04/12/2016 11:47 AM, Lucio Correia wrote:
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- tests/test_model_network.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/tests/test_model_network.py b/tests/test_model_network.py index e27036d..ccb2c4d 100644 --- a/tests/test_model_network.py +++ b/tests/test_model_network.py @@ -92,8 +92,19 @@ def _do_network_test(self, model, params): network = json.loads(resp.read()) self.assertEquals('inactive', network['state'])
+ # Edit (rename) the network + params['name'] += u'renamed' + params.pop('connection')
What is in params already? It is better (at least to review) to create a new group of parameters so we can easily identify which parameters you are trying to update.
This is only a rename test added for the already existing network types in tests. Since there are various network types handled by this code (a loop), we would need to specify the parameters according to the type.
+ req = json.dumps(params) + resp = self.request(uri, req, 'PUT') + self.assertEquals(303, resp.status) + + # Assert old name does not exist anymore + resp = self.request(uri, '{}', 'GET') + self.assertEquals(404, resp.status) + # Delete the network - resp = self.request(uri, '{}', 'DELETE') + resp = self.request(uri + 'renamed'.encode('utf-8'), '{}', 'DELETE') self.assertEquals(204, resp.status)
Please, also add tests to edit the other parameters: subnet, iface, dhcp, etc.
OK, subnet, iface, vlan_id.
-- Lucio Correia Software Engineer IBM LTC Brazil
participants (2)
-
Aline Manera -
Lucio Correia