[Kimchi-devel] [PATCH V1] [Kimchi] Only on s390x add default networks to template if template.conf has default network configuration uncommented.
Daniel Henrique Barboza
dhbarboza82 at gmail.com
Thu Aug 18 14:55:03 UTC 2016
This patch broke the following unit tests:
======================================================================
FAIL: test_storagevolume_action
(test_model_storagevolume.StorageVolumeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "test_model_storagevolume.py", line 274, in
test_storagevolume_action
_do_volume_test(self, model, host, ssl_port, 'default')
File "test_model_storagevolume.py", line 244, in _do_volume_test
self.assertEquals(202, resp.status)
AssertionError: 202 != 400
----------------------------------------------------------------------
Ran 2 tests in 63.746s
FAILED (failures=1)
More comments inline:
On 08/17/2016 09:38 AM, archus at linux.vnet.ibm.com wrote:
> From: Archana Singh <archus at linux.vnet.ibm.com>
>
> Below are the changes done for s390x architecture to only add default networks
> to template if template.conf has network configuration uncommented
> otherwise do not add any network by default.
>
> 1) Only on s390x, osinfo._get_tmpl_defaults() does not add
> default network in case template.conf has commented out network configuration.
> 2) Only on s390x, NetworksModel._check_default_networks() does
> not throw key error if networks not found in tmpl_defaults.
> 3) Only on s390x, VMTemplate.validate_integrity() does not throw
> key error if networks not found in the template.
> 4) Only on s390x, LibvirtVMTemplate._network_validate() does not
> throw key error on s390x if networks not found in tmpl_default, as on s390x networks is optional.
> 5) Only on s390x, VMTemplate._get_network_xml() does
> return empty string and does not throw key error if networks not found in template object.
> 6) Only on s390x, control/templates.py gets networks as [] if not found
> self.info.
>
> Signed-off-by: Archana Singh <archus at linux.vnet.ibm.com>
> ---
> control/templates.py | 2 +-
> model/networks.py | 7 ++++++-
> model/templates.py | 5 ++++-
> osinfo.py | 14 +++++++++++++-
> vmtemplate.py | 15 +++++++++++++--
> 5 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/control/templates.py b/control/templates.py
> index bb2e068..2dd8601 100644
> --- a/control/templates.py
> +++ b/control/templates.py
> @@ -67,7 +67,7 @@ class Template(Resource):
> 'memory': self.info['memory'],
> 'cdrom': self.info.get('cdrom', None),
> 'disks': self.info['disks'],
> - 'networks': self.info['networks'],
> + 'networks': self.info.get('networks', []),
> 'folder': self.info.get('folder', []),
> 'graphics': self.info['graphics'],
> 'cpu_info': self.info.get('cpu_info')
> diff --git a/model/networks.py b/model/networks.py
> index 35431d4..c9db979 100644
> --- a/model/networks.py
> +++ b/model/networks.py
> @@ -21,6 +21,7 @@ import copy
> import ipaddr
> import libvirt
> import time
> +import os
wrong import order
> from libvirt import VIR_INTERFACE_XML_INACTIVE
>
> from wok.exception import InvalidOperation, InvalidParameter
> @@ -55,7 +56,11 @@ class NetworksModel(object):
> self.caps = CapabilitiesModel(**kargs)
>
> def _check_default_networks(self):
> - networks = list(set(tmpl_defaults['networks']))
> + if os.uname()[4] in ['s390x']:
This call
os.uname()[4] in ['s390x']
Is used 4 times in this patch alone. Another similar verification is
made in osinfo.py
I believe this warrants a function is_running_in_s390x() in utils.py to
avoid
code repetition.
> + networks = list(set(tmpl_defaults.get('networks', [])))
> + else:
> + networks = list(set(tmpl_defaults['networks']))
> +
> conn = self.conn.get()
>
> for net_name in networks:
> diff --git a/model/templates.py b/model/templates.py
> index a299c85..877bea1 100644
> --- a/model/templates.py
> +++ b/model/templates.py
> @@ -358,7 +358,10 @@ class LibvirtVMTemplate(VMTemplate):
> return sorted(map(lambda x: x.decode('utf-8'), names))
>
> def _network_validate(self):
> - names = self.info['networks']
> + if os.uname()[4] in ['s390x']:
> + names = self.info.get('networks', [])
> + else:
> + names = self.info['networks']
> for name in names:
> try:
> conn = self.conn.get()
> diff --git a/osinfo.py b/osinfo.py
> index 2c59312..af37c72 100644
> --- a/osinfo.py
> +++ b/osinfo.py
> @@ -138,10 +138,22 @@ def _get_tmpl_defaults():
> 'pool': '/plugins/kimchi/storagepools/default'}},
> 'processor': {'vcpus': '1', 'maxvcpus': 1},
> 'graphics': {'type': 'spice', 'listen': '127.0.0.1'}}
> +
> + The default values on s390x architecture:
> +
> + {'memory': {'current': 1024, 'maxmemory': 1024},
> + 'storage': { 'disk.0': {'format': 'qcow2', 'size': '10',
> + 'pool': '/plugins/kimchi/storagepools/default'}},
> + 'processor': {'vcpus': '1', 'maxvcpus': 1},
> + 'graphics': {'type': 'spice', 'listen': '127.0.0.1'}}
> """
> # Create dict with default values
> tmpl_defaults = defaultdict(dict)
> - tmpl_defaults['main']['networks'] = ['default']
> +
> + host_arch = _get_arch()
> + if host_arch not in ['s390x']:
> + tmpl_defaults['main']['networks'] = ['default']
> +
> tmpl_defaults['memory'] = {'current': _get_default_template_mem(),
> 'maxmemory': _get_default_template_mem()}
> tmpl_defaults['storage']['disk.0'] = {'size': 10, 'format': 'qcow2',
> diff --git a/vmtemplate.py b/vmtemplate.py
> index dc81fe2..5a45123 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -288,7 +288,13 @@ class VMTemplate(object):
> networks = ""
> params = {'type': 'network',
> 'model': self.info['nic_model']}
> - for nw in self.info['networks']:
> +
> + if os.uname()[4] in ['s390x']:
> + info_networks = self.info.get('networks', [])
> + else:
> + info_networks = self.info['networks']
> +
> + for nw in info_networks:
> params['network'] = nw
> networks += get_iface_xml(params, self.info['arch'],
> self.info['os_distro'],
> @@ -484,7 +490,12 @@ class VMTemplate(object):
> def validate_integrity(self):
> invalid = {}
> # validate networks integrity
> - invalid_networks = list(set(self.info['networks']) -
> + if os.uname()[4] in ['s390x']:
> + networks = self.info.get('networks', [])
> + else:
> + networks = self.info['networks']
> +
> + invalid_networks = list(set(networks) -
> set(self._get_all_networks_name()))
> if invalid_networks:
> invalid['networks'] = invalid_networks
More information about the Kimchi-devel
mailing list