14:00:20 <croelandt> #startmeeting glance
14:00:20 <opendevmeet> Meeting started Thu Apr 24 14:00:20 2025 UTC and is due to finish in 60 minutes.  The chair is croelandt. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:20 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:20 <opendevmeet> The meeting name has been set to 'glance'
14:00:30 <croelandt> #topic roll call
14:00:36 <croelandt> #link https://etherpad.openstack.org/p/glance-team-meeting-agenda
14:00:53 <abhishekk_> Hi
14:01:27 <mhen> o/
14:01:37 <rosmaita> o/
14:01:42 <croelandt> let's start
14:01:49 <croelandt> I'll be chairing since mrjoshi is feeling unwell
14:02:08 <croelandt> #topic Release/periodic job updates
14:02:16 <abhishekk_> Ok
14:02:20 <croelandt> so glance.tests.functional.v2.test_images_import_locking.TestImageImportLocking.test_import_copy_bust_lock has been failing  for a long time
14:02:26 <croelandt> both in periodic jobs and on reviews
14:02:33 <croelandt> I cannot reproduce this failure on my machine
14:02:48 <croelandt> If anyone can, it would be helpful to figure out what's happening and maybe bisect the issue
14:03:04 <croelandt> otherwise, I'm not sure what to do. Do we want to "temporarily" disable the test?
14:04:06 <rosmaita> is it only failing on py310 ?
14:04:14 <abhishekk_> I don’t think disabling the test is a right option
14:04:42 <croelandt> no, it's failing on multiple versions of Python
14:04:46 <croelandt> but not always :)
14:05:04 <croelandt> abhishekk_: so how do we debug that?
14:05:21 <croelandt> I've tried running the failing test in a loop, and it *never* fails locally :/
14:05:42 <abhishekk_> For time being
14:06:33 <abhishekk_> We can add tri catch and if it fails
14:06:38 <rosmaita> do we randomize the test ordering?
14:07:01 <croelandt> abhishekk_: and silently ignore the failure?
14:07:08 <abhishekk_> I think ao
14:07:10 <croelandt> rosmaita: hm, how do we check that?
14:07:19 <croelandt> it would be tox/stestr doing that, I guess?
14:07:56 <rosmaita> yes, there
14:08:21 <croelandt> hm do you know what config option turns the randomization on/off?
14:08:21 <rosmaita> would be in tox.ini
14:08:23 <abhishekk_> Is it possible to run that test in the end?
14:09:47 <rosmaita> looks like we do not randomize
14:10:01 <croelandt> rosmaita: is it a good thing or a bad one? :)
14:10:15 <rosmaita> though, to an extent you can get different ordering based on number of cpus available, i think
14:10:38 <croelandt> yeah
14:10:42 <croelandt> so "action items" :)
14:10:52 <croelandt> abhishekk_: you'd want to try/except the failure?
14:11:12 <rosmaita> well, it's good for this question, because it means that the failure isn't due to some test not cleaning up and then causing a problem
14:11:44 <rosmaita> so probably the issue here isn't insufficient test isolation causing an issue
14:11:57 <abhishekk_> I need to check for that, I remember i had done it earlier for one test
14:12:12 <croelandt> ok so maybe we force it to be first or last
14:12:15 <croelandt> see if it keeps failing
14:12:29 <abhishekk_> It should be last imo
14:12:58 <croelandt> I see "stestr run" has a "--load-list" option
14:13:13 <croelandt> maybe listing all tests in there and rearranging them can do the trick
14:13:50 <rosmaita> well, i think run stestr twice: first time, everything *except* the bad test, second time *only* the bad test
14:14:00 <croelandt> oh I see
14:14:27 <rosmaita> i think there's a "combine" option to stestr that you give the second one
14:14:40 <croelandt> ok I'll send a DNM patch that does that
14:14:57 <croelandt> yeah we can do "--exclude" for the first run, pass the test name for the second run, and combine
14:15:13 <croelandt> ok good, let's move on
14:15:22 <croelandt> #topic Important Zuul patches
14:15:34 <croelandt> #link https://review.opendev.org/c/openstack/glance/+/933614
14:15:44 <croelandt> so this is an old patch of mine removing standalone deployments
14:16:01 <croelandt> I don't remember the details to be honest :) But iirc devstack is no longer really using that, and without eventlet we'll drop that too
14:16:04 * dansmith stumbles in late
14:16:13 <croelandt> does it make sense to remove standalone deployments from our test runs?
14:17:25 <dansmith> seems fine to me to remove standalone if we're trying to ... remove that ability :)
14:17:57 <croelandt> yeah and I think that's in our plans
14:18:16 <dansmith> croelandt: if you can link me to an example of the bust lock failure I can have a quick look at least
14:18:33 <croelandt> oh it has *not* been merged indevstack
14:18:36 <croelandt> https://review.opendev.org/c/openstack/devstack/+/932201
14:18:42 <rosmaita> https://56667f4336a8ddb79330-f6a340cbf02833d06cf13bf014ff46d4.ssl.cf1.rackcdn.com/openstack/e9a8cede349346b1a792b4a7af6ac4f7/testr_results.html
14:18:46 <croelandt> rosmaita: thanks
14:18:48 <rosmaita> dansmith: ^^
14:18:51 <dansmith> thanks
14:18:57 <croelandt> so ok, maybe I'll check with stephenfin first
14:19:01 <croelandt> abhishekk_: what do you think?
14:19:04 <dansmith> croelandt: but the goal is to remove eventlet, which means removing standalone
14:19:15 <rosmaita> i've looked at a few, seems to be the KeyError: 'os_glance_import_task' on the ones i've seen
14:19:24 <dansmith> we could probably remove a bunch of eventlet code in glance by removing tandalone mode
14:19:29 <croelandt> rosmaita: yeah it's always this one
14:19:35 <croelandt> dansmith: then let's do it :)
14:19:40 <abhishekk> ++
14:20:21 <croelandt> ok so I'll let you take a look at the zuul patch
14:20:26 <croelandt> #topic One easy patch per core dev
14:20:34 <croelandt> #link https://review.opendev.org/c/openstack/glance/+/944381
14:20:51 <croelandt> so this is about making sure we correctly update the doc headers every release
14:21:16 <croelandt> but I also wonder whether we have a document listing all steps to take when making a release (send a patch to openstack/releases, update the doc headers, etc.)
14:21:19 <croelandt> as PTL that would help
14:21:23 <croelandt> otherwise I'll write down my own notes
14:21:37 <abhishekk> +1 for doc
14:22:37 <croelandt> yeah so if that does not exist I'll take notes and try to add something in doc/source/ by the end of the cycle
14:23:01 <abhishekk> ack
14:23:08 <croelandt> Moving on
14:23:11 <croelandt> #topic  Open Discussion
14:23:21 <croelandt> So I'm gonna be away next two Thursdays
14:23:35 <croelandt> oh and also the 29th
14:23:59 <dansmith> should we chat about the image size transfer encoding thing?
14:24:00 <croelandt> I think Mridula will be back to chair the meeting though
14:24:02 <abhishekk> I want someone to have look at my scrubber patch
14:24:10 <abhishekk> yes
14:24:12 <croelandt> abhishekk_: yeah I need to take a look at the new version
14:24:15 <rosmaita> croelandt: https://docs.openstack.org/glance/latest/contributor/releasecycle.html
14:24:16 <croelandt> dansmith: you have the mic
14:24:41 <croelandt> rosmaita: oh nice!
14:24:55 <rosmaita> looks like it needs an update
14:25:04 <dansmith> So I'm very much not in favor of disabling chunked transfers when someone passes image_size (or a flag to disable chunked transfers), especially since we've been doing chunked transfers for a long time, and we're about to freeze glanceclient
14:25:10 <croelandt> rosmaita: yeah I'll read and update
14:25:33 <dansmith> so I don't want to swoop in with a last-minute change to completely alter how we transfer gigs of image data, then freeze the library,
14:25:34 <abhishekk> dansmith: in short we will be calculating size at client and then pass it to API
14:25:52 <dansmith> especially since the right thing, IMHO, is to take image_size but send it to the API instead of changing the transfer encoding
14:26:09 <abhishekk> AND at api side we will reject upload if size is not specified or continue without that?
14:26:27 <dansmith> abhishekk: we can't calculate it, we need to take it as a parameter, but communicate it some other way to the API and not just change from chunked to unchunked transfer encoding
14:27:07 <abhishekk> Ack, so user specified size we will rely on and then reject the whole upload if size mis-matches?
14:27:32 <dansmith> it's not just the reject that matters, although we should do that too,
14:27:45 <dansmith> it's that glance needs to know the size before the transfer starts so we can avoid resizing the backend every time we read a chunk
14:28:26 <dansmith> to be clear, I'm referring to this: https://review.opendev.org/c/openstack/glance-specs/+/947423
14:29:32 <abhishekk> Sorry If I am missing something, last time I was under impression that we will know the filename and we can calculate the size based on that
14:29:41 <dansmith> the original proposal was to add a "bypass chunked transfer" flag, which requires the user to know why that matters, when to use it, and why it's linked to the backend write behavior, which is wrong IMHO
14:30:08 <dansmith> abhishekk: not always, if we pass you a generator you can't know (but nova knows)
14:30:48 <abhishekk> ack
14:30:52 <dansmith> the discussion has led towards more of a hack (IMHO) which is to bypass chunked encoding if we pass a size, which is just more obscure but the same problem, IMHO
14:31:24 <dansmith> and it means the nova->glanceclient->glance->backend data pipeline we've been using for a long time will change subtly because we start passing the image size,
14:31:33 <dansmith> only because glance requires it in content-length only
14:32:08 <dansmith> so Im just trying to argue against changing all that data pipeline stuff, and linking the passing of image size to the fundamental way that all works, just because it's the "easiest" solution
14:33:14 <abhishekk> We need to worry about traditional image upload scenario only I guess
14:33:33 <dansmith> basically, if we do the "easy" solution it's only a patch to glanceclient, which can be backported easily (downstream) but has big ramifications to what's actually happening
14:33:37 <dansmith> and I think that's a mistake
14:33:58 <dansmith> abhishekk: probably.. I dunno how the plumbing works on import, but I suspect it has the size at that point, so everything is fine
14:34:13 <dansmith> i.e. once it's in staging, we know the size
14:34:49 <abhishekk> right, I am also assuming that we set size once image is staged
14:35:16 <abhishekk> Need to confirm though I saw resize calls in logs on import as well
14:35:40 <dansmith> if you use web-download, then it would not know the size and you'd see resizes
14:35:58 <abhishekk> for web-download also we stage the image locally
14:36:18 <dansmith> oh, I guess I forgot
14:36:33 <abhishekk> yes we stage for all import operartions
14:36:33 <dansmith> well, that's a different problem then
14:36:53 <dansmith> maybe we set size during the import task flow then? if that's the case, then that's fixable internally fairly easily
14:37:15 <abhishekk> agree
14:37:16 <dansmith> but still, focusing on the thing in the spec,
14:37:27 <abhishekk> ack, let me explain what I understand and see we are on right page
14:38:02 <abhishekk> 1. Introduce --size option on upload (use existing if it is present), then pass it to API
14:38:22 <abhishekk> 2. At API side if we don't have size continue the old way
14:38:59 <abhishekk> 3. If we have size then compare it like hash at the end of upload and log warning and set correct size or reject the upload
14:39:24 <dansmith> reject, but yes :)
14:40:05 <abhishekk> Here at point 3, if user specified size as 38MB and actual size is 50MB then I guess it should fail for rbd backend as Initially we create volume of specified size right?
14:40:18 <dansmith> yes
14:40:28 <abhishekk> what will be the impact on other backends, specially swift, cinder and s3?
14:40:37 <abhishekk> I guess cinder will also fail
14:41:05 <dansmith> I think we can check the size in common code and make them all behave the same
14:41:07 <dansmith> can, and should
14:42:42 <dansmith> so will you rewrite the spec to say the 1,2,3 above?
14:42:46 <abhishekk> ack
14:42:56 <abhishekk> Yes, I will rewrite the spec
14:43:02 <dansmith> cool thanks
14:43:19 <abhishekk> mention current discussion as proposed change and the alternative as bypassing chunk transfer?
14:43:44 <dansmith> sure
14:44:11 <dansmith> but with the reasoning to reject the alternative as incorrectly linking weird glance behavior with the transfer mechanics of the client as considered harmful :D
14:44:29 <abhishekk> :d
14:44:46 <abhishekk> thank you, I will update the spec by tomorrow eod
14:45:04 <croelandt> good, anything else for today?
14:45:06 <dansmith> thanks
14:45:14 <mhen> hi! regarding the image encryption patchset ...
14:45:21 <mhen> still on it - need to look into the image import stuff a bit more to allow glance-direct and copy-image to be used on encrypted images, which the current state of the patchset does not allow but I agree would be useful
14:45:38 <abhishekk> no, please look at scrubber patch
14:45:38 <mhen> I'm currently involved in a project for the next few weeks that limits my time to work on this even more than usual unfortunately though :/
14:45:52 <abhishekk> mhen: let me know if you need anything
14:46:20 <mhen> thanks, will do
14:46:53 <croelandt> yeah don't hesitate to ping us directly
14:46:53 <abhishekk> thanks for the update, as promised during PTG I will or someone from team will look at tempest coverage for this
14:47:47 <croelandt> Anything else?
14:48:07 <abhishekk> dansmith: I guess we need to specs for this topic now since API side change is also involved :o
14:48:17 <abhishekk> to --- two
14:48:24 <dansmith> it's already a spec?
14:48:28 <dansmith> or you mean instead of spec-lite?
14:48:40 <dansmith> oh two specs.. do we?
14:48:54 <dansmith> in nova, it'd be one, I'm fine with one for sure :)
14:48:55 <abhishekk> for glanceclient I can convert it to lite spec and main spec will be in glance
14:49:17 <abhishekk> ack, then lets go wit one spec only :D
14:49:29 <abhishekk> @croelandt nothing else :D
14:49:40 <dansmith> oh because there are separate dirs.. well, whatever, I think one should be plenty but I defer to croelandt I guess :)
14:50:26 <abhishekk> I will move the spec to glance instead of client
14:51:03 <croelandt> good
14:51:17 <croelandt> I guess we have no additional topics, so thanks everyone for joining!
14:51:26 <abhishekk> thank you!!
14:52:13 <croelandt> #endmeeting