12:38 p.m.
Aline pointed out some test failures I'd assumed were the same as
failures on the mater branch, but were actually as result of the
changes to check for valid url paths. The fix was causing a false
negative to be returned for valid repository URLs.
The only change in this patchset versus the previous (v3) version
is 2/3: Fix verification of remote ISO. 1/3 and 3/3 are the same
as version 3.
Christy Perez (3):
Fix Key Error when editing CD ROM path
Fix verification of remote ISO
Add unit tests for remote-backed CD ROM updates.
src/kimchi/model/vmstorages.py | 2 +-
src/kimchi/utils.py | 26 ++++++++++++++++++++++----
tests/test_model.py | 23 ++++++++++++++++++++++-
tests/utils.py | 20 +++++++++++++++++++-
4 files changed, 64 insertions(+), 7 deletions(-)
--
1.9.3
12:38 p.m.
New subject: [PATCH v4 1/3] Fix Key Error when editing CD ROM path
lxml only accepts strings as parameters when looking
up xml elements. The port for remote ISO was being
passed in as an int, and a KeyError was thrown.
This patch just casts the ints to strings for the
lookups.
Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com>
---
src/kimchi/model/vmstorages.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py
index b5311db..e5be17c 100644
--- a/src/kimchi/model/vmstorages.py
+++ b/src/kimchi/model/vmstorages.py
@@ -67,7 +67,7 @@ def _get_storage_xml(params, ignore_source=False):
# Working with url paths
if src_type == 'network':
output = urlparse.urlparse(params.get('path'))
- port = output.port or socket.getservbyname(output.scheme)
+ port = str(output.port or socket.getservbyname(output.scheme))
host = E.host(name=output.hostname, port=port)
source = E.source(protocol=output.scheme, name=output.path)
source.append(host)
--
1.9.3
12:38 p.m.
New subject: [PATCH v4 2/3] Fix verification of remote ISO
Currently the entire ISO is grabbed, which times out.
Use httplib to connect to the server and just get
the head of the http object.
Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com>
---
src/kimchi/utils.py | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
index 9ed8802..efd3ba9 100644
--- a/src/kimchi/utils.py
+++ b/src/kimchi/utils.py
@@ -28,9 +28,10 @@
import traceback
import urllib2
import xml.etree.ElementTree as ET
+from httplib import HTTPConnection, HTTPException
from multiprocessing import Process, Queue
from threading import Timer
-
+from urlparse import urlparse
from cherrypy.lib.reprconf import Parser
from kimchi.asynctask import AsyncTask
@@ -138,14 +139,31 @@ def import_module(module_name):
return __import__(module_name, globals(), locals(), [''])
+def url_to_fqdn(path):
+ parse_result = urlparse(path)
+ return parse_result.netloc
+
+
def check_url_path(path):
try:
- code = urllib2.urlopen(path).getcode()
+ code = ''
+ server_name = url_to_fqdn(path)
+ url_parts = path.split('://') # [0] = prefix, [1] = rest of URL
+ if len(url_parts) > 1 and server_name == url_parts[1] :
+ # Just a server, as with a repo.
+ code = urllib2.urlopen(path).getcode()
+ else:
+ # socket.gaierror could be raised,
+ # which is a child class of IOError
+ conn = HTTPConnection(server_name, timeout=15)
+ # Don't try to get the whole file:
+ conn.request('HEAD', path)
+ code = conn.getresponse().status
+ conn.close()
if code != 200:
return False
- except (urllib2.URLError, ValueError):
+ except (urllib2.URLError, HTTPException, IOError, ValueError):
return False
-
return True
--
1.9.3
12:38 p.m.
New subject: [PATCH v4 3/3] Add unit tests for remote-backed CD ROM updates.
Adding tests to ensure that updating an existing CD ROM
can be done when using remote ISOs. This is to test for
regression for the libxml KeyError seen when even saving
the same CD ROM ISO as the update.
Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com>
---
tests/test_model.py | 23 ++++++++++++++++++++++-
tests/utils.py | 20 +++++++++++++++++++-
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py
index 92b34b6..7fd0f00 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -23,6 +23,7 @@
import platform
import psutil
import pwd
+import re
import shutil
import tempfile
import threading
@@ -285,7 +286,7 @@ def _attach_disk(expect_bus='virtio'):
self.assertEquals(u'disk', disk_info['type'])
inst.vmstorage_delete(vm_name, disk)
- # Specify pool and path at sametime will fail
+ # Specifying pool and path at same time will fail
disk_args = {"type": "disk",
"pool": pool,
"vol": vol,
@@ -361,6 +362,11 @@ def test_vm_cdrom(self):
self.assertRaises(InvalidParameter, inst.vmstorage_update,
vm_name, cdrom_dev, {'path': wrong_iso_path})
+ # Make sure CD ROM still exists after failure
+ cd_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
+ self.assertEquals(u'cdrom', cd_info['type'])
+ self.assertEquals(iso_path, cd_info['path'])
+
# update path of existing cd with existent iso of shutoff vm
inst.vmstorage_update(vm_name, cdrom_dev, {'path': iso_path2})
cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
@@ -387,6 +393,21 @@ def test_vm_cdrom(self):
storage_list = inst.vmstorages_get_list(vm_name)
self.assertEquals(prev_count, len(storage_list))
+ # Create a new cdrom using a remote iso
+ valid_remote_iso_path = utils.get_remote_iso_path()
+ cdrom_args = {"type": "cdrom",
+ "path": valid_remote_iso_path}
+ cdrom_dev = inst.vmstorages_create(vm_name, cdrom_args)
+ storage_list = inst.vmstorages_get_list(vm_name)
+ self.assertEquals(prev_count + 1, len(storage_list))
+
+ # Update remote-backed cdrom with the same ISO
+ inst.vmstorage_update(vm_name, cdrom_dev,
+ {'path': valid_remote_iso_path})
+ cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
+ cur_cdrom_path = re.sub(":80/", '/',
cdrom_info['path'])
+ self.assertEquals(valid_remote_iso_path, cur_cdrom_path)
+
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root')
def test_vm_storage_provisioning(self):
inst = model.Model(objstore_loc=self.tmp_store)
diff --git a/tests/utils.py b/tests/utils.py
index 30c9cae..7f9570e 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -21,19 +21,20 @@
import base64
import cherrypy
import httplib
+import json
import os
import socket
import sys
import threading
import unittest
-
from contextlib import closing
from lxml import etree
import kimchi.mockmodel
import kimchi.server
+from kimchi.config import paths
from kimchi.exception import OperationFailed
_ports = {}
@@ -146,6 +147,23 @@ def request(host, port, path, data=None, method='GET',
headers=None):
conn = httplib.HTTPSConnection(host, port)
return _request(conn, path, data, method, headers)
+def get_remote_iso_path():
+ """
+ Get a remote iso with the right arch from the distro files shipped
+ with kimchi.
+ """
+ host_arch = os.uname()[4]
+ remote_path = ''
+ with open(os.path.join(paths.conf_dir, 'distros.d', 'fedora.json'))\
+ as fedora_isos:
+ # Get a list of dicts
+ json_isos_list = json.load(fedora_isos)
+ for iso in json_isos_list:
+ if (iso.get('os_arch')) == host_arch:
+ remote_path = iso.get('path')
+ break
+
+ return remote_path
def patch_auth(sudo=True):
"""
--
1.9.3
8:02 p.m.
New subject: [PATCH v4 2/3] Fix verification of remote ISO
On 08/12/2014 02:38 PM, Christy Perez wrote:
Currently the entire ISO is grabbed, which times out.
Use httplib to connect to the server and just get
the head of the http object.
Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com>
---
src/kimchi/utils.py | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
index 9ed8802..efd3ba9 100644
--- a/src/kimchi/utils.py
+++ b/src/kimchi/utils.py
@@ -28,9 +28,10 @@
import traceback
import urllib2
import xml.etree.ElementTree as ET
+from httplib import HTTPConnection, HTTPException
from multiprocessing import Process, Queue
from threading import Timer
-
+from urlparse import urlparse
from cherrypy.lib.reprconf import Parser
from kimchi.asynctask import AsyncTask
@@ -138,14 +139,31 @@ def import_module(module_name):
return __import__(module_name, globals(), locals(), [''])
+def url_to_fqdn(path):
+ parse_result = urlparse(path)
+ return parse_result.netloc
+
+
def check_url_path(path):
try:
- code = urllib2.urlopen(path).getcode()
+ code = ''
+ server_name = url_to_fqdn(path)
+ url_parts = path.split('://') # [0] = prefix, [1] =
rest of URL
+ if len(url_parts) > 1 and server_name == url_parts[1] :
You can use urlparse(path) here too and check the existence of a path
parse_result = urlparse(path)
server_name = parse_result.netloc
urlpath = parse_result.path
if not urlpath:
...
else:
...
As you need to update it, I think we can also remove url_to_fqdn()
function as it is not used in other place
+ # Just a server, as with a repo.
+ code = urllib2.urlopen(path).getcode()
+ else:
+ # socket.gaierror could be raised,
+ # which is a child class of IOError
+ conn = HTTPConnection(server_name, timeout=15)
+ # Don't try to get the whole file:
+ conn.request('HEAD', path)
+ code = conn.getresponse().status
+ conn.close()
if code != 200:
return False
- except (urllib2.URLError, ValueError):
+ except (urllib2.URLError, HTTPException, IOError, ValueError):
return False
-
return True
8:55 a.m.
New subject: [PATCH v4 2/3] Fix verification of remote ISO
On 08/12/2014 08:02 PM, Aline Manera wrote:
On 08/12/2014 02:38 PM, Christy Perez wrote:
> Currently the entire ISO is grabbed, which times out.
> Use httplib to connect to the server and just get
> the head of the http object.
>
> Signed-off-by: Christy Perez <christy(a)linux.vnet.ibm.com>
> ---
> src/kimchi/utils.py | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
> index 9ed8802..efd3ba9 100644
> --- a/src/kimchi/utils.py
> +++ b/src/kimchi/utils.py
> @@ -28,9 +28,10 @@
> import traceback
> import urllib2
> import xml.etree.ElementTree as ET
> +from httplib import HTTPConnection, HTTPException
> from multiprocessing import Process, Queue
> from threading import Timer
> -
> +from urlparse import urlparse
> from cherrypy.lib.reprconf import Parser
>
> from kimchi.asynctask import AsyncTask
> @@ -138,14 +139,31 @@ def import_module(module_name):
> return __import__(module_name, globals(), locals(), [''])
>
>
> +def url_to_fqdn(path):
> + parse_result = urlparse(path)
> + return parse_result.netloc
> +
> +
> def check_url_path(path):
> try:
> - code = urllib2.urlopen(path).getcode()
> + code = ''
> + server_name = url_to_fqdn(path)
> + url_parts = path.split('://') # [0] = prefix, [1] = rest of URL
> + if len(url_parts) > 1 and server_name == url_parts[1] :
You can use urlparse(path) here too and check the existence of a path
Thanks.
I'd just copied that from somewhere else, so I'll update that to
do the same if it works in that situation as well -- and send a v5.
parse_result = urlparse(path)
server_name = parse_result.netloc
urlpath = parse_result.path
if not urlpath:
...
else:
...
As you need to update it, I think we can also remove url_to_fqdn()
function as it is not used in other place
> + # Just a server, as with a repo.
> + code = urllib2.urlopen(path).getcode()
> + else:
> + # socket.gaierror could be raised,
> + # which is a child class of IOError
> + conn = HTTPConnection(server_name, timeout=15)
> + # Don't try to get the whole file:
> + conn.request('HEAD', path)
> + code = conn.getresponse().status
> + conn.close()
> if code != 200:
> return False
> - except (urllib2.URLError, ValueError):
> + except (urllib2.URLError, HTTPException, IOError, ValueError):
> return False
> -
> return True
>
>
3552
days inactive
3554
days old
6 comments
2 participants
participants (2)
-
Aline Manera -
Christy Perez