Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support direct upload/download to Image I/O daemon on oVirt node #35

Merged
merged 6 commits into from
May 25, 2020
Merged

Support direct upload/download to Image I/O daemon on oVirt node #35

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f281406
Support direct upload/download to Image I/O daemon on oVirt node
ganto May 2, 2020
af575d4
ovirt_disk: auto-detect proxy connection based on engine version
ganto May 2, 2020
36446c7
Use proxy as fallback
mnecas May 21, 2020
7c90057
add create_transfer_connection
mnecas May 21, 2020
c6da921
correct warn msg
mnecas May 21, 2020
0fdab53
update transfer names
mnecas May 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 30 additions & 25 deletions plugins/modules/ovirt_disk.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,25 @@ def _search_by_lun(disks_service, lun_id):
return res[0] if res else None


def create_transfer_connection(module, transfer, context, connect_timeout=10, read_timeout=60):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be useful to expose the timeouts to the user so they can override the timeouts if needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah thought about that but we have default timeout 3min and not sure if it would be best to wait that long

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would this to future work.

url = urlparse(transfer.transfer_url)
connection = HTTPSConnection(
url.netloc, context=context, timeout=connect_timeout)
try:
connection.connect()
except OSError as e:
# Typically ConnectionRefusedError or socket.gaierror.
module.warn("Cannot connect to %s, trying %s: %s" % (transfer.transfer_url, transfer.proxy_url, e))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same as using {}. Please check module.warn() interface - does it work like Pyhon logging, accepting
fmt, *args or it a single srting?

Then check how other code in this area generate messages and use the same style.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that was your question it only accepts one string no other args.
And it is only message the warning styling is done automatically.


url = urlparse(transfer.proxy_url)
connection = HTTPSConnection(
url.netloc, context=context, timeout=connect_timeout)
connection.connect()

connection.sock.settimeout(read_timeout)
return connection, url


def transfer(connection, module, direction, transfer_func):
transfers_service = connection.system_service().image_transfers_service()
transfer = transfers_service.add(
Expand All @@ -366,7 +385,6 @@ def transfer(connection, module, direction, transfer_func):
time.sleep(module.params['poll_interval'])
transfer = transfer_service.get()

proxy_url = urlparse(transfer.proxy_url)
context = ssl.create_default_context()
auth = module.params['auth']
if auth.get('insecure'):
Expand All @@ -375,17 +393,11 @@ def transfer(connection, module, direction, transfer_func):
elif auth.get('ca_file'):
context.load_verify_locations(cafile=auth.get('ca_file'))

proxy_connection = HTTPSConnection(
proxy_url.hostname,
proxy_url.port,
context=context,
)

transfer_connection, transfer_url = create_transfer_connection(module, transfer, context)
transfer_func(
transfer_service,
proxy_connection,
proxy_url,
transfer.signed_ticket
transfer_connection,
transfer_url,
)
return True
finally:
Expand Down Expand Up @@ -416,17 +428,10 @@ def transfer(connection, module, direction, transfer_func):


def download_disk_image(connection, module):
def _transfer(transfer_service, proxy_connection, proxy_url, transfer_ticket):
def _transfer(transfer_service, transfer_connection, transfer_url):
BUF_SIZE = 128 * 1024
transfer_headers = {
'Authorization': transfer_ticket,
}
proxy_connection.request(
'GET',
proxy_url.path,
headers=transfer_headers,
)
r = proxy_connection.getresponse()
transfer_connection.request('GET', transfer_url.path)
r = transfer_connection.getresponse()
path = module.params["download_image_path"]
image_size = int(r.getheader('Content-Length'))
with open(path, "wb") as mydisk:
Expand All @@ -448,14 +453,14 @@ def _transfer(transfer_service, proxy_connection, proxy_url, transfer_ticket):


def upload_disk_image(connection, module):
def _transfer(transfer_service, proxy_connection, proxy_url, transfer_ticket):
def _transfer(transfer_service, transfer_connection, transfer_url):
BUF_SIZE = 128 * 1024
path = module.params['upload_image_path']

image_size = os.path.getsize(path)
proxy_connection.putrequest("PUT", proxy_url.path)
proxy_connection.putheader('Content-Length', "%d" % (image_size,))
proxy_connection.endheaders()
transfer_connection.putrequest("PUT", transfer_url.path)
transfer_connection.putheader('Content-Length', "%d" % (image_size,))
transfer_connection.endheaders()
with open(path, "rb") as disk:
pos = 0
while pos < image_size:
Expand All @@ -464,7 +469,7 @@ def _transfer(transfer_service, proxy_connection, proxy_url, transfer_ticket):
if not chunk:
transfer_service.pause()
raise RuntimeError("Unexpected end of file at pos=%d" % pos)
proxy_connection.send(chunk)
transfer_connection.send(chunk)
pos += len(chunk)

return transfer(
Expand Down