14:00:39 <mrjoshi> #startmeeting glance
14:00:39 <opendevmeet> Meeting started Thu Jun 13 14:00:39 2024 UTC and is due to finish in 60 minutes.  The chair is mrjoshi. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:39 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:39 <opendevmeet> The meeting name has been set to 'glance'
14:00:39 <mrjoshi> #topic roll call
14:00:39 <mrjoshi> #link https://etherpad.openstack.org/p/glance-team-meeting-agenda
14:00:49 <mrjoshi> o/
14:01:10 <rubasov> o/
14:09:32 <dansmith> o/
14:09:37 <stephenfin> o/
14:10:06 <mrjoshi> abhishek and pranali are not around today
14:10:26 <mrjoshi> shall we start?
14:12:34 * abhishekk joining from mobile, might be late in replying
14:12:39 <mrjoshi> let's start
14:12:52 <mrjoshi> #topic release/periodic jobs updates
14:13:18 <mrjoshi> M2 is 3 weeks from now
14:13:34 <mrjoshi> periodic jobs are all green
14:13:52 <abhishekk> ack
14:14:03 <mrjoshi> #topic Important Review
14:14:58 <mrjoshi> Spec - deprecate metadata-encryption-key #link- https://review.opendev.org/c/openstack/glance-specs/+/916178
14:15:21 <mrjoshi> Spec - Revised spec for Image Encryption #link - https://review.opendev.org/c/openstack/glance-specs/+/915726
14:15:37 <abhishekk> I think pranali and you should provide reviews on encryption spec on priority
14:15:45 <mrjoshi> New location APIs #link - https://review.opendev.org/q/topic:%22New-Location-Apis%22+project:openstack/glance
14:16:10 <mrjoshi> Fix 500 if multi-tenant swift is enabled along with conf file #link- https://review.opendev.org/c/openstack/glance/+/920170
14:16:12 <dansmith> so what's the status on the location api work? is it _actually_ ready for review now?
14:16:38 <mrjoshi> abhishekk, ack
14:17:41 <abhishekk> dansmith: last time I checked, it is ready IMO
14:18:04 <dansmith> abhishekk: okay doesn't look like it has +1 or +2 on all the patches
14:18:25 <dansmith> every time I've gone through it I've found really obviously wrong stuff so I don't want to waste my time if it's not really ready
14:18:32 <dansmith> but if you say it is I'll take another pass through it
14:18:46 <abhishekk> I haven't reviewed it since busy with other stuff but I will have a look at those
14:19:05 <abhishekk> may be croeland1 will also help us here ^^
14:19:19 <dansmith> okay maybe I'll hold off a week as I too have other things in the fire now
14:19:30 <abhishekk> ack, makes sense
14:19:41 <dansmith> yeah that'd be good if croeland1 can could take a pass
14:20:08 <abhishekk> I will request him offline, he might be not around today
14:20:17 <dansmith> cool, thanks
14:20:28 <abhishekk> np!, lets move ahead
14:20:38 <mrjoshi> moving ahead
14:20:53 <mrjoshi> #topic Open Discussions
14:22:16 <stephenfin> Guess we're up. mbooth is out sick 🤒 today so I'm going to cover for him
14:22:28 <abhishekk> ohh
14:23:46 <abhishekk> you can take over
14:24:06 <stephenfin> Per the agenda doc, we're working on openshift-installer and would like to take advantage of web-download to store the boot images for k8s nodes in Glance, avoiding proxying everything through localhost
14:25:05 <stephenfin> The image on the remote host is a qcow2.gz file, and we would expect to be able to use rely on the image_decompression plugin to decompress this before saving it in the store(s)
14:25:40 <abhishekk> we have import method discovery call which we can extend to provide required information
14:25:55 <abhishekk> https://docs.openstack.org/api-ref/image/v2/#import-methods-and-values-discovery
14:25:58 <stephenfin> Yup, that's what we need. It need sto be discoverable
14:26:04 <stephenfin> *It needs to
14:26:29 <stephenfin> As things stand, we can identify the supported import methods, but not the supported plugins
14:26:43 <abhishekk> I think as this is admin only api we can add --include-plugins option there to include available plugins
14:27:19 <dansmith> not all the plugins are really relevant I think (like conversion) but knowing if decompression is available is obviously pretty important to expose
14:27:58 <stephenfin> Oh, is '/v2/info/import' admin-only?
14:28:21 <abhishekk> stephenfin: I think so
14:28:31 <dansmith> is it? that seems, odd
14:28:35 <stephenfin> How does a user know they can import an image using e.g. web-download without access to that API? Try and wait for a failure?
14:28:39 <dansmith> users need to know if web-download is available, for example
14:28:47 <stephenfin> ^ this
14:29:08 <abhishekk> can't verify right now, as away from machine
14:29:30 <dansmith> so,
14:29:33 <dansmith> the only thing I can think of,
14:29:34 <stephenfin> nah, 'OS_CLOUD=devstack openstack image import info' gives me results back
14:29:52 <stephenfin> (running against a normal devstack deploy)
14:29:54 <stephenfin> so I think we're good
14:30:00 <abhishekk> ack
14:30:07 <dansmith> is that the import stuff is sort of intertwined with the old tasks stuff, which was admin-only, so there may be some cases there where some or all of the details are hidden to normal users
14:30:26 <dansmith> but the discovery endpoint should be pretty open I expect
14:30:32 <abhishekk> hmm
14:30:51 <stephenfin> yes, I will check in more detail but the quick reproducer suggests this is open by default
14:30:56 <abhishekk> So we need a spec lite for this change and then we can make it quick to implement
14:31:17 <stephenfin> sweet. I am happy to implement this if it would
14:31:20 <abhishekk> yes, it is open, checked policy file and found no policy rule related to it
14:31:21 <stephenfin> help
14:31:33 <abhishekk> ++
14:31:33 <dansmith> so, do you think any operators will be opposed to exposing, for example that image conversion or meta injection is enabled?
14:31:40 <dansmith> those are sort of operator policy decisions,
14:31:53 <dansmith> but decompression is more like "fyi, this is available"
14:32:09 <dansmith> not like they won't see the end result of the metadata/conversion once it's done of course...
14:32:19 <abhishekk> Not sure about conversion, but inject metadata should not be exposed
14:32:31 <stephenfin> I'm happy to treat decompression as special. We don't care about the other things since (afaict) that doesn't affect the user
14:33:01 <dansmith> so we could either (a) have an exclude list of plugins to expose, or (b) just have a special flag for "is decompression enabled"
14:33:18 <abhishekk> better to go with b
14:33:23 <dansmith> (or just make decompression compulsory, as I kinda expect there's not much reason to not support that, if you're doing other transformation)
14:34:02 <abhishekk> if we make it compulsory then we need to tweak a code to not include it in taskflow if image provided is not decompressed?
14:34:10 <stephenfin> making it compulsory seems even better, otherwise imo the import should fail if the image is compressed and the plugin doesn't exist
14:34:10 <dansmith> could also make it a list of supported decompression routines, so people know, and then it can be empty if disabled and we can add to it later if we support like bzip2 or something
14:34:51 <dansmith> "The supported archive types for Image Decompression are zip, lha/lzh and gzip"
14:34:59 <stephenfin> makes sense
14:35:08 <stephenfin> so  it sounds like there's general acceptance that this is a valid feature and I should go write a small spec?
14:35:11 <dansmith> that ^ is tied to a specific release and a list of those in discovery would be more useful
14:35:17 <dansmith> yes
14:35:30 <stephenfin> spot on, I can start on that so
14:35:40 <stephenfin> it won't help us right now, but it will down the line
14:35:45 <abhishekk> ++, thank you for taking it up
14:35:55 <stephenfin> np
14:36:03 <stephenfin> we have one other things but I think rubasov is up next
14:36:21 <rubasov> hi everyone, I hope to ask for a bit of review attention
14:36:37 <rubasov> I have a wip bugfix with two open questions
14:36:50 <abhishekk> I have answered one I think
14:36:54 <abhishekk> on the review
14:37:17 <rubasov> thanks already!
14:37:29 <dansmith> link?
14:37:31 <abhishekk> just one suggestion, the bug has two many patches attached, I would suggest to abandon those which are not requird
14:37:35 <rubasov> https://review.opendev.org/c/openstack/glance_store/+/915711
14:37:52 <abhishekk> #link https://bugs.launchpad.net/glance-store/+bug/1965679
14:38:04 <rubasov> the old fix was proposed by someone else so I cannot abandon that
14:38:49 <abhishekk> Ok, I will ask PTL to abandon that
14:39:11 <abhishekk> So you have 3 active patches out of the current you linked here is WIP
14:39:40 <rubasov> the first two in the series are not too important refactors
14:39:54 <rubasov> the 3rd wip patch if the proposed bugfix
14:40:35 <rubasov> and that's where I was a bit lost with my questions, the remaining question is: is there a way to detect from glance if we have multiple glance processe started by wsgi?
14:40:57 <rubasov> (because the bug itself only occurs when we have multiple glance processes)
14:41:23 * abhishekk need to check for uwsgi
14:41:57 <rubasov> it's not urgent in any way, but if you could add a review comment about it I'd really appreciate it
14:42:01 <dansmith> is the problem when running glance standalone/eventlet mode or under wsgi with real threads? I assumed the former.
14:42:18 <rubasov> and then I could turn the patch into something properly reviewable (not wip)
14:42:47 <rubasov> I believe in both mode we can have multiple processes
14:43:03 <rubasov> so I think I need to cover both modes
14:43:20 <dansmith> there's no *need* for multiple processes in wsgi/real-thread mode but sure, okay
14:43:51 <dansmith> is the problem just that you need to use a lock(external=True) so that all the threads respect the lock around some cinder setup?
14:44:07 <rubasov> basically yes
14:44:57 <dansmith> https://docs.openstack.org/oslo.concurrency/ocata/api/lockutils.html#oslo_concurrency.lockutils.synchronized
14:45:01 <dansmith> external=True
14:45:51 <dansmith> oh you're already using external_lock in your patch
14:46:08 <rubasov> yes, but we also need to keep counting from how many places we need to keep the volume attached
14:46:22 <dansmith> oh you need a refcount
14:47:24 <rubasov> that's why the current patch has a lockfile (to lock the acces to the state file) and a state file, keeping track of all volume uses
14:47:33 <dansmith> not really any way to do that across the processes without either doing it in the database or with posix ipc sort of stuff
14:47:51 <dansmith> ack, I haven't reviewed the patch, just skimmed while we're talking here
14:49:09 <rubasov> I hope the idea is workable and clear from the current state, but because of the open questions could not make it non-wip
14:49:47 <dansmith> ack, I need to go grok the patch probably
14:49:57 <rubasov> so if you could look at a bit despite it being wip, I'd appreciate that and could continue with it
14:50:34 <dansmith> I see the state file stuff, I'd probably rather not do it that way but we can discuss more on the patch
14:51:03 <rubasov> and that's all from me unless you have other questions about it
14:51:13 <rubasov> I'm open to all suggestions and directions
14:52:13 <rubasov> thanks in advance for your review!
14:52:18 <abhishekk> thanks rubasov, stephenfin I think you can continue now
14:52:48 <stephenfin> sweet
14:53:04 <stephenfin> so our other issue is hopefully self-explanatory
14:53:53 <stephenfin> we have images and we have md5, sha1, and sha256 hashes available from the image provider, but glance will only gives us a sha512 hash
14:54:14 <stephenfin> we'd like to be able to ask glance to gives us hashes in (a limited set of) other formats
14:55:22 <abhishekk> we have hashing_algo config option
14:55:55 <stephenfin> right, but that's not user-configurable so we can't use it on public clouds or clouds where we're just one of many tenants
14:56:26 <abhishekk> So you want to override the default by providing it while creating the image?
14:56:43 <dansmith> I think we'd need to carefully consider that.. seems like a bad thing to give users control over
14:57:30 <dansmith> AFAIK, the "these are the hashes we maintain on images" is sort of a site-wide policy/security decision, not to mention a CPU usage consideration
14:58:12 <stephenfin> That's mbooth's suggestion, yes. We could also store multiple hashes and allow the user to select which one they see, but there's a CPU consumption question there of course
14:58:28 <dansmith> i.e. asking for "no hashes" or only weak ones is sort of an attack vector
14:58:38 <dansmith> we *do* store multiple hashes right?
14:58:53 <dansmith> I mean, glance does/can
14:59:49 <dansmith> okay maybe not multiple simultaneously
15:00:03 <abhishekk> I haven't seen it though
15:00:11 <dansmith> mrjoshi: I have to run to another meeting now, maybe we can continue this topic next week
15:00:19 <abhishekk> ++
15:00:34 <mrjoshi> dansmith, ack
15:00:41 <abhishekk> we are out of time anyway
15:00:55 <abhishekk> thanks stephenfin and rubasov for joining
15:01:05 <rubasov> thanks everyone!
15:01:10 <mrjoshi> let's wrap up then!
15:01:11 <stephenfin> okay, I can continue the discussion on #openstack-glance later. Thanks for your time!
15:01:27 <mrjoshi> thanks everyone for joining!
15:01:49 <mrjoshi> #endmeeting