Nir Soffer
2018-Oct-09 11:28 UTC
Re: [Libguestfs] [PATCH v2 3/3] v2v: -o rhv-upload: Add a test.
On Thu, Sep 20, 2018 at 11:51 AM Richard W.M. Jones <rjones@redhat.com> wrote:> Previously this output method was almost completely untested. > > This commit adds a fake ovirtsdk4 module so we can test the > -o rhv-upload method fairly completely without needing an actual > oVirt instance around. > --- > v2v/Makefile.am | 4 + > .../ovirtsdk4/__init__.py | 146 ++++++++++++++++++ > .../ovirtsdk4/types.py | 125 +++++++++++++++ > v2v/test-v2v-o-rhv-upload.sh | 51 ++++++ > 4 files changed, 326 insertions(+) > > diff --git a/v2v/Makefile.am b/v2v/Makefile.am > index 14183572b..aab356637 100644 > --- a/v2v/Makefile.am > +++ b/v2v/Makefile.am > @@ -382,6 +382,7 @@ TESTS += \ > test-v2v-o-openstack.sh \ > test-v2v-o-qemu.sh \ > test-v2v-o-rhv.sh \ > + test-v2v-o-rhv-upload.sh \ > test-v2v-o-vdsm-options.sh \ > test-v2v-oa-option.sh \ > test-v2v-of-option.sh \ > @@ -539,6 +540,9 @@ EXTRA_DIST += \ > test-v2v-o-qemu.sh \ > test-v2v-o-rhv.ovf.expected \ > test-v2v-o-rhv.sh \ > + test-v2v-o-rhv-upload.sh \ > + test-v2v-o-rhv-upload-module/ovirtsdk4/__init__.py \ > + test-v2v-o-rhv-upload-module/ovirtsdk4/types.py \ > test-v2v-o-rhv-upload-oo-query.sh \ > test-v2v-o-vdsm-oo-query.sh \ > test-v2v-o-vdsm-options.ovf.expected \ > diff --git a/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/__init__.py > b/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/__init__.py > new file mode 100644 > index 000000000..2ddb950ea > --- /dev/null > +++ b/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/__init__.py > @@ -0,0 +1,146 @@ > +# -*- python -*- > +# Copyright (C) 2018 Red Hat Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License along > +# with this program; if not, write to the Free Software Foundation, Inc., > +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + > +# Fake ovirtsdk4 module used as a test harness. > +# See v2v/test-v2v-o-rhv-upload.sh > + > +class Error(Exception): > + pass > +class NotFoundError(Error): > + pass > + > +class Connection(object): > + def __init__( > + self, > + url = None, > + username = None, > + password = None, > + ca_file = None, > + log = None, > + insecure = False, > + ): > + pass > + > + def system_service(self): > + return SystemService() > + > +class SystemService(object): > + def data_centers_service(self): > + return DataCentersService() > + > + def disks_service(self): > + return DisksService() > + > + def image_transfers_service(self): > + return ImageTransfersService() > + > + def storage_domains_service(self): > + return StorageDomainsService() > + > + def vms_service(self): > + return VmsService() > + > +class DataCentersService(object): > + def list(self, search=None, case_sensitive=False): > + return [] > + > +class DiskService(object): > + def __init__(self, disk_id): > + self._disk_id = disk_id > + > + def get(self): > + return types.Disk() > + > + def remove(self): > + pass > + > +class DisksService(object): > + def add(self, disk=None): > + return disk > + > + def disk_service(self, disk_id): > + return DiskService(disk_id) > + > +class ImageTransferService(object): > + def __init__(self): > + self._finalized = False > + > + def get(self): > + if self._finalized: > + raise NotFoundError > + else: > + return types.ImageTransfer() > + > + def finalize(self): > + self._finalized = True > + > +class ImageTransfersService(object): > + def add(self, transfer): > + return transfer > + > + def image_transfer_service(self, id): > + return ImageTransferService() > + > +class StorageDomain(object): > + id = "ba87af68-b630-4211-a73a-694c1a689405" > + > +class StorageDomainsService(object): > + def list(self, search=None): > + return [ StorageDomain() ] > + > +class VmsService(object): > + def add(self, vm): > + return vm > + > + def list(self, search=None): > + return [] > + > +# Create a background thread running a web server which is > +# simulating the imageio server. >This functionality should be separated from the fake SDK module, since it is not part of the SDK, and may be replaced by real imageio server later.> + > +from http.server import HTTPServer, BaseHTTPRequestHandler > +import random > +import threading > + > +# Choose a random port number in range [50000,59999] > +imageio_port = random.randint(50000,60000) > + > +class RequestHandler(BaseHTTPRequestHandler): >This request handler is using HTTP/1.0, and will close the connection after every request. This is not a good implementation of the imageio server, and also hides bugs in this code. Should be fixed by adding: protocol_version = "HTTP/1.1"> + def do_OPTIONS(self):+ self.send_response(200)> + self.send_header("Content-type", "application/json; > charset=UTF-8") > + self.end_headers() > + # Advertize only zero support. > + self.wfile.write(b'''{ "features": [ "zero" ] }''') > + > + # eg. zero request. Just ignore it. > + def do_PATCH(self): >This must read content-length bytes from self.rfile.> + self.send_response(200) > + self.end_headers() > + > + # Flush request. Ignore it. > + def do_PUT(self): >This must read content-length bytes from self.rfile. Both bugs are hidden by the fact that this server close the connection at the end of the request.> + self.send_response(200) > + self.end_headers() > + > +def server(): > + server_address = ("", imageio_port) > + httpd = HTTPServer(server_address, RequestHandler) > + httpd.serve_forever() >Using HTTP instead of HTTPS is not a good idea. We are not testing the same code on the client side. It is easy to run HTTPS server using pre-created certificate, if we disable certificate verification on the client side (e.g. --insecure). Nir> + > +thread = threading.Thread(target = server, args = [], daemon = True) > +thread.start() > diff --git a/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/types.py > b/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/types.py > new file mode 100644 > index 000000000..9b3f557ee > --- /dev/null > +++ b/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/types.py > @@ -0,0 +1,125 @@ > +# -*- python -*- > +# Copyright (C) 2018 Red Hat Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License along > +# with this program; if not, write to the Free Software Foundation, Inc., > +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + > +# Fake ovirtsdk4 module used as a test harness. > +# See v2v/test-v2v-o-rhv-upload.sh > + > +from enum import Enum > +from ovirtsdk4 import imageio_port > + > +class Cluster(object): > + def __init__(self, name): > + pass > + > +class Configuration(object): > + def __init__(self, type=None, data=None): > + pass > +class ConfigurationType(Enum): > + OVA = 'ova' > + OVF = 'ovf' > + > + def __init__(self, image): > + self._image = image > + > + def __str__(self): > + return self._image > + > +class DiskFormat(Enum): > + COW = "cow" > + RAW = "raw" > + > + def __init__(self, image): > + self._image = image > + > + def __str__(self): > + return self._image > + > +class DiskStatus(Enum): > + ILLEGAL = "illegal" > + LOCKED = "locked" > + OK = "ok" > + > + def __init__(self, image): > + self._image = image > + > + def __str__(self): > + return self._image > + > +class Disk(object): > + def __init__( > + self, > + id = None, > + name = None, > + description = None, > + format = None, > + initial_size = None, > + provisioned_size = None, > + sparse = False, > + storage_domains = None > + ): > + pass > + > + id = 123 > + status = DiskStatus.OK > + > +class ImageTransferPhase(Enum): > + CANCELLED = 'cancelled' > + FINALIZING_FAILURE = 'finalizing_failure' > + FINALIZING_SUCCESS = 'finalizing_success' > + FINISHED_FAILURE = 'finished_failure' > + FINISHED_SUCCESS = 'finished_success' > + INITIALIZING = 'initializing' > + PAUSED_SYSTEM = 'paused_system' > + PAUSED_USER = 'paused_user' > + RESUMING = 'resuming' > + TRANSFERRING = 'transferring' > + UNKNOWN = 'unknown' > + > + def __init__(self, image): > + self._image = image > + > + def __str__(self): > + return self._image > + > +class ImageTransfer(object): > + def __init__( > + self, > + disk = None, > + host = None, > + inactivity_timeout = None, > + ): > + pass > + > + id = 456 > + phase = ImageTransferPhase.TRANSFERRING > + transfer_url = "http://localhost:" + str(imageio_port) + "/" > + > +class Initialization(object): > + def __init__(self, configuration): > + pass > + > +class StorageDomain(object): > + def __init__(self, name = None): > + pass > + > +class Vm(object): > + def __init__( > + self, > + cluster = None, > + initialization = None > + ): > + pass > diff --git a/v2v/test-v2v-o-rhv-upload.sh b/v2v/test-v2v-o-rhv-upload.sh > new file mode 100755 > index 000000000..8bda7cc0b > --- /dev/null > +++ b/v2v/test-v2v-o-rhv-upload.sh > @@ -0,0 +1,51 @@ > +#!/bin/bash - > +# libguestfs virt-v2v test script > +# Copyright (C) 2018 Red Hat Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write to the Free Software > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > 02110-1301 USA. > + > +# Test -o rhv-upload. > +# > +# These uses a test harness (see > +# v2v/test-v2v-o-rhv-upload-module/ovirtsdk4) to fake responses from > +# oVirt. > + > +set -e > +set -x > + > +$TEST_FUNCTIONS > +skip_if_skipped > +skip_if_backend uml > +skip_unless_phony_guest windows.img > + > +libvirt_uri="test://$abs_top_builddir/test-data/phony-guests/guests.xml" > +f=$top_builddir/test-data/phony-guests/windows.img > + > +export VIRT_TOOLS_DATA_DIR="$top_srcdir/test-data/fake-virt-tools" > +export VIRTIO_WIN="$top_srcdir/test-data/fake-virtio-win" > +export PYTHONPATH=$srcdir/test-v2v-o-rhv-upload-module:$PYTHONPATH > + > +# Run virt-v2v -o rhv-upload. > +# > +# The fake ovirtsdk4 module doesn't care about most of the options > +# like -oc, -oo rhv-cafile, -op etc. Any values may be used. > +$VG virt-v2v --debug-gc -v -x \ > + -i libvirt -ic "$libvirt_uri" windows \ > + -o rhv-upload \ > + -oc https://example.com/ovirt-engine/api \ > + -oo rhv-cafile=/dev/null \ > + -oo rhv-direct \ > + -op /dev/null \ > + -os . > -- > 2.19.0.rc0 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs >
Richard W.M. Jones
2018-Oct-15 15:21 UTC
Re: [Libguestfs] [PATCH v2 3/3] v2v: -o rhv-upload: Add a test.
On Tue, Oct 09, 2018 at 02:28:10PM +0300, Nir Soffer wrote:> > +# Create a background thread running a web server which is > > +# simulating the imageio server. > > > > This functionality should be separated from the fake SDK module, since it is > not part of the SDK, and may be replaced by real imageio server later.Well possibly, but it was very convenient to put it in the class here, and this test is meant for running completely standalone without any other service available.> > +class RequestHandler(BaseHTTPRequestHandler): > > > > This request handler is using HTTP/1.0, and will close the connection after > every request. This is not a good implementation of the imageio server, and > also > hides bugs in this code. > > Should be fixed by adding: > > protocol_version = "HTTP/1.1"I tried the attached patch, but for some reason it just hangs.> > + def do_OPTIONS(self): > > + self.send_response(200) > > + self.send_header("Content-type", "application/json; > > charset=UTF-8") > > + self.end_headers() > > + # Advertize only zero support. > > + self.wfile.write(b'''{ "features": [ "zero" ] }''') > > + > > + # eg. zero request. Just ignore it. > > + def do_PATCH(self): > > > > This must read content-length bytes from self.rfile. > > > > + self.send_response(200) > > + self.end_headers() > > + > > + # Flush request. Ignore it. > > + def do_PUT(self): > > > > This must read content-length bytes from self.rfile. > > Both bugs are hidden by the fact that this server close the connection at > the end > of the request.Also this is covered by the attached patch.> > + self.send_response(200) > > + self.end_headers() > > + > > +def server(): > > + server_address = ("", imageio_port) > > + httpd = HTTPServer(server_address, RequestHandler) > > + httpd.serve_forever() > > > > Using HTTP instead of HTTPS is not a good idea. We are not testing > the same code on the client side. > > It is easy to run HTTPS server using pre-created certificate, if we > disable certificate verification on the client side > (e.g. --insecure).This one doesn't seem to be as simple to fix. However point taken, so I added a note in the code. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Nir Soffer
2018-Oct-15 15:43 UTC
Re: [Libguestfs] [PATCH v2 3/3] v2v: -o rhv-upload: Add a test.
On Mon, Oct 15, 2018 at 6:21 PM Richard W.M. Jones <rjones@redhat.com> wrote:> On Tue, Oct 09, 2018 at 02:28:10PM +0300, Nir Soffer wrote: > > > +# Create a background thread running a web server which is > > > +# simulating the imageio server. > > > > > > > This functionality should be separated from the fake SDK module, since > it is > > not part of the SDK, and may be replaced by real imageio server later. > > Well possibly, but it was very convenient to put it in the class here, > and this test is meant for running completely standalone without any > other service available. > > > > +class RequestHandler(BaseHTTPRequestHandler): > > > > > > > This request handler is using HTTP/1.0, and will close the connection > after > > every request. This is not a good implementation of the imageio server, > and > > also > > hides bugs in this code. > > > > Should be fixed by adding: > > > > protocol_version = "HTTP/1.1" > > I tried the attached patch, but for some reason it just hangs. >The issue is probably missing "content-length: 0" header in the response. If we don't close the connection and don't send content-length the client cannot do much but wait :-)> > + def do_OPTIONS(self): > > > > + self.send_response(200) > > > + self.send_header("Content-type", "application/json; > > > charset=UTF-8") > > > + self.end_headers() > > > + # Advertize only zero support. > > > + self.wfile.write(b'''{ "features": [ "zero" ] }''') > > > + > > > + # eg. zero request. Just ignore it. > > > + def do_PATCH(self): > > > > > > > This must read content-length bytes from self.rfile. > > > > > > > + self.send_response(200) > > > + self.end_headers() > > > + > > > + # Flush request. Ignore it. > > > + def do_PUT(self): > > > > > > > This must read content-length bytes from self.rfile. > > > > Both bugs are hidden by the fact that this server close the connection at > > the end > > of the request. > > Also this is covered by the attached patch. > > > > + self.send_response(200) > > > + self.end_headers() > > > + > > > +def server(): > > > + server_address = ("", imageio_port) > > > + httpd = HTTPServer(server_address, RequestHandler) > > > + httpd.serve_forever() > > > > > > > Using HTTP instead of HTTPS is not a good idea. We are not testing > > the same code on the client side. > > > > It is easy to run HTTPS server using pre-created certificate, if we > > disable certificate verification on the client side > > (e.g. --insecure). > > This one doesn't seem to be as simple to fix. However point taken, so > I added a note in the code. > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat > http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-builder quickly builds VMs from scratch > http://libguestfs.org/virt-builder.1.html >