[Kimchi-devel] [PATCH] [Kimchi 1/2] Add backend support for editing virtual networks
Aline Manera
alinefm at linux.vnet.ibm.com
Wed Apr 13 19:19:24 UTC 2016
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 at 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
>>
>
>
More information about the Kimchi-devel
mailing list