Skip to content
This repository has been archived by the owner on Sep 12, 2022. It is now read-only.

Fixes for application_to_provider Glance client v1 native (non-iRODS) upload method #618

Merged
merged 2 commits into from
May 18, 2018

Conversation

c-mart
Copy link
Contributor

@c-mart c-mart commented May 16, 2018

Description

Glance Client v1 uses a different method for uploading image data; this PR accommodates that.

This codepath was previously untested with Glance Client v1 because we were using the "direct iRODS transfer" mode of application_to_provider. We recently stopped using that (because Marana Cloud Glance is now backed by Ceph), and this bug surfaced.

Also, apparently in Glance API v1, it is valid for an image's checksum to be missing even though that image is in active state; we can catch this and log a warning.

Checklist before merging Pull Requests

  • New test(s) included to reproduce the bug/verify the feature
  • Add an entry in the changelog
  • Documentation created/updated (include links)
  • If creating/modifying DB models which will contain secrets or sensitive information, PR to clank updating sanitation queries in roles/sanitary-sql-access/templates/sanitize-dump.sh.j2
  • Reviewed and approved by at least one other contributor.
  • New variables supported in Clank
  • New variables committed to secrets repos

@coveralls
Copy link

coveralls commented May 16, 2018

Coverage Status

Coverage decreased (-0.007%) to 37.31% when pulling fe31d74 on c-mart:app-to-provider-glanceclient-v1-fixes into bd6d66a on cyverse:master.

@c-mart c-mart changed the title WIP: Fixes for application_to_provider Glance client v1 native (non-iRODS) upload method Fixes for application_to_provider Glance client v1 native (non-iRODS) upload method May 16, 2018
@@ -577,7 +580,11 @@ def migrate_image_data_glance(src_glance_client, dst_glance_client, img_uuid, lo
logging.debug("Attempting to upload image data to destination provider")
with open(local_path, 'rb') as img_file:
try:
dst_glance_client.images.upload(img_uuid, img_file)
# "Upload" method is different for Glance client v1, than for v2
if isinstance(dst_glance_client, glanceclient.v1.client.Client):
Copy link
Contributor

Choose a reason for hiding this comment

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

Sort of minor, did a bit of reading to confirm this, you have the option to do type(dst_glance_client) is glanceclient.v1.client.Client. If the v2 client was a subclass of the v1 client, then your check wouldn't work as intended. isinstance is typically preferred, because of duck typing (when you want to check if an instance has at minimum the behavior of some class and its parents), but in cases like this I think the preferred approach is to check for an exact class match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sound reasonable, change made as suggested

@cdosborn cdosborn force-pushed the app-to-provider-glanceclient-v1-fixes branch from fe31d74 to e7e7370 Compare May 18, 2018 22:40
@cdosborn cdosborn merged commit 57a6c6e into cyverse:master May 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants