Open Stack

Two inline replies, throw out my quick thougths.

On Sat, Aug 2, 2014 at 4:41 AM, Jay Pipes <jaypipes at gmail.com> wrote:
> cc'ing ML since it's an important discussion, IMO...
>
>
> On 07/31/2014 11:54 AM, Arnaud Legendre wrote:
>>
>> Hi Jay,
>>
>> I would be interested if you could share your point of view on this
>> item: we want to make the glance stores a standalone library
>> (glance.stores) which would be consumed directly by Nova and Cinder.
>
>
> Yes, I have been enthusiastic about this effort for a long time now :) In
> fact, I have been pushing a series of patches (most merged at this point) in
> Nova to clean up the (very) messy nova.image.glance module and standardize
> the image API in Nova.
>
> The messiest part of the current image API in Nova, by far, is the
> nova.image.glance.GlanceImageService.download() method, which you highlight
> below. The reason it is so messy is that the method does different things
> (and returns different things!) depending on how you call it and what
> arguments you provide. :(
>
>
>> I think it would be nice to get your pov since you worked a lot on
>> the Nova image interface recently. To give you an example:
>>
>> Here
>> https://github.com/openstack/nova/blob/master/nova/image/glance.py#L333,
>>  we would do:
>>
>> 1. location = get_image_location(image_id),
>> 2. get(location) from the
>> glance.stores library like for example rbd
>> (https://github.com/openstack/glance/blob/master/glance/store/rbd.py#L206)
>
>
> Yup. Though I'd love for this code to live in olso, not glance...
>
> Plus, I'd almost prefer to see an interface that hides the location URIs
> entirely and makes the discovery of those location URIs entirely
> encapsulated within glance.store. So, for instance, instead of getting the
> image location using a call to glanceclient.show(), parsing the locations
> collection from the v2 API response, and passing that URI to the
> glance.store.get() function, I'd prefer to see an interface more like this:
>
> ```python
> # This code would go in a new nova.image.API.copy() method:
> import io
>
> from oslo.image import move
> from oslo.image.move import exception as mexc
>
> from nova import exception as exc
>
> ...
>     def copy(image_id_or_uri, stream_writer):
>         try:
>             config = {
>                # Some Nova CONF options...
>             }
>             mover = move.Mover(image_id_or_uri, config)
>             success, bytes_written = mover.copy(stream_writer)
>             if success:
>                 if bytes_written == 0:
>                     LOG.info("Copied image %s using zero-copy "
>                              "transfer.", image_id_or_uri)
>                 else:
>                     LOG.info("Copied image %s using standard "
>                              "filesystem copy. Copied %d bytes.",
>                              image_id_or_uri, bytes_written)
>             return success
>         except mexc.ImageNotFound:
>             raise exc.NotFound(...)
>         except mexc.ImageInvalidApi:
>             # Fall back to pull image from Glance
>             # API server via HTTP and write to disk
>             # via the stream_writer argument's write()
>             # interface... and return True or False
>             # depending on whether write()s succeeded
> ```
>

This idea looks more neat, but I'm a little worries on implementation
since most CoW based zero-copy and smart full-copy approaches need
leverage the capability from particular storage (e.g. ceph) and/or
hypervisor (e.g. vsphere), so IMHO we almost couldn't to make a
consistent logic of zero-copy or smart full-copy/transferring and
encapsulate them into glance.store (or oslo.image) that separated from
special hypervisor and storage context, only if we do necessary
function that hypervisor and storage needed in the lib internal.

> And then, the caller of such an nova.image.API.copy() function would be in
> the existing various virt utils and imagebackends, and would call the API
> function like so:
>
> ```python
> # This code would go in something like nova.virt.libvirt.utils:
>
> from nova import image
>
> IMAGE_API = image.API()
>
> write_file = io.FileIO(dst_path, mode='wb')
> writer = io.BufferedWriter(write_file)
>
> image_id_or_uri = "https://images.example.com/images/123"
>
> result = IMAGE_API.copy(image_id_or_uri, writer)
> # Test result if needed...
> ```
>
> Notice that the caller never needs to know about the locations collection of
> the image -- and thus we correct the leaked implementation details that
> currently ooze out of the download() method in
> nova.image.glance.GlanceImageService.download.
>
> Also note that we no longer pass a variety of file descriptors, file
> writers, file destination paths to the download method. Instead, we always
> just pass the image ID or URI and a writeable bytestream iterator. And we
> always return either True or False, instead of None or a file iterator
> depending on the supplied arguments to download().
>
>
>>  The same kind of logic could be added in Cinder.
>
>
> Sure.
>
>
>> We see that as a benefit for Nova, which would be able to directly
>> consume the stores instead of going through the glance api.
>
>
> Exactly.
>
>
>> We had a vote today to figure out if we continue the effort on the
>> glance.stores library. We had a majority of +1 but there was a couple
>> of -1 due to the fact that we don’t have enough concrete examples of
>> this will be useful or not.
>
>
> It will definitely be useful in the following:
>
> 1) Making the copy/zero-copy/transfer/download methods consistent between
> all the various places in the Nova virt drivers that do similar things.
>
> 2) Allowing a single place to innovate for the transfer of image bits
> between sources and destinations
>

I agreed zero-copy and smart full-copy/transferring functions are
worth cases for us, but IMHO, at least from current basic
understanding, they seems don't like a responsibility of glance.store
lib, currently the lib just provides the basic IO function set for
those necessary operations backend storage direct related on image
bits (including add, delete, get_size, g/set_acls), so even I'm sure
we can add more interfaces to glance.store to support more innovatory
functions in future, I'm afraid if it's a good place to place those
high level functions instead of a separated (new or existing)
service/lib.

Thanks,
zhiyan

> Hopefully, the above sample code and interfaces will spark some renewed
> interest in this. I'd love to work on it, and see it pushed forward.
>
> Best,
> -jay
>
>> Please also see this etherpad containing a list of
>> advantages/drawbacks: https://etherpad.openstack.org/p/glance-store
>>
>> Looking forward to get your pov.
>>
>> Thanks, Arnaud
>>
>>
>>
>> On Jul 28, 2014, at 1:23 PM, Flavio Percoco <flavio at redhat.com
>> <mailto:flavio at redhat.com>> wrote:
>>
>>> On 07/28/2014 09:12 PM, Arnaud Legendre wrote:
>>>>
>>>> Flavio — yeah, glance.store lost momentum at the meetup… I think
>>>> it would be nice if you could recap the advantages of doing
>>>> that. At the meetup, we quickly did an "advantages vs drawbacks”
>>>> poll and I think we might have missed advantages (since you were
>>>> not here :)) so there was more drawbacks.
>>>
>>>
>>> Was this published somewhere?
>>>
>>>> Also, the gap between glance.store and glance will continuously
>>>> increase if we do not define some rules to keep both in sync
>>>> (while we are making glance.store ready). If we choose to use
>>>> glance.store, I think we have to switch glance to use it asap.
>>>
>>>
>>> It's not difficult to keep the gap close if we keep enough eyes on
>>> reviews. We've been doing this for oslo.messaging and the old
>>> oslo-rpc code.
>>>
>>>
>>>>
>>>> This has been added to the agenda for the irc meeting next
>>>> Thursday, that would be awesome if you could attend...
>>>
>>>
>>> I'll try to make it there but if I can't, I'd really appreciate
>>> this discussion to be brought to the mailing list. I'd really like
>>> to understand why the feeling changed and what are the bases for
>>> this new feeling about the library.
>>>
>>> Pls, don't get me wrong. I'm just really surprised and a bit
>>> frustrated but I do want to understand what the current feeling
>>> about it is and hopefully convince the team otherwise.
>>>
>>> Cheers, Flavio
>>>
>>>>
>>>> -Arnaud
>>>>
>>>>
>>>> On Jul 28, 2014, at 9:44 AM, Flavio Percoco <flavio at redhat.com
>>>> <mailto:flavio at redhat.com>> wrote:
>>>>
>>>>> Hey Guys,
>>>>>
>>>>> I was talking to Nikhil earlier today and he mentioned there
>>>>> was a mini discussion about glance.store in the mini-summit.
>>>>> One thing he mentioned is that some folks don't want it to be
>>>>> merged/exists.
>>>>>
>>>>> I'm writing to get your feeling about this and understand what
>>>>> the concerns are. After 1 1/2 development cycle working on the
>>>>> library, based on our previous agreement, I'm sure you guys
>>>>> understand my frustration after hearing this.
>>>>>
>>>>> Mark, I understand you are going on vacation in about 2 weeks,
>>>>> I'd really like to clear this before you leave so I can move
>>>>> this work forward and don't miss the feature freeze.
>>>>>
>>>>> Thanks in advance for any feedback, Flavio
>>>>>
>>>>> P.S: If you guys prefer, we can have a meeting about me. FWIW,
>>>>> I've been trying to contact most of you on IRC but it's been
>>>>> really hard because of the timezones.
>>>>>
>>>>> -- @flaper87 Flavio Percoco
>>>>
>>>>
>>>
>>>
>>> -- @flaper87 Flavio Percoco
>>
>>
>