| *** haleyb_ is now known as haleyb | 12:52 | |
| *** dasm|off is now known as dasm|ruck | 13:05 | |
| pdeore | #startmeeting glance | 14:00 |
|---|---|---|
| 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 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 14:00 |
| opendevmeet | The meeting name has been set to 'glance' | 14:00 |
| pdeore | #topic roll call | 14:00 |
| pdeore | #link https://etherpad.openstack.org/p/glance-team-meeting-agenda | 14:00 |
| pdeore | o/ | 14:00 |
| abhishekk | o/ | 14:00 |
| alistarle | o/ | 14:00 |
| * abhishekk will be here for short time only | 14:00 | |
| abhishekk | jokke_, is on leave | 14:01 |
| croelandt | o/ | 14:01 |
| pdeore | we have short agenda for today | 14:01 |
| dansmith | o/ | 14:01 |
| mrjoshi | o/ | 14:01 |
| pdeore | let's start | 14:01 |
| abhishekk | Lets start | 14:01 |
| pdeore | #topic release/periodic jobs | 14:01 |
| pdeore | We have tagged M2 last week | 14:02 |
| pdeore | and M3 is 6 weeks from now... | 14:02 |
| pdeore | Periodic Jobs, all green except the 'translation update' jobs failing with RETRY_LIMIT | 14:02 |
| abhishekk | For M3 we have 2 major patches to be merged | 14:02 |
| abhishekk | 1 Glance download import method | 14:03 |
| abhishekk | 2 Extend store details information for all stores | 14:03 |
| abhishekk | both patches are up for reviews | 14:03 |
| pdeore | yes | 14:03 |
| abhishekk | Unfortunately we did not here anything from tobias regarding injecting metadata during upload process | 14:04 |
| abhishekk | so probably we are going to skip it in this cycle | 14:05 |
| * dansmith nods | 14:05 | |
| abhishekk | lets move ahead | 14:06 |
| pdeore | yeah | 14:06 |
| pdeore | #topic Bugs | 14:06 |
| pdeore | #link https://bugs.launchpad.net/python-glanceclient/+bug/1597623 | 14:06 |
| croelandt | that was me :) | 14:06 |
| croelandt | I think we've stated multiple times that we only want to use image IDs in commands | 14:06 |
| croelandt | and not image names | 14:06 |
| croelandt | am I mistaken or can we"close" this bug? | 14:06 |
| abhishekk | We can mark it as won't fix | 14:07 |
| croelandt | great, thanks for confirming that :) | 14:07 |
| dansmith | I really hate that behavior, fwiw :) | 14:07 |
| abhishekk | ++ | 14:07 |
| dansmith | and glance is the outlier here, AFAICT | 14:07 |
| croelandt | it's kind of more user-friendly I guess | 14:07 |
| dansmith | but basically, I have to use my mouse to interact with glanceclient, which is infuriating | 14:08 |
| croelandt | in bug reports sometimes I wish we had names rather than UUIDs | 14:08 |
| 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:08 |
| 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 |
| 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 |
| dansmith | but literally I can interact with the whole rest of the stack with names, except for glance | 14:09 |
| croelandt | yeah and it's the kind of patch that's gonna need multiple rounds | 14:10 |
| croelandt | because someone is not gonna be happy about the tests etc. | 14:10 |
| croelandt | and it's going to drag on for 3 years | 14:10 |
| abhishekk | :D | 14:10 |
| dansmith | well, I don't think it's about the tests or implementation, but the principle I suspect :) | 14:10 |
| croelandt | yeah also that | 14:10 |
| croelandt | I mean the implementation is interesting as well | 14:11 |
| croelandt | there's some potential for errors if people name multple images the same | 14:11 |
| croelandt | etc. | 14:11 |
| dansmith | it's the same for all other resources though | 14:11 |
| croelandt | maybe something to revisit in the future once we're tired of copy/pasting uuids with the damn mouse | 14:11 |
| abhishekk | mostly principle, I remember there is lots of negative opinions from glance team at that time | 14:11 |
| dansmith | names are not unique constrants in many places | 14:11 |
| croelandt | jsut wait for our wrists to be broken | 14:11 |
| pdeore | :D :D | 14:12 |
| croelandt | abhishekk: we **are** the Glance team :D | 14:12 |
| abhishekk | croelandt, yeah, but it was discussed during Hawana cycle as well | 14:12 |
| croelandt | let'sd discuss it again at the next H cycle | 14:12 |
| abhishekk | I remember I had a PoC ready for this at that time :D | 14:13 |
| dansmith | maybe we could get an operator survey question about it | 14:13 |
| abhishekk | Sounds good | 14:13 |
| croelandt | dansmith: +1 | 14:13 |
| pdeore | ++ | 14:13 |
| pdeore | move to next ? :) | 14:14 |
| abhishekk | Lets take help of rosmaita for drafting operators question | 14:14 |
| abhishekk | yes, lets move ahead | 14:15 |
| pdeore | #topic Glance download import work | 14:15 |
| abhishekk | alistarle, is back now | 14:15 |
| pdeore | yeah, and there are some updates on the patch , https://review.opendev.org/c/openstack/glance/+/840318/23..24 | 14:16 |
| abhishekk | alistarle, do you have any questions related to review which you want to discuss? | 14:16 |
| alistarle | hmmm yes, I see dan comment | 14:17 |
| alistarle | For the URL check, do you think a cast to UUID check is enough ? | 14:17 |
| dansmith | alistarle: probably, or a quick check that they're just hex characters | 14:18 |
| dansmith | for the image_id you mean | 14:18 |
| alistarle | yep | 14:18 |
| dansmith | I still think we should re-use that scheme checker on the url we construct | 14:18 |
| dansmith | which should be easy I think | 14:18 |
| dansmith | if glance image ids are always uuids then cast to uuid seems okay | 14:19 |
| 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 |
| 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 |
| dansmith | right for sure | 14:20 |
| dansmith | apologize that my "add glance-download here" meant physically "here" but I meant "add this check for glance-download too" | 14:21 |
| alistarle | ok so no problem, I will do and update the doc accordingly | 14:21 |
| dansmith | we should have some tests for these things too, so let me know if I need to write those | 14:22 |
| dansmith | but hopefully my test module got you started enough to copy for other scenarios | 14:22 |
| alistarle | yep sure | 14:22 |
| alistarle | and regarding the content-length stuff, so the header check isn't enough like for the web-download ? | 14:23 |
| alistarle | (I will check if glance return it btw) | 14:23 |
| dansmith | I don't think glance sets it AFAIK | 14:24 |
| 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:24 |
| alistarle | Hmmm ok, but checking the image size glance side will require an additional call to remote glance API | 14:25 |
| abhishekk | https://docs.openstack.org/api-ref/image/v2/?expanded=download-binary-image-data-detail#response | 14:25 |
| alistarle | And are we sure glance will provide us the right size ? | 14:25 |
| dansmith | yeah, which is what we're trying to do here.. provide a glance-aware download mechanism :) | 14:26 |
| 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 |
| abhishekk | ack | 14:27 |
| abhishekk | will look at it | 14: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:28 |
| abhishekk | right | 14:29 |
| 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 |
| 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:30 |
| alistarle | issue is we copy metadata before we download the image, so we can't do the check | 14:32 |
| dansmith | oh right, heh :) | 14:32 |
| dansmith | well, then we probably need to just do the extra call then | 14:33 |
| 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 |
| abhishekk | can we pass the size to following task as an input? | 14:33 |
| 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:33 |
| dansmith | abhishekk: we could set it as os_glance_remote_size and then remove that when we've checked | 14:34 |
| dansmith | but honestly, I don't think the extra call is that bad | 14:34 |
| abhishekk | ok | 14:34 |
| dansmith | alistarle: yeah bad idea I think | 14:34 |
| 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 |
| abhishekk | alistarle, afaik, we can pass param between tasks | 14:35 |
| dansmith | but if metadata is disabled then we won't have a size to check that way | 14:36 |
| abhishekk | yeah | 14:36 |
| 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 |
| abhishekk | disabled means there is no item in that config option, right? | 14:36 |
| alistarle | nope, it is forcefully added to the flow | 14:37 |
| 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 |
| dansmith | oh okay, then nevermind | 14:37 |
| alistarle | we don't wanted the make it configurable for inconsistency reason | 14:37 |
| alistarle | you can choose to copy nothing, but actually the plugin will run (at least in the current patch set) | 14:38 |
| alistarle | to update at least the container_format and disk_format | 14:38 |
| 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 |
| dansmith | but yeah I see now | 14:38 |
| abhishekk | https://review.opendev.org/c/openstack/glance/+/840318/24/glance/async_/flows/api_image_import.py#85 | 14:38 |
| alistarle | and should we rely more on the size or virtual_size attribute ? I remember there is two field in the API | 14:40 |
| 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:40 |
| abhishekk | I think size is what we should verify | 14:41 |
| 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 |
| dansmith | yes, size | 14:41 |
| 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 |
| abhishekk | ack | 14:42 |
| alistarle | ok let's do that, I will rework the patch accordingly | 14:43 |
| 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:43 |
| abhishekk | ++ | 14:44 |
| 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 |
| alistarle | seems ok to me | 14:44 |
| abhishekk | perfect | 14:44 |
| dansmith | alistarle: and sanitize the image_id | 14:44 |
| alistarle | abhishekk: I will check with you if I struggle to forward param between tasks | 14:44 |
| abhishekk | alistarle, ack | 14:44 |
| 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 |
| alistarle | sure | 14:45 |
| dansmith | all good to move on? | 14:47 |
| alistarle | yep | 14:47 |
| alistarle | sorry for the long time we spend on that topic ;) | 14:47 |
| dansmith | not a problem IMHO, glad we're progressing | 14:48 |
| abhishekk | agree | 14:48 |
| pdeore | moving to Open discussion... | 14:49 |
| pdeore | #topic Open Discussion | 14:49 |
| abhishekk | alistarle, https://docs.openstack.org/taskflow/latest/user/inputs_and_outputs.html hopefully this will help | 14:49 |
| abhishekk | nothing from me for Open discussion | 14:50 |
| dansmith | abhishekk: ah nice | 14:50 |
| abhishekk | o/ | 14:51 |
| pdeore | so should we wrap up ? | 14:51 |
| dansmith | ++ | 14:51 |
| abhishekk | yep | 14:51 |
| abhishekk | thank you all | 14:52 |
| pdeore | gr8!! Thanks evryone for joining !! | 14:52 |
| pdeore | #endmeeting | 14:52 |
| opendevmeet | Meeting ended Thu Jul 21 14:52:29 2022 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 14:52 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/glance/2022/glance.2022-07-21-14.00.html | 14:52 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/glance/2022/glance.2022-07-21-14.00.txt | 14:52 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/glance/2022/glance.2022-07-21-14.00.log.html | 14:52 |
| *** dasm|ruck is now known as dasm|off | 21:04 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!