simplestreams

Merge lp://qastaging/~danilo/simplestreams/ignore-inactive-images into lp://qastaging/simplestreams

  1. ignore-inactive-images
  2. Merge into trunk
Proposed by Данило Шеган
Status: Merged
Merged at revision: 433
Proposed branch: lp://qastaging/~danilo/simplestreams/ignore-inactive-images
Merge into: lp://qastaging/simplestreams
Diff against target: 15 lines (+5/-0)
1 file modified
simplestreams/mirrors/glance.py (+5/-0)
To merge this branch: bzr merge lp://qastaging/~danilo/simplestreams/ignore-inactive-images
Reviewer Review Type Date Requested Status
simplestreams-dev Pending
Review via email: mp+295240@code.qastaging.launchpad.net

Description of the change

If connection to glance is broken during GlanceMirror.sync(), and image would be left over in "status": "saving".

Glance itself tries to protect against that, however when it is restarted (eg. "service glance-api restart"), it does not clean the image up. (I still think glance should be fixed to at least clean up "saving" images when it's gracefully restarted, so I'll file a bug about that if I can find exactly where to do that :).

This change makes load_product() ignore any images that do not have "status": "active". I wasn't sure if I should maybe only check for status != "saving" instead, but it seems images can only have deleted, active or saving status, so this should be good.

Note that this isn't perfect. The image in "saving" state is never cleaned up, but at least the resulting cloud works. If we want to clean up broken images, it'd be hard to do on the simplestreams side (glance is better placed to do that: it knows if anything is going on with any of them, eg. is it being currently updated or not).

I didn't provide any tests though: load_products is currently untested and would need some serious refactoring or mocking and faking to get any decent test for this.

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

is status going to be guaranteed to be there in all image response?
I suspect it is, but should be reasonable sure of that so we dont get a KeyError on image['status']

Revision history for this message
Данило Шеган (danilo) wrote :

That's a good point. I'll double check glance code once again, though it seemed to me that image is first created with {"status": "saving"}, and on successful upload completion turned into {"status": "active"}. Will report back when I look at it (tomorrow).

Of course, there's no harm in doing image.get('status') as well to ensure we don't hit the problem even if things change in glance.

lp://qastaging/~danilo/simplestreams/ignore-inactive-images updated
433. By Данило Шеган

Switch to .get('status') to not barf when image does not have status defined.

434. By Данило Шеган

Be consistent in using .get().

Revision history for this message
Данило Шеган (danilo) wrote :

I've confirmed that status is always defined from the looks of it, but switched to .get('status') just in case.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches