14:00:09 <pdeore> #startmeeting glance
14:00:09 <opendevmeet> Meeting started Thu Jul 21 14:00:09 2022 UTC and is due to finish in 60 minutes.  The chair is pdeore. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:09 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:09 <opendevmeet> The meeting name has been set to 'glance'
14:00:09 <pdeore> #topic roll call
14:00:09 <pdeore> #link https://etherpad.openstack.org/p/glance-team-meeting-agenda
14:00:21 <pdeore> o/
14:00:35 <abhishekk> o/
14:00:48 <alistarle> o/
14:00:57 * abhishekk will be here for short time only
14:01:14 <abhishekk> jokke_, is on leave
14:01:17 <croelandt> o/
14:01:22 <pdeore> we have short agenda for today
14:01:29 <dansmith> o/
14:01:30 <mrjoshi> o/
14:01:47 <pdeore> let's start
14:01:48 <abhishekk> Lets start
14:01:55 <pdeore> #topic release/periodic jobs
14:02:01 <pdeore> We have tagged M2 last week
14:02:09 <pdeore> and M3 is 6 weeks from now...
14:02:29 <pdeore> Periodic Jobs, all green except the 'translation update' jobs failing with RETRY_LIMIT
14:02:55 <abhishekk> For M3 we have 2 major patches to be merged
14:03:06 <abhishekk> 1 Glance download import method
14:03:20 <abhishekk> 2 Extend store details information for all stores
14:03:37 <abhishekk> both patches are up for reviews
14:03:49 <pdeore> yes
14:04:10 <abhishekk> Unfortunately we did not here anything from tobias regarding injecting metadata during upload process
14:05:10 <abhishekk> so probably we are going to skip it in this cycle
14:05:19 * dansmith nods
14:06:07 <abhishekk> lets move ahead
14:06:13 <pdeore> yeah
14:06:14 <pdeore> #topic Bugs
14:06:20 <pdeore> #link https://bugs.launchpad.net/python-glanceclient/+bug/1597623
14:06:23 <croelandt> that was me :)
14:06:34 <croelandt> I think we've stated multiple times that we only want to use image IDs in commands
14:06:38 <croelandt> and not image names
14:06:45 <croelandt> am I mistaken or can we"close" this bug?
14:07:02 <abhishekk> We can mark it as won't fix
14:07:12 <croelandt> great, thanks for confirming that :)
14:07:25 <dansmith> I really hate that behavior, fwiw :)
14:07:33 <abhishekk> ++
14:07:40 <dansmith> and glance is the outlier here, AFAICT
14:07:59 <croelandt> it's kind of more user-friendly I guess
14:08:02 <dansmith> but basically, I have to use my mouse to interact with glanceclient, which is infuriating
14:08:06 <croelandt> in bug reports sometimes I wish we had names rather than UUIDs
14:08:56 <dansmith> it also makes writing docs more confusing because we'll show a list of images, and then a bunch of commands with UUIDs and the reader has to do their own correlation to know what we're doing
14:09:03 <croelandt> dansmith: I would not be against it, but I'm not willing to do it myself, and it's not a hill I want to die on
14:09:39 <dansmith> yeah, I would do it, but I also feel like it's going to be some giant fight for basic usability, which is hard to get excited about
14:09:56 <dansmith> but literally I can interact with the whole rest of the stack with names, except for glance
14:10:02 <croelandt> yeah and it's the kind of patch that's gonna need multiple rounds
14:10:10 <croelandt> because someone is not gonna be happy about the tests etc.
14:10:15 <croelandt> and it's going to drag on for 3 years
14:10:22 <abhishekk> :D
14:10:46 <dansmith> well, I don't think it's about the tests or implementation, but the principle I suspect :)
14:10:52 <croelandt> yeah also that
14:11:06 <croelandt> I mean the implementation is interesting as well
14:11:14 <croelandt> there's some potential for errors if people name multple images the same
14:11:17 <croelandt> etc.
14:11:27 <dansmith> it's the same for all other resources though
14:11:37 <croelandt> maybe something to revisit in the future once we're tired of copy/pasting uuids with the damn mouse
14:11:38 <abhishekk> mostly principle, I remember there is lots of negative opinions from glance team at that time
14:11:42 <dansmith> names are not unique constrants in many places
14:11:43 <croelandt> jsut wait for our wrists to be broken
14:12:06 <pdeore> :D :D
14:12:13 <croelandt> abhishekk: we **are** the Glance team :D
14:12:44 <abhishekk> croelandt, yeah, but it was discussed during Hawana cycle as well
14:12:58 <croelandt> let'sd discuss it again at the next H cycle
14:13:00 <abhishekk> I remember I had a PoC ready for this at that time :D
14:13:09 <dansmith> maybe we could get an operator survey question about it
14:13:28 <abhishekk> Sounds good
14:13:28 <croelandt> dansmith: +1
14:13:46 <pdeore> ++
14:14:33 <pdeore> move to next ? :)
14:14:54 <abhishekk> Lets take help of rosmaita for drafting operators question
14:15:18 <abhishekk> yes, lets move ahead
14:15:29 <pdeore> #topic Glance download import work
14:15:56 <abhishekk> alistarle, is back now
14:16:14 <pdeore> yeah, and there are some updates on the patch , https://review.opendev.org/c/openstack/glance/+/840318/23..24
14:16:22 <abhishekk> alistarle, do you have any questions related to review which you want to discuss?
14:17:35 <alistarle> hmmm yes, I see dan comment
14:17:59 <alistarle> For the URL check, do you think a cast to UUID check is enough ?
14:18:26 <dansmith> alistarle: probably, or a quick check that they're just hex characters
14:18:33 <dansmith> for the image_id you mean
14:18:37 <alistarle> yep
14:18:44 <dansmith> I still think we should re-use that scheme checker on the url we construct
14:18:49 <dansmith> which should be easy I think
14:19:42 <dansmith> if glance image ids are always uuids then cast to uuid seems okay
14:20:40 <dansmith> I can do that work if you want, but I think it's easy.. a couple lines in each location at most
14:20:47 <alistarle> I agree with the use of scheme checker, but we need to move it to the task execution because we don't have the URL yet, so it not fail fast as the 400 we return during a web_download
14:20:54 <dansmith> right for sure
14:21:20 <dansmith> apologize that my "add glance-download here" meant physically "here" but I meant "add this check for glance-download too"
14:21:45 <alistarle> ok so no problem, I will do and update the doc accordingly
14:22:31 <dansmith> we should have some tests for these things too, so let me know if I need to write those
14:22:46 <dansmith> but hopefully my test module got you started enough to copy for other scenarios
14:22:53 <alistarle> yep sure
14:23:34 <alistarle> and regarding the content-length stuff, so the header check isn't enough like for the web-download ?
14:23:55 <alistarle> (I will check if glance return it btw)
14:24:07 <dansmith> I don't think glance sets it AFAIK
14:24:46 <dansmith> if it does, then perhaps that's okay, but I think wrapping glance with apache that does gzip offloading will eliminate it, since you can't pre-calculate the compressed size for many gigs of data
14:25:33 <alistarle> Hmmm ok, but checking the image size glance side will require an additional call to remote glance API
14:25:56 <abhishekk> https://docs.openstack.org/api-ref/image/v2/?expanded=download-binary-image-data-detail#response
14:25:58 <alistarle> And are we sure glance will provide us the right size ?
14:26:03 <dansmith> yeah, which is what we're trying to do here.. provide a glance-aware download mechanism :)
14:27:12 <dansmith> abhishekk: I don't see where that is set in the code, but if apache is doing gzip for us, I think it has to eliminate that
14:27:28 <abhishekk> ack
14:27:39 <abhishekk> will look at it
14:28:27 <dansmith> also this, if we do our own gzip in middleware: https://github.com/openstack/glance/blob/bfcab95ff2d7e13afe05fed8835159949490afd8/glance/api/middleware/gzip.py#L47-L50
14:29:22 <abhishekk> right
14:30:35 <dansmith> alistarle: maybe you can implement this in the metadata copy part, where you compare what our glance and the remote glance think is the size,
14:30:55 <dansmith> but that means you'll lose that check if you don't have the metadata copy module enabled, which is probably not what people would expect
14:32:02 <alistarle> issue is we copy metadata before we download the image, so we can't do the check
14:32:14 <dansmith> oh right, heh :)
14:33:03 <dansmith> well, then we probably need to just do the extra call then
14:33:23 <dansmith> it would be so easy for long-running downloads to terminate mid-stream, we claim "okay your image is good to go, metadata and all" and have it be corrupted
14:33:25 <abhishekk> can we pass the size to following task as an input?
14:33:51 <alistarle> or we can manually update the image size during the metadata copy, but maybe it is a bad idea to deal with that
14:34:04 <dansmith> abhishekk: we could set it as os_glance_remote_size and then remove that when we've checked
14:34:15 <dansmith> but honestly, I don't think the extra call is that bad
14:34:21 <abhishekk> ok
14:34:28 <dansmith> alistarle: yeah bad idea I think
14:35:29 <alistarle> abhishekk: I don't know if we can easily pass param between task, but if yes maybe giving the size as an input is good
14:35:59 <abhishekk> alistarle, afaik, we can pass param between tasks
14:36:19 <dansmith> but if metadata is disabled then we won't have a size to check that way
14:36:33 <abhishekk> yeah
14:36:34 <dansmith> if we put an explicit call to remote in dowload, then that continues to work if you've disabled the metadata plugin
14:36:52 <abhishekk> disabled means there is no item in that config option, right?
14:37:16 <alistarle> nope, it is forcefully added to the flow
14:37:22 <dansmith> no I think disabled means you don't list the metadata plugin in the plugins.. wasn't that one of the things desired? to be able to disable that separately?
14:37:28 <dansmith> oh okay, then nevermind
14:37:46 <alistarle> we don't wanted the make it configurable for inconsistency reason
14:38:25 <alistarle> you can choose to copy nothing, but actually the plugin will run (at least in the current patch set)
14:38:38 <alistarle> to update at least the container_format and disk_format
14:38:48 <dansmith> it's so weird to me that we have some of the flow steps in api_image_import, but the body of the implementation in the plugin file
14:38:52 <dansmith> but yeah I see now
14:38:59 <abhishekk> https://review.opendev.org/c/openstack/glance/+/840318/24/glance/async_/flows/api_image_import.py#85
14:40:20 <alistarle> and should we rely more on the size or virtual_size attribute ? I remember there is two field in the API
14:40:38 <abhishekk> alistarle, if it is not possible to pass the param as an input then as dan suggested set reserved property to image os_glance_remote_size and then remove it after checking
14:41:22 <abhishekk> I think size is what we should verify
14:41:31 <dansmith> I really don't like setting it on the image, so if there's a better way we should do that, but if not, that should be an option
14:41:38 <dansmith> yes, size
14:42:02 <dansmith> virtual_size could be correct if we read the first few kilobytes of a qcow2 file, but not have any more of it
14:42:20 <abhishekk> ack
14:43:02 <alistarle> ok let's do that, I will rework the patch accordingly
14:43:41 <dansmith> alistarle: please speak up if you get stuck so we can help.. we don't want this to get delayed any longer than necessary
14:44:01 <abhishekk> ++
14:44:20 <alistarle> so to sum up: Will handle the content-length based on the size attribute of the remote glance, I need to revert the metadata to the original value, and I need to check the target url against the scheme validator
14:44:31 <alistarle> seems ok to me
14:44:39 <abhishekk> perfect
14:44:43 <dansmith> alistarle: and sanitize the image_id
14:44:50 <alistarle> abhishekk: I will check with you if I struggle to forward param between tasks
14:44:59 <abhishekk> alistarle, ack
14:45:03 <dansmith> alistarle: to be clear, you need to compare size==size, not size==content_length, because the latter will be different with gzip
14:45:18 <alistarle> sure
14:47:21 <dansmith> all good to move on?
14:47:45 <alistarle> yep
14:47:55 <alistarle> sorry for the long time we spend on that topic ;)
14:48:12 <dansmith> not a problem IMHO, glad we're progressing
14:48:19 <abhishekk> agree
14:49:03 <pdeore> moving to Open discussion...
14:49:05 <pdeore> #topic Open Discussion
14:49:42 <abhishekk> alistarle, https://docs.openstack.org/taskflow/latest/user/inputs_and_outputs.html hopefully this will help
14:50:13 <abhishekk> nothing from me for Open discussion
14:50:28 <dansmith> abhishekk: ah nice
14:51:06 <abhishekk> o/
14:51:46 <pdeore> so should we wrap up ?
14:51:50 <dansmith> ++
14:51:59 <abhishekk> yep
14:52:03 <abhishekk> thank you all
14:52:13 <pdeore> gr8!! Thanks evryone for joining !!
14:52:29 <pdeore> #endmeeting