| *** soniya29 is now known as soniya29|afk | 07:16 | |
| *** blarnath is now known as d34dh0r53 | 13:05 | |
| *** dasm|off is now known as dasm | 13:10 | |
| abhishekk | #startmeeting glance | 14:00 |
|---|---|---|
| opendevmeet | Meeting started Thu Jan 12 14:00:05 2023 UTC and is due to finish in 60 minutes. The chair is abhishekk. 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 |
| abhishekk | #topic roll call | 14:00 |
| abhishekk | #link https://etherpad.openstack.org/p/glance-team-meeting-agenda | 14:00 |
| abhishekk | o/ | 14:00 |
| mrjoshi__ | o/ | 14:00 |
| abhishekk | pdeore is not around due to some technical issues | 14:00 |
| croelandt | o/ | 14:01 |
| jokke_ | o/ | 14:01 |
| abhishekk | lets wait couple of minutes | 14:01 |
| rosmaita | o/ | 14:01 |
| abhishekk | cool,lets start | 14:01 |
| abhishekk | #topic Updates | 14:02 |
| abhishekk | We tagged M2 last week | 14:02 |
| abhishekk | and glance-store 4.2.0 is also released | 14:02 |
| abhishekk | We postponed a spec freeze by couple of weeks due to holiday season, so new spec freeze will be next week | 14:03 |
| abhishekk | Kindly have a look at important specs | 14:03 |
| abhishekk | M3 is 5 weeks away | 14:03 |
| abhishekk | Periodic jobs green except for Fips (as usual) | 14:03 |
| abhishekk | any questions? | 14:03 |
| abhishekk | lets move to next one | 14:04 |
| abhishekk | #topic Critical: glanceclient gate has *2* blocker bugs | 14:04 |
| abhishekk | croelandt, this is you, right? | 14:04 |
| croelandt | yes | 14:04 |
| croelandt | so basically, jokke and I squashed the changes | 14:04 |
| croelandt | https://review.opendev.org/c/openstack/python-glanceclient/+/869868 | 14:04 |
| croelandt | this is needed to unblock the gate | 14:04 |
| croelandt | There are *2* issues, not one :) | 14:04 |
| jokke_ | We had a lengthy discussion about it in #os-g | 14:05 |
| croelandt | Because of a bump in the Python version and the tox version | 14:05 |
| abhishekk | so this is the single patch which is required now | 14:05 |
| croelandt | I wonder if this is something that could have been caught | 14:05 |
| croelandt | abhishekk: yes | 14:05 |
| jokke_ | yes, we need to have it in signle patch as both issues will independently block the gate ... just waiting for zuul | 14:05 |
| croelandt | Zuul is running, so if 2 people could commit to taking a look in a couople of hours | 14:05 |
| abhishekk | I will go through channel logs to get more idea | 14:05 |
| croelandt | like before EOD | 14:05 |
| croelandt | jokke_: ^ | 14:05 |
| croelandt | rosmaita: ^ | 14:05 |
| abhishekk | I will have a look as well | 14:05 |
| rosmaita | i can watch that | 14:06 |
| abhishekk | great | 14:06 |
| croelandt | rosmaita: thanks for reviewing that last year btw :) | 14:06 |
| rosmaita | i should've ninja-approved it, would have prevented some unpleasantness | 14:06 |
| abhishekk | a year old blocking patch :D | 14:06 |
| abhishekk | moving ahead | 14:07 |
| abhishekk | #topic Specs | 14:07 |
| abhishekk | Repropose new location APIs spec | 14:07 |
| abhishekk | #link https://review.opendev.org/c/openstack/glance-specs/+/863209 | 14:07 |
| abhishekk | whoami-rajat, has updated the spec | 14:07 |
| abhishekk | anyways this has dependency on keystone as well | 14:07 |
| abhishekk | even keystone spec is merged, patch is still wating for review | 14:08 |
| whoami-rajat | hi | 14:08 |
| abhishekk | hello whoami-rajat | 14:08 |
| whoami-rajat | in the last review, there was a proposal to consider import method for this implementation | 14:08 |
| whoami-rajat | wanted to discuss about that | 14:08 |
| abhishekk | I guess jokke_ and dansmith suggested that | 14:09 |
| whoami-rajat | yes | 14:09 |
| * whoami-rajat looking for the comment | 14:09 | |
| jokke_ | so I don't think it's good idea at all to mix it with the import | 14:10 |
| jokke_ | as there is no importing happening | 14:10 |
| whoami-rajat | https://review.opendev.org/c/openstack/glance-specs/+/863209/4..7/specs/2023.1/approved/glance/new-location-info-apis.rst#b236 | 14:10 |
| jokke_ | I think I should have wrapped my response with <sarcasm></sarcasm> | 14:11 |
| whoami-rajat | :D | 14:11 |
| jokke_ | And I still don't understand why it's depending on the service roles either | 14:11 |
| whoami-rajat | create i think is not dependent but the get location call is | 14:12 |
| abhishekk | yes | 14:12 |
| whoami-rajat | we only want to allow fetching the location from a consumer service and not a user | 14:12 |
| jokke_ | Ohh yeah there was that | 14:12 |
| Uggla | o/ | 14:12 |
| abhishekk | correct, so whoami-rajat I guess you know now what to do, remove import related changes and re propose the spec\ | 14:13 |
| abhishekk | Uggla, hey, will get back to you once current discussion is over | 14:13 |
| jokke_ | So unless we want to just push this down yet another cycle I don't think we should be too worried waiting for that before agreeing on the spec | 14:13 |
| whoami-rajat | ok but do we want to allow end users to update the location for HTTP? | 14:14 |
| whoami-rajat | i think that was the start of this discussion | 14:14 |
| jokke_ | yes and all the adds should happen with the projects user token anyways | 14:14 |
| abhishekk | yes | 14:15 |
| jokke_ | So we can have traces who did what for accountability and auditability | 14:15 |
| whoami-rajat | ok, i will add a new command for that and remove the existing import part | 14:15 |
| abhishekk | ack | 14:15 |
| whoami-rajat | thanks abhishekk and jokke_ | 14:16 |
| jokke_ | I don't mind if it stays there for completeness of discussion. It's only mentioned in alternative solutions, right? | 14:16 |
| whoami-rajat | no, it's in the end user impact | 14:16 |
| jokke_ | ohh | 14:17 |
| whoami-rajat | https://review.opendev.org/c/openstack/glance-specs/+/863209/4..7/specs/2023.1/approved/glance/new-location-info-apis.rst#280 | 14:17 |
| jokke_ | ok | 14:17 |
| abhishekk | then there are some other specs as well, kindly have a look at those and give your responses | 14:17 |
| jokke_ | missed that part ... I even read it couple of times through this week | 14:17 |
| abhishekk | aah | 14:17 |
| whoami-rajat | even in the proposed change section it's there, https://review.opendev.org/c/openstack/glance-specs/+/863209/4..7/specs/2023.1/approved/glance/new-location-info-apis.rst#120 | 14:17 |
| whoami-rajat | i can add it to alternatives if needed | 14:17 |
| whoami-rajat | i.e. move the current sections to alternatives | 14:18 |
| abhishekk | ack | 14:18 |
| abhishekk | moving to Open discussions | 14:18 |
| abhishekk | #topic Open discussion | 14:19 |
| jokke_ | whoami-rajat: if you feel like. I think it wouldn't hurt to keep that mentioned in the alternative solution section. That way we demonstrate that it was discussed and thought of | 14:19 |
| croelandt | whoami-rajat released a patch to refactor the Cinder store | 14:19 |
| croelandt | it's pretty good, I encourage everyone to review it :) | 14:19 |
| whoami-rajat | jokke_, sure, will do, thanks | 14:19 |
| abhishekk | Uggla, hey, go ahead if you want to discuss anything specific | 14:19 |
| whoami-rajat | croelandt, thanks Cyril, i saw your comments and will address them soon | 14:19 |
| Uggla | Yep thx, artom and I are working on nova | 14:20 |
| artom | o/ | 14:20 |
| Uggla | We got a bug from Lee, and I will copy paste what Artom wrapup last time: | 14:20 |
| Uggla | In that BZ, Lee is saying that os_hash_(alog|value) should not be set for an image representing a volume snapshot, as no data actually resides in Glance | 14:20 |
| Uggla | Nova's current hack is to upload an empty string as data, this causes os_hash_(alog|value) to be calculated | 14:20 |
| Uggla | We've tried not uploading any image data at all, but then the image remains in `queued` | 14:20 |
| Uggla | So we're trying to figure out how to activate an image out of `queued` and into `active` without uploading any data | 14:20 |
| Uggla | Ah, based on https://docs.openstack.org/glance/latest/user/statuses.html, looks like size has to be 0? | 14:20 |
| Uggla | Lee was saying something else in the BZ: | 14:21 |
| Uggla | "When creating an ephemeral snapshot that uses the underlying direct snapshot mechanism in Nova imageCreate doesn't do this, instead creating an image initially with an unset size [4][5] that results in nothing being created in Glance's backend. As we don't upload any further data to the image in this case [6] the hash properties are not recalculated and left empty." | 14:21 |
| Uggla | But I guess size gets set to 0 eventually in that code path? We just haven't found where yet | 14:21 |
| abhishekk | afaik, it is not possible to have image with no data | 14:22 |
| abhishekk | jokke_, rosmaita am I right? | 14:22 |
| jokke_ | well if you upload zero size blop of data, it will be seen by gapi and set to 0 | 14:22 |
| artom | Will it try to calculate os_hash_(alog|value) on that empty data? | 14:23 |
| jokke_ | abhishekk: correct. Glance does not have concept for active image without data. There is no benefit for it. Why is Nova doing this in the first place? | 14:23 |
| jokke_ | artom: sure does | 14:24 |
| artom | Lee was saying that it shouldn't? | 14:24 |
| artom | jokke_, so, as far as I can tell (and storage is *not* my area of expertise), for volume snapshot where the backend does the snapshotting, no actual data lives in Glance | 14:24 |
| artom | It's a "pointer" for lack of a better term | 14:24 |
| rosmaita | probably for the nova/glance shared ceph backend? | 14:25 |
| jokke_ | artom: I'm not sure how hashlib behaves if you pass it just 0 length data on the call, but glance should populate the algo and hash once the upload is done | 14:25 |
| artom | Uggla, can you paste the BZ here for full context? | 14:25 |
| Uggla | yep give a sec | 14:26 |
| jokke_ | rosmaita: for that there is snapshot in ceph that gets put into location and that activates the image | 14:26 |
| abhishekk | correct | 14:26 |
| Uggla | https://bugzilla.redhat.com/show_bug.cgi?id=2011771 | 14:26 |
| artom | "'<jokke_> rosmaita: for that there is snapshot in ceph that gets put into location and that activates the image" | 14:26 |
| artom | OK, I think ^^^ is what we want | 14:26 |
| artom | Is that a Glance API call that Nova can do? | 14:26 |
| jokke_ | but if nova is snapshotting something which data is not accessible via Glance, nova should not use Glance for trashcan-as-service generic database to track it | 14:27 |
| artom | *snerk* | 14:27 |
| artom | I think it's the user requesting the snapshot, so presumably they want to then do stuff with it later | 14:27 |
| artom | Hence the Glance image creation | 14:27 |
| jokke_ | artom: if you look how it's done with ceph, that's the mechanism. But gapi will not take the location in unless there is valid store configured for it | 14:28 |
| artom | jokke_, ah, so that'll only work if Nova and Glance have the same Ceph backend configured? | 14:29 |
| abhishekk | yes | 14:29 |
| jokke_ | artom: the point is, if user wants a snapshot image, Nova should either provide valid location (like with ceph) or upload the data so that valid image can be activated | 14:29 |
| artom | jokke_, so I think what Nova is currently doing is an ugly hack wherein it uploads an empty string as the data | 14:30 |
| jokke_ | artom: yeah, which creates image, sure but it cannot be used anyways | 14:30 |
| artom | For the first case, where Nova should actually be providing the location... which I guess it might be doing anyways? I dunno, that code is a mess, and only Lee understood it | 14:30 |
| abhishekk | for the first case you mean nova and glance using ceph? | 14:32 |
| artom | I... guess? I don't know yet | 14:32 |
| artom | But at least I understand what Glance expects | 14:32 |
| * croelandt has another meeting | 14:32 | |
| jokke_ | artom: looking the bug, Nova seems to get the correct location, no prob, but then do this mess anyways just because it was volume backed instance instead of ephemeral | 14:32 |
| abhishekk | yes for nova and glance using ceph it does not calculates hash and just nova calls location add for glance | 14:33 |
| artom | jokke_, so you're saying just stop doing this empty string mess? | 14:33 |
| artom | And as long as the location is provided by Nova, Glance will he happy? | 14:33 |
| jokke_ | artom: so as long as the case is ceph all the way through, you should be able to throw out all that "" upload crap and use the same logic as is used with ephemeral to provide the location uri to glance and get actual accessible image out of it | 14:33 |
| artom | Ack | 14:34 |
| artom | How is the location URI provided? Via PUT /images? | 14:34 |
| jokke_ | artom: currently it's update call, so yeah I think it's PUT to /v2/images/<ID> and it is JSON patch for the location | 14:35 |
| artom | Ack | 14:35 |
| artom | Uggla, ^^ you followed that? | 14:35 |
| Uggla | artom, to be honest not really :) | 14:35 |
| jokke_ | But the ephemeral snapshotting code in nova should be pretty easy to grasp, I've looked at it quite a few times. IIRC it doesn't contain any crazy black magic | 14:36 |
| artom | jokke_, nothing storage-related in Nova is easy to grasp | 14:36 |
| abhishekk | https://github.com/openstack/nova/blob/master/nova/image/glance.py#L558 ? | 14:36 |
| artom | Or maybe I'm just dumb | 14:36 |
| artom | And I think this is specifically the volume backed case | 14:37 |
| artom | Though... come to think of it, if the ephemeral backend is also Ceph, and the same Ceph is configured in Glance... there's no reason not to use the same mechanism of direct snapshotting | 14:37 |
| artom | ... right? | 14:38 |
| jokke_ | I wouldn't be surprised if the volume backed case is totally different logic. What I'm saying is, look how it's done in the ceph ephemeral and simplify the volume part :D | 14:38 |
| jokke_ | artom: exactly | 14:38 |
| abhishekk | artom, if you will need any further help ping us on openstack-glance irc channel | 14:40 |
| artom | abhishekk, will do, I appreciate it | 14:41 |
| jokke_ | That in theory 'though. I'm sure you'll find some Nova core who thinks that the complexity that only one person can understand is absolutely critical and mut be maintained :P | 14:41 |
| jokke_ | must | 14:41 |
| artom | Nah, those cores have all left ;) | 14:41 |
| jokke_ | And you might find more than one of those cores in Nova side of the fence :P | 14:42 |
| artom | We only have reasonable people now ;) | 14:42 |
| jokke_ | oh, good to hear | 14:42 |
| jokke_ | then you should have no problem fixing that :D | 14:42 |
| artom | Famous last words | 14:42 |
| abhishekk | I don't have anything specific to discuss for today! | 14:42 |
| artom | OK, we'll go off and look at the Nova code again, and probably come back even more confused | 14:42 |
| jokke_ | point on the _you_ and _should_ :P | 14:43 |
| artom | Appreciated folks, thank you! | 14:43 |
| abhishekk | thanks for joining us | 14:43 |
| jokke_ | good luck! | 14:43 |
| Uggla | thank you | 14:43 |
| abhishekk | thanks Uggla! | 14:43 |
| abhishekk | glance team, kindly have a look at the specs in the agenda as next week is a spec freeze | 14:43 |
| abhishekk | lets close unless you guys have anything further to add/discuss | 14:44 |
| abhishekk | I will take that as no | 14:45 |
| abhishekk | thank you all | 14:45 |
| jokke_ | thanks all | 14:45 |
| abhishekk | have a nice week and weekend ahead! | 14:45 |
| abhishekk | #endmeeting | 14:45 |
| opendevmeet | Meeting ended Thu Jan 12 14:45:41 2023 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 14:45 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/glance/2023/glance.2023-01-12-14.00.html | 14:45 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/glance/2023/glance.2023-01-12-14.00.txt | 14:45 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/glance/2023/glance.2023-01-12-14.00.log.html | 14:45 |
| *** dasm is now known as dasm|off | 22:31 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!