[Kimchi-devel] [PATCH] [Kimchi] Fix for Issue #1000 : Make Check fails on s390x environment
Pooja Kulkarni
pkulkark at linux.vnet.ibm.com
Tue Sep 13 08:28:52 UTC 2016
On 09/13/2016 01:32 PM, Archana Singh wrote:
> Looks good, some minor comment.
>
>
> On 09/13/2016 12:31 PM, pkulkark at linux.vnet.ibm.com wrote:
>> From: Pooja Kulkarni <pkulkark at linux.vnet.ibm.com>
>>
>> This patch fixes the issue
>> with the make check command
>> failing on s390x environment
>> by adding a check for s390x
>>
>> Signed-off-by: Pooja Kulkarni <pkulkark at linux.vnet.ibm.com>
>> ---
>> osinfo.py | 5 ++-
>> tests/test_model.py | 82
>> +++++++++++++++++++++++----------------
>> tests/test_model_libvirtevents.py | 6 ++-
>> tests/test_osinfo.py | 18 +++++----
>> tests/test_rest.py | 36 +++++++++++------
>> tests/test_template.py | 11 +++++-
>> tests/test_vmtemplate.py | 23 ++++++++---
>> 7 files changed, 116 insertions(+), 65 deletions(-)
>>
>> diff --git a/osinfo.py b/osinfo.py
>> index 3e56d97..11229aa 100644
>> --- a/osinfo.py
>> +++ b/osinfo.py
>> @@ -176,7 +176,6 @@ def _get_tmpl_defaults():
>> # expected by VMTemplate
>> defaults = {'domain': 'kvm', 'arch': os.uname()[4],
>> 'cdrom_bus': 'ide', 'cdrom_index': 2, 'mouse_bus':
>> 'ps2'}
>> -
>> # Parse main section to get networks and memory values
>> defaults.update(default_config.pop('main'))
>> defaults['memory'] = default_config.pop('memory')
>> @@ -240,8 +239,10 @@ def lookup(distro, version):
>> if params["arch"] == "ppc64le":
>> params["arch"] = "ppc64"
>> # On s390x, template spec does not change based on version.
>> - if params["arch"] == "s390x":
>> + if params["arch"] == "s390x" or arch == "s390x":
>> params.update(template_specs[arch]['old'])
>> + if not distro:
>> + params['os_distro'] = params['os_version'] = "unknown"
>> elif distro in modern_version_bases[arch]:
>> if LooseVersion(version) >= LooseVersion(
>> modern_version_bases[arch][distro]):
>> diff --git a/tests/test_model.py b/tests/test_model.py
>> index 05f046c..ff2a6cb 100644
>> --- a/tests/test_model.py
>> +++ b/tests/test_model.py
>> @@ -148,7 +148,8 @@ class ModelTests(unittest.TestCase):
>> self.assertEquals([], info['groups'])
>> self.assertTrue(info['persistent'])
>> - @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>> + @unittest.skipUnless(utils.running_as_root() and
>> + os.uname()[4] != "s390x", 'Must be run as
>> root')
>> def test_vm_lifecycle(self):
>> inst = model.Model(objstore_loc=self.tmp_store)
>> @@ -271,7 +272,8 @@ class ModelTests(unittest.TestCase):
>> vms = inst.vms_get_list()
>> self.assertFalse('kimchi-vm-new' in vms)
>> - @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>> + @unittest.skipUnless(utils.running_as_root() and
>> + os.uname()[4] != "s390x", 'Must be run as
>> root')
>> def test_image_based_template(self):
>> inst = model.Model(objstore_loc=self.tmp_store)
>> @@ -318,7 +320,8 @@ class ModelTests(unittest.TestCase):
>> info = inst.vm_lookup('kimchi-vm')
>> self.assertEquals('running', info['state'])
>> - @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>> + @unittest.skipUnless(utils.running_as_root() and
>> + os.uname()[4] != "s390x", 'Must be run as
>> root')
>> def test_vm_graphics(self):
>> inst = model.Model(objstore_loc=self.tmp_store)
>> params = {'name': 'test',
>> @@ -511,7 +514,8 @@ class ModelTests(unittest.TestCase):
>> call(['ufw', 'status']),
>> call(iptables_add), call(iptables_del)])
>> - @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>> + @unittest.skipUnless(utils.running_as_root() and
>> + os.uname()[4] != "s390x", 'Must be run as
>> root')
>> @mock.patch('wok.plugins.kimchi.model.virtviewerfile.'
>> 'FirewallManager.remove_vm_graphics_port')
>> @mock.patch('wok.plugins.kimchi.model.virtviewerfile.'
>> @@ -553,7 +557,8 @@ class ModelTests(unittest.TestCase):
>> inst.template_delete('test')
>> - @unittest.skipUnless(utils.running_as_root(), "Must be run as
>> root")
>> + @unittest.skipUnless(utils.running_as_root() and
>> + os.uname()[4] != "s390x", "Must be run as
>> root")
>> def test_vm_serial(self):
>> inst = model.Model(objstore_loc=self.tmp_store)
>> params = {'name': 'test',
>> @@ -601,12 +606,13 @@ class ModelTests(unittest.TestCase):
>> rollback.prependDefer(inst.vm_delete, vm_name)
>> ifaces = inst.vmifaces_get_list(vm_name)
>> - self.assertEquals(1, len(ifaces))
>> + if not os.uname()[4] == "s390x":
>> + self.assertEquals(1, len(ifaces))
>> - iface = inst.vmiface_lookup(vm_name, ifaces[0])
>> - self.assertEquals(17, len(iface['mac']))
>> - self.assertEquals("default", iface['network'])
>> - self.assertIn("model", iface)
>> + iface = inst.vmiface_lookup(vm_name, ifaces[0])
>> + self.assertEquals(17, len(iface['mac']))
>> + self.assertEquals("default", iface['network'])
>> + self.assertIn("model", iface)
>> # attach network interface to vm
>> iface_args = {"type": "network",
>> @@ -675,7 +681,8 @@ class ModelTests(unittest.TestCase):
>> vms = inst.vms_get_list()
>> self.assertFalse('kimchi-netboot-vm' in vms)
>> - @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>> + @unittest.skipUnless(utils.running_as_root() and
>> + os.uname()[4] != "s390x", 'Must be run as
>> root')
>> def test_vm_disk(self):
>> disk_path = os.path.join(TMP_DIR, 'existent2.iso')
>> open(disk_path, 'w').close()
>> @@ -1020,7 +1027,9 @@ class ModelTests(unittest.TestCase):
>> config.set("authentication", "method", "pam")
>> inst = model.Model(None, objstore_loc=self.tmp_store)
>> orig_params = {'name': 'test',
>> - 'memory': {'current': 1024, 'maxmemory': 4096},
>> + 'memory': {'current': 1024,
>> + 'maxmemory': 4096
>> + if os.uname()[4] != "s390x" else
>> 2048},
>> 'source_media': {'type': 'disk', 'path':
>> UBUNTU_ISO}}
>> inst.templates_create(orig_params)
>> @@ -1198,32 +1207,36 @@ class ModelTests(unittest.TestCase):
>> # make sure "vm_update" works when the domain has a
>> snapshot
>> inst.vmsnapshots_create(u'kimchi-vm1')
>> - # update vm graphics when vm is not running
>> - inst.vm_update(u'kimchi-vm1',
>> - {"graphics": {"passwd": "123456"}})
>> + if os.uname()[4] != "s390x":
>> + # update vm graphics when vm is not running
>> + inst.vm_update(u'kimchi-vm1',
>> + {"graphics": {"passwd": "123456"}})
>> - inst.vm_start('kimchi-vm1')
>> - rollback.prependDefer(utils.rollback_wrapper,
>> inst.vm_poweroff,
>> - 'kimchi-vm1')
>> + inst.vm_start('kimchi-vm1')
>> + rollback.prependDefer(utils.rollback_wrapper,
>> inst.vm_poweroff,
>> + 'kimchi-vm1')
>> - vm_info = inst.vm_lookup(u'kimchi-vm1')
>> - self.assertEquals('123456', vm_info['graphics']["passwd"])
>> - self.assertEquals(None,
>> vm_info['graphics']["passwdValidTo"])
>> + vm_info = inst.vm_lookup(u'kimchi-vm1')
>> + self.assertEquals('123456',
>> vm_info['graphics']["passwd"])
>> + self.assertEquals(None,
>> vm_info['graphics']["passwdValidTo"])
>> - # update vm graphics when vm is running
>> - inst.vm_update(u'kimchi-vm1',
>> - {"graphics": {"passwd": "abcdef",
>> - "passwdValidTo": 20}})
>> - vm_info = inst.vm_lookup(u'kimchi-vm1')
>> - self.assertEquals('abcdef', vm_info['graphics']["passwd"])
>> - self.assertGreaterEqual(20,
>> vm_info['graphics']['passwdValidTo'])
>> + # update vm graphics when vm is running
>> + inst.vm_update(u'kimchi-vm1',
>> + {"graphics": {"passwd": "abcdef",
>> + "passwdValidTo": 20}})
>> + vm_info = inst.vm_lookup(u'kimchi-vm1')
>> + self.assertEquals('abcdef',
>> vm_info['graphics']["passwd"])
>> + self.assertGreaterEqual(20,
>> + vm_info['graphics']['passwdValidTo'])
>> - info = inst.vm_lookup('kimchi-vm1')
>> - self.assertEquals('running', info['state'])
>> + info = inst.vm_lookup('kimchi-vm1')
>> + self.assertEquals('running', info['state'])
>> - params = {'name': 'new-vm'}
>> - self.assertRaises(InvalidParameter, inst.vm_update,
>> - 'kimchi-vm1', params)
>> + params = {'name': 'new-vm'}
>> + self.assertRaises(InvalidParameter, inst.vm_update,
>> + 'kimchi-vm1', params)
>> + else:
>> + inst.vm_start('kimchi-vm1')
>> # change VM users and groups, when wm is running.
>> inst.vm_update(u'kimchi-vm1',
>> @@ -1294,7 +1307,8 @@ class ModelTests(unittest.TestCase):
>> inst.vm_update('kimchi-vm1', params)
>> rollback.prependDefer(utils.rollback_wrapper,
>> inst.vm_delete,
>> u'пeω-∨м')
>> - self.assertEquals(info['uuid'],
>> inst.vm_lookup(u'пeω-∨м')['uuid'])
>> + self.assertEquals(vm_info['uuid'],
>> + inst.vm_lookup(u'пeω-∨м')['uuid'])
>> info = inst.vm_lookup(u'пeω-∨м')
>> # Max memory is returned, add to test
>> params['memory']['maxmemory'] = 2048
>> diff --git a/tests/test_model_libvirtevents.py
>> b/tests/test_model_libvirtevents.py
>> index 69bf9d5..177bbae 100644
>> --- a/tests/test_model_libvirtevents.py
>> +++ b/tests/test_model_libvirtevents.py
>> @@ -54,7 +54,8 @@ def setUpModule():
>> def tearDownModule():
>> global TMP_DIR, TMP_EVENT
>> - os.unlink(TMP_EVENT)
>> + if os.path.exists(TMP_EVENT):
>> + os.unlink(TMP_EVENT)
>> shutil.rmtree(TMP_DIR)
>> @@ -120,7 +121,8 @@ class LibvirtEventsTests(unittest.TestCase):
>> data = {'domain': dom.name(), 'event': 'Rebooted'}
>> _store_event('%s|%s' % (_get_next_event_id(),
>> json.dumps(data)))
>> - @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>> + @unittest.skipUnless(utils.running_as_root() and
>> + os.uname()[4] != "s390x", 'Must be run as
>> root')
>> def test_events_vm_lifecycle(self):
>> inst = model.Model(objstore_loc=self.tmp_store)
>> self.objstore = inst.objstore
>> diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py
>> index c7a1d0d..e78e1c0 100644
>> --- a/tests/test_osinfo.py
>> +++ b/tests/test_osinfo.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 os
>> import unittest
>> from wok.plugins.kimchi.osinfo import _get_arch,
>> get_template_default, lookup
>> @@ -28,7 +29,8 @@ class OSInfoTests(unittest.TestCase):
>> entry = lookup(None, None)
>> self.assertEquals('unknown', entry['os_distro'])
>> self.assertEquals('unknown', entry['os_version'])
>> - self.assertEquals(['default'], entry['networks'])
>> + if not os.uname()[4] == "s390x":
>> + self.assertEquals(['default'], entry['networks'])
>> def test_old_distros(self):
>> old_versions = {'debian': '5.0', 'ubuntu': '7.04',
>> 'opensuse': '10.1',
>> @@ -41,12 +43,14 @@ class OSInfoTests(unittest.TestCase):
>> get_template_default('old',
>> 'nic_model'))
>> def test_modern_bases(self):
>> - for distro, version in
>> modern_version_bases[_get_arch()].iteritems():
>> - entry = lookup(distro, version)
>> - self.assertEquals(entry['disk_bus'],
>> - get_template_default('modern',
>> 'disk_bus'))
>> - self.assertEquals(entry['nic_model'],
>> - get_template_default('modern',
>> 'nic_model'))
>> + if not os.uname()[4] == "s390x":
>> + for distro, version in\
>> + modern_version_bases[_get_arch()].iteritems():
>> + entry = lookup(distro, version)
>> + self.assertEquals(entry['disk_bus'],
>> + get_template_default('modern', 'disk_bus'))
>> + self.assertEquals(entry['nic_model'],
>> + get_template_default('modern', 'nic_model'))
>> def test_modern_distros(self):
>> # versions based on ppc64 modern distros
>> diff --git a/tests/test_rest.py b/tests/test_rest.py
>> index 00ad7f3..3a61d13 100644
>> --- a/tests/test_rest.py
>> +++ b/tests/test_rest.py
>> @@ -162,9 +162,10 @@ class RestTests(unittest.TestCase):
>> resp = self.request('/plugins/kimchi/vms/vm-1', req, 'PUT')
>> self.assertEquals(400, resp.status)
>> - req = json.dumps({'memory': {'maxmemory': 3072}})
>> - resp = self.request('/plugins/kimchi/vms/vm-1', req, 'PUT')
>> - self.assertEquals(200, resp.status)
>> + if not os.uname()[4] == "s390x":
>> + req = json.dumps({'memory': {'maxmemory': 3072}})
>> + resp = self.request('/plugins/kimchi/vms/vm-1', req, 'PUT')
>> + self.assertEquals(200, resp.status)
>> resp = self.request('/plugins/kimchi/vms/vm-1/start',
>> '{}', 'POST')
>> self.assertEquals(200, resp.status)
>> @@ -252,7 +253,11 @@ class RestTests(unittest.TestCase):
>> # Memory was hot plugged
>> vm['name'] = u'∨м-црdαtеd'
>> vm['cpu_info'].update(params['cpu_info'])
>> - vm['memory'].update(params['memory'])
>> + if not os.uname()[4] == "s390x":
>> + vm['memory'].update(params['memory'])
>> + else:
>> + vm['memory']['current'] = 3072
>> + vm['memory']['maxmemory'] = 3072
> Why its 3072 and not read from params?
The previous change (line 162) updates maxmemory(for 'vm' dict) value to
3072. But since hotplug is not supported in s390x environment, it
remains unchanged. After updating the vm with the params, both current
and maxmemory(for 'vm_updated' dict) are set to 3072. This difference
would raise an exception for the check done in the next line. Reading
from params would only update current value, hence I manually updated
both with 3072.
>
>> for key in params.keys():
>> self.assertEquals(vm[key], vm_updated[key])
>> @@ -849,10 +854,11 @@ class RestTests(unittest.TestCase):
>> req = json.dumps({'path': cdrom})
>> resp =
>> self.request('/plugins/kimchi/vms/test-vm/storages/' +
>> cd_dev, req, 'PUT')
>> - self.assertEquals(200, resp.status)
>> - cd_info = json.loads(resp.read())
>> - self.assertEquals(urlparse.urlparse(cdrom).path,
>> - urlparse.urlparse(cd_info['path']).path)
>> + if not os.uname()[4] == "s390x":
>> + self.assertEquals(200, resp.status)
>> + cd_info = json.loads(resp.read())
>> + self.assertEquals(urlparse.urlparse(cdrom).path,
>> + urlparse.urlparse(cd_info['path']).path)
>> # Test GET
>> devs = json.loads(
>> @@ -885,7 +891,10 @@ class RestTests(unittest.TestCase):
>> self.assertEquals(200, resp.status)
>> # delete volumes
>> - l = '/plugins/kimchi/vms/test-vm/storages/hdd'
>> + if not os.uname()[4] == "s390x":
>> + l = '/plugins/kimchi/vms/test-vm/storages/hdd'
>> + else:
>> + l = '/plugins/kimchi/vms/test-vm/storages/vdb'
>> resp = self.request(l, {}, 'DELETE')
>> self.assertEquals(204, resp.status)
>> @@ -938,7 +947,8 @@ class RestTests(unittest.TestCase):
>> ifaces = json.loads(
>> self.request('/plugins/kimchi/vms/test-vm/ifaces').read()
>> )
>> - self.assertEquals(1, len(ifaces))
>> + if not os.uname()[4] == "s390x":
>> + self.assertEquals(1, len(ifaces))
>> for iface in ifaces:
>> res = json.loads(
>> @@ -1405,7 +1415,8 @@ class RestTests(unittest.TestCase):
>> self.assertIn('path', distro)
>> else:
>> # Distro not found error
>> - self.assertIn('KCHDISTRO0001E', distro.get('reason'))
>> + if distro.get('reason'):
>> + self.assertIn('KCHDISTRO0001E', distro.get('reason'))
>> # Test in PPC
>> ident = "Fedora 24 LE"
>> @@ -1420,7 +1431,8 @@ class RestTests(unittest.TestCase):
>> self.assertIn('path', distro)
>> else:
>> # Distro not found error
>> - self.assertIn('KCHDISTRO0001E', distro.get('reason'))
>> + if distro.get('reason'):
>> + self.assertIn('KCHDISTRO0001E', distro.get('reason'))
>> class HttpsRestTests(RestTests):
>> diff --git a/tests/test_template.py b/tests/test_template.py
>> index 727a354..72bf08d 100644
>> --- a/tests/test_template.py
>> +++ b/tests/test_template.py
>> @@ -110,6 +110,8 @@ class TemplateTests(unittest.TestCase):
>> tmpl = json.loads(
>> self.request('/plugins/kimchi/templates/test').read()
>> )
>> + if os.uname()[4] == "s390x":
>> + keys.append("interfaces")
>> self.assertEquals(sorted(tmpl.keys()), sorted(keys))
>> self.assertEquals(t['source_media']['path'], tmpl["cdrom"])
>> disk_keys = ['index', 'pool', 'size', 'format']
>> @@ -248,9 +250,14 @@ class TemplateTests(unittest.TestCase):
>> self.assertEquals(update_tmpl['cpu_info'],
>> cpu_info_data['cpu_info'])
>> # Test memory and max memory
>> - # - memory greated than max memory (1024 default)
>> - req = json.dumps({'memory': {'current': 2048}})
>> + # - memory greated than max memory (1024 default on x86
>> + # otherwise 2048)
>> + if os.uname()[4] == "s390x":
>> + req = json.dumps({'memory': {'current': 4096}})
>> + else:
>> + req = json.dumps({'memory': {'current': 2048}})
>> resp = self.request(new_tmpl_uri, req, 'PUT')
>> + print resp.read()
> Do we need print?
Oops. Forgot to remove it. Will send a v2.
>> self.assertEquals(400, resp.status)
>> # - max memory greater than limit: 16TiB to PPC and 4TiB to
>> x86
>> req = json.dumps({'memory': {'maxmemory': MAX_MEM_LIM +
>> 1024}})
>> diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py
>> index 74816ef..ca4eae5 100644
>> --- a/tests/test_vmtemplate.py
>> +++ b/tests/test_vmtemplate.py
>> @@ -56,6 +56,8 @@ class VMTemplateTests(unittest.TestCase):
>> args = {'name': 'test', 'cdrom': self.iso}
>> t = VMTemplate(args)
>> for name, val in fields:
>> + if os.uname()[4] == "s390x" and name == 'networks':
>> + continue
>> self.assertEquals(val, t.info.get(name))
>> def test_construct_overrides(self):
>> @@ -97,22 +99,29 @@ class VMTemplateTests(unittest.TestCase):
>> self.assertEquals(slots, xpath_get_text(xml, expr)[0])
>> def test_to_xml(self):
>> - graphics = {'type': 'spice', 'listen': '127.0.0.1'}
>> + if not os.uname()[4] == "s390x":
>> + graphics = {'type': 'spice', 'listen': '127.0.0.1'}
>> + else:
>> + graphics = {'type': 'vnc', 'listen': '127.0.0.1'}
>> vm_uuid = str(uuid.uuid4()).replace('-', '')
>> t = VMTemplate({'name': 'test-template', 'cdrom': self.iso})
>> xml = t.to_vm_xml('test-vm', vm_uuid, graphics=graphics)
>> self.assertEquals(vm_uuid, xpath_get_text(xml,
>> "/domain/uuid")[0])
>> self.assertEquals('test-vm', xpath_get_text(xml,
>> "/domain/name")[0])
>> - expr = "/domain/devices/graphics/@type"
>> - self.assertEquals(graphics['type'], xpath_get_text(xml,
>> expr)[0])
>> - expr = "/domain/devices/graphics/@listen"
>> - self.assertEquals(graphics['listen'], xpath_get_text(xml,
>> expr)[0])
>> + if not os.uname()[4] == "s390x":
>> + expr = "/domain/devices/graphics/@type"
>> + self.assertEquals(graphics['type'], xpath_get_text(xml,
>> expr)[0])
>> + expr = "/domain/devices/graphics/@listen"
>> + self.assertEquals(graphics['listen'],
>> xpath_get_text(xml, expr)[0])
>> expr = "/domain/maxMemory/@slots"
>> # The default is memory and maxmemory have the same value, so
>> # max memory tag is not set
>> self.assertEquals(0, len(xpath_get_text(xml, expr)))
>> expr = "/domain/memory"
>> - self.assertEquals(str(1024), xpath_get_text(xml, expr)[0])
>> + if os.uname()[4] == "s390x":
>> + self.assertEquals(str(2048), xpath_get_text(xml, expr)[0])
>> + else:
>> + self.assertEquals(str(1024), xpath_get_text(xml, expr)[0])
>> if hasattr(psutil, 'virtual_memory'):
>> host_memory = psutil.virtual_memory().total >> 10
>> @@ -158,6 +167,8 @@ class VMTemplateTests(unittest.TestCase):
>> t = VMTemplate({'name': 'test'}, netboot=True)
>> for name, val in fields:
>> + if os.uname()[4] == "s390x" and name == 'networks':
>> + continue
>> self.assertEquals(val, t.info.get(name))
>> self.assertNotIn('cdrom', t.info.keys())
>
More information about the Kimchi-devel
mailing list