Thursday, 2022-04-28

*** dasm|off is now known as dasm13:10
pdeore#startmeeting glance14:00
opendevmeetMeeting started Thu Apr 28 14:00:14 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
opendevmeetUseful Commands: #action #agreed #help #info #idea #link #topic #startvote.14:00
opendevmeetThe meeting name has been set to 'glance'14:00
pdeore#topic roll call14:00
pdeore#link https://etherpad.openstack.org/p/glance-team-meeting-agenda14:00
pdeoreo/14:00
abhishekko/14:00
dansmitho/14:00
pdeorelets wait few minutes for everyone to join14:00
alistarleo/14:02
rosmaitao/14:02
mrjoshio/14:02
pdeoreok let's start14:03
pdeore#topic release/periodic jobs14:03
pdeoreMilestone 1 is just 3 weeks away14:03
pdeoreI think almost all the specs are up for review14:03
pdeorewe will go through the list in next topic14:04
pdeorePeriodic job, all green except POST_FAILURE for fips job and one time out14:04
pdeoremoving ahead14:04
pdeore#topic Specs For Review14:04
pdeoreMost of the specs up for review which needs some attention14:04
pdeoreNew API for instant caching of image : https://review.opendev.org/c/openstack/glance-specs/+/83864214:05
pdeorethis one needs new revision14:05
pdeoreExpanding store details - https://review.opendev.org/c/openstack/glance-specs/+/835606 (one +2)14:05
pdeoreDelete API for metadef resource types - https://review.opendev.org/c/openstack/glance-specs/+/81819214:05
abhishekkFor instant caching I need to update the spec as per our PTG discussion, somehow I missed about updating existing API (queue image) rather than adding new API   14:05
dansmithpdeore: I was mostly okay with the stores-detail one,14:05
dansmithso if it has added detail now hopefully it's good, I'll do that after the meeting14:05
pdeorecool14:06
pdeorethe Delete api for metadef rd types spec is up for review from last cycle, kindly please review this so that we can have this change in this cycle :)14:06
pdeores/rd/rs14:06
pdeoreUpdate proposal for duplicate image download - https://review.opendev.org/c/openstack/glance-specs/+/73468314:07
pdeorethis one also needs to be updated14:07
pdeorespec-lite: ability to purge all rows by glance-manage - https://review.opendev.org/c/openstack/glance-specs/+/819622 (one +2)14:07
abhishekkthis one is also sitting from last cycle14:07
pdeoreyeah14:08
pdeoreSo all members, kindly review the spec and give your suggestions14:08
pdeoremoving ahead14:09
abhishekkack14:09
pdeore#topic Secure RBAC14:09
dansmithpdeore: where were you on the meeting??!14:09
pdeoreWe couldn't get updates on this in policy pop up meeting this week due to some technical issue with meetpad, so it's moved to next week14:09
dansmiththat was annoying.. meetpad--14:09
dansmithsorry you got bit by that :(14:10
pdeoredansmith, I was there but i was not able to hear anything :/14:10
dansmithpdeore: I know, I'm poking fun :)14:10
abhishekk:D14:10
pdeore:D :D 14:10
dansmithrescheduled quickly for next week and using another tool, so hopefully will be better :)14:10
abhishekkpdeore, as I added in etherpad we do need to have that validation for POST APIs in client as well as API side14:11
pdeoreyeah hopefully :)14:11
dansmiththe heat cross-scope thing actually affects glance I imagine14:11
pdeoreabhishekk, ack14:11
dansmithin that anything we nail down to system scope may be a problem for heat users that currently create things as admin14:11
* jokke_ sneaks in14:12
dansmith"we" meaning glance14:12
abhishekkoh14:12
pdeoreohh ok14:12
abhishekkand I guess they are going to work on that during this cycle14:12
dansmithit's a real mess and I'm not sure what the solution is going to be, but hopefully we'll get some more clarity next week14:12
dansmithabhishekk: I don't know.. very little people working on heat these days, like horizon, who is also impacted14:13
abhishekkack, I will try to be there for next meeting14:13
abhishekkI think then we are pretty much sorted out for RBAC14:13
dansmithyeah14:13
pdeorekindly please have a look at the glance policy matrix as well, #link https://review.opendev.org/c/openstack/glance/+/83885714:14
abhishekkok,lets wait for the outcome of next meeting then 14:14
pdeoreI hope I have not missed anything there14:14
abhishekkpdeore, ack, have you updated it after our discussion?14:14
pdeoreyes 14:14
abhishekkcool,I will have a look today or tomorrow14:15
pdeoreabhishekk, Thanks !14:15
abhishekkwe will shift next topic to last14:15
pdeoreok lets move ahead14:15
abhishekkas it will be lengthy discussion14:15
pdeoresure14:16
pdeore#topic Policy Refactor part 214:16
pdeoreAs per discussion in PTG, the DNM patch has been submitted14:16
pdeore#link  https://review.opendev.org/c/openstack/glance/+/83949114:16
abhishekkyeah and as I stated functional tests are passing as well as no impact on tempest side14:16
jokke_++14:16
abhishekkI skipped affected unittests so that you guys can get idea about it 14:17
abhishekkSo once minimum two member will give go ahead on DNM patch I will start removing the code14:17
abhishekks/I/or pdeore 14:18
dansmithgood stuff14:18
pdeorelet's move ahead14:18
pdeorecroelandt, I think next is you 14:18
jokke_I think that dnm is giving us as much confidence as we can get that the code is not used14:18
jokke_so ++ on cleanup from me14:19
abhishekk+114:19
croelandtyes, as discussed during PTG, we'll have a review "party" next week14:19
croelandtkindly add your name in the little list so we can schedule it :)14:19
abhishekkI think we should utilize the meeting time for it14:19
croelandtI'll put together a small (no) list of patches14:20
croelandtabhishekk: works for me :)14:20
abhishekk+114:20
pdeore+114:20
croelandtand I guess that's it, I'll promise it will be fun14:20
croelandtfor some definition of "fun"14:20
abhishekk:D14:21
jokke_join review party, they say, it'll be fun, they say14:21
pdeore:D :D14:21
jokke_So that means no meeting next week?14:21
pdeoreyup :)14:22
abhishekkwe will keep 10 minutes and discuss face to face if there is anything in agenda14:22
pdeoreplease add your availability to time proposed by croelandt 14:22
croelandtjokke_: instead you get two hours of patch reviews!14:22
pdeoreok, let's move ahead14:23
pdeore#topic glance-download import method14:23
pdeoreNew glance-download import method - https://review.opendev.org/c/openstack/glance-specs/+/83613214:23
pdeorenew patch set is updated for this14:23
abhishekkalistarle, pslestang do you have anything to add here14:23
pdeoreExpanding store details - https://review.opendev.org/c/openstack/glance-specs/+/83560614:23
abhishekkI guess the spec is still missing info about minimum copying of metadata14:24
abhishekkdo you guys have any difficulty about it?14:24
pdeoreoops, please ignore last link .. :/14:24
alistarleOh we thought we should follow the jokke_ comment14:25
abhishekkthat is a comment not the decision,right?14:25
alistarleBut no issue we can update if we need14:25
alistarleI understand we have to then, so we'll do that shortly14:26
abhishekkSo jokke is insisting that we should stick to the fundamental of design14:26
abhishekkthat is, glance-direct method should just download the data like web-download14:26
abhishekks/glance-download14:27
dansmithalistarle: would be great if we could see the revision before the next meeting too.. it's kindof a rush at 7am to re-review before the meetings :)14:27
abhishekkSo I have added one more suggestion to the spec14:27
abhishekkCan we have two plugins for this solution14:27
abhishekk1. glance-download internal import method for importing the image from another glance14:27
abhishekk2. import-metadata external plugin like inject metadata for importing metadata from another glance14:27
abhishekkAnd if glance-download method is enabled then it should have import-metadata enabled as well.14:27
alistarleYeah sorry, we sync up our side just before the meeting also 14:27
pslestanghello14:28
dansmithtwo plugins means we have to hit the remote api more, probably less code-reuse, more opportunity to have it mis-configured14:28
dansmithbut if that's what we have to do then, it's better than noting14:28
abhishekkI think this is better idea14:28
pslestangabhishekk: if we do the import-metadata plugin we will have to be sure that it is run before the inject-metadat plugin14:28
abhishekkwe can work on second part if alistarle and pslestang finds it difficult14:28
dansmithpslestang: right, easier to mis-configure14:29
jokke_I don't think it needs to be even plugin, we can have a task down in the flow (we have quite a few there between establishing the image payload and the configurable plugins) which can be noop when there is no metadata to import14:29
abhishekkyeah, but we can document it 14:29
alistarledansmith: agree with the mis-configuration possibility, that's what we thought also14:29
abhishekkthat will also do14:29
abhishekksee my only concern is if the image is unusable then it will be additional overhead to admin to get it in usable state14:31
dansmithright, copy-image has a ton of flow steps,14:31
dansmithso this would be the same.. an extra flow in the glance-download thing right?14:31
jokke_Like I've been saying from very beginning, I'm not against importing the metadata and in long run we should have _consistent_ way of doing it ... which is why I'm dead against that being part of the internal plugin which job is to get the payload available for the rest of the ttaskflow14:31
abhishekkyep, like extra flow in glance-download flow14:32
jokke_I'm prefectly fine if there is a task in the import flow that does the importing of metadata should needed details be available14:32
alistarleOk, but what about updating the disk_format and container_type, shouldn't we put that here also ? In this extra step ?14:32
dansmithalistarle: I'm not sure why it wouldn't be the same step14:33
dansmithalistarle: you've just fetched the remote metadata and are about to apply it to the local image14:33
alistarleOk the glance-download never call the glance-api, except for downlading the data itself of course14:33
jokke_alistarle: IMO yes, all the metadata operations into it's own task ... that way should we ever get to an agreement on the export, we can be consistent in one place handling the metadata regardless what the import plugin used is14:34
alistarlecall to remote glance-api to retrieve container, disk and metadata will be handled by the extra workflow step dedicated to that14:34
abhishekkalistarle, I think you need to have one extra step/task to validate the hash as well to ensure we have entire image downloaded in source glance14:35
dansmithmaybe we're struggling with discussing in the abstract.. it would be helpful to have more detail in the spec, but maybe if we could get WIP code up we can actually discuss line-by-line instead of this?14:35
abhishekksounds good14:35
jokke_the hash, if we can calculate it is fine to do in the dowload plugin, that is part of ensuring we have the payload14:36
abhishekkok14:36
jokke_now we should not just fail it if we don't have the original hashing algo available14:36
dansmithto be clear, I'm talking about this: https://github.com/openstack/glance/blob/a42fda92dc8fe160bea1dd4d22cb1135fab89022/glance/async_/flows/api_image_import.py#L802-L85814:36
alistarlejokke_ Yes but we don't have the reference hash, as we don't call glance-api in that step, only data downloading14:36
dansmiththere can be a flow.add(_get_metadata), followed by flow.add(_get_the_image)14:37
dansmithget the metadata and container/disk format, first, set on the image, then fetch the data into the image, then after this, inject_metadata would run and correct whatever is needed14:37
abhishekkfine with me14:38
alistarlefine with me also, and we don't fail if we don't have the right hashing algo14:39
jokke_works for me ... IIRC taskflow is fine with registering variables that tasks inbetween does not know about so it shouldn't even require any piping through for the tasks in between 14:40
dansmithI dunno, that seems bad to just quietly say "we couldn't verify this image".. why woudl we do that?14:40
dansmithlike saying "I couldn't find the key to verify the image signature, so I'm going to assume it's fine" ?14:40
abhishekklater seems better14:41
jokke_dansmith: as that's what we do with any hashes currently ... say with nova snapshots, we never get hash and we do not fail because it's not there14:41
dansmithjokke_: if there is no hash, then fine, but if there is...14:41
dansmithyou said "hash_algo" not "hash" above14:42
jokke_dansmith: if we don't have algorithm to replicate the calculation, say like with MD5 with modern security hardened systems, we still don't fail it14:42
dansmithregardless, this is the kind of detail that should be in the spec, where we can explain the decision on things like this, and justify why that is so we have it for the future14:42
abhishekkI think alistarle and pslestang got enough idea about what to donow14:43
abhishekks/donow/ do now14:43
dansmithjokke_: yeah, I don't think that's what we *should* do, but perhaps there's some "glance doesn't do this in other places anyway" detail that I'm mis-assuming14:43
dansmithabhishekk: agreed, and I would just say, it would be great to iterate on this quicker :)14:43
jokke_dansmith: so you're saying that we should never accept image because it's hash was calculated with something we cannot support?14:44
abhishekkSo I would suggest we can have updated spec before next week and we can discuss it for 5/10 minutes in our review party14:44
alistarleOk fine, regarding the glanceclient, we plan to do another spec dedicated to that right ?14:44
abhishekkif croelandt permits14:44
abhishekkalistarle, yes14:44
jokke_dansmith: and with that force the user to do it all manually, in which case we do not enforce it either14:44
dansmithjokke_: yeah, if an image has a hash value and we can't calculate that, we shouldn't just silently ignore it.. we should flag it as unverified, or reject unless there's a force, or require them to pre-calculate with a different algo14:45
abhishekk15 minutes left, I have one topic in Open discussion14:45
dansmithjokke_: but to be clear, I'm not arguing this specifically for the glance-download, I'm just talking about in general.. I'd like to see the discussion of the logic in the spec and do some pondering on it for this case14:46
dansmithabhishekk: good to move on, I can follow up with jokke_ in -glance after this about hashing generalities :)14:47
dansmithpdeore: ^14:47
abhishekkdansmith, cool14:48
jokke_I'm fine with that14:48
pdeoreok, lets move to open discussion14:48
pdeore#topic Open Discussion14:48
abhishekkthat is me14:48
abhishekktoday we noticed that our gate is broken (doc job) due to oslo.policy 3.12.0 which is released last night14:49
abhishekkI have reported a bug14:49
abhishekk#link https://bugs.launchpad.net/oslo.policy/+bug/197072514:49
abhishekkso can we blacklist the 3.12.0 version in our requirements.txt ?14:50
jokke_yes, I thought after the discussion at the morn that you had already done it14:50
abhishekknah, I didn't, wanted to make sure this is the right step14:50
dansmithI think you can, but I would check with people in -qa first14:51
jokke_yup, always in these kind of situations better keep gate going before someone else goes and puts the new version as dependency in their requirements14:51
jokke_When the gate is working, it generally much easier to find time for long term solutions14:52
abhishekkack, will submit the patch right away after the meeting14:52
jokke_and affects much less people that way14:52
abhishekkyep14:52
abhishekkthat's it from me14:53
abhishekkwill submit a patch after the meeting14:53
jokke_Is it just something we are doing in our docs job or are others broken as well?14:53
dansmithI saw it broken in another project as well,14:54
rosmaitaprobably broke cinder too because rajat put up the ptach to oslo.oplicy14:54
dansmithyeah14:54
jokke_ok, so if we're not even the only one, then for sure bl it on requirements and free up gate14:54
abhishekkack14:54
jokke_abhishekk: put the bug link into the commit message and ping me when the patch is up, please14:55
jokke_I ninja it in14:55
abhishekkyes14:55
abhishekkI will add related-bug tag14:55
pdeore5 min left, anyone has anything else to discuss?14:55
jokke_sounds good14:55
abhishekknothing from me14:55
pdeoreok, Thanks everyone for joining !!14:57
rosmaitajust catching up ... we will do review party during next week's meeting?14:57
jokke_thanks all14:57
pdeoreSee you all in review party!!14:57
abhishekkthank you14:57
abhishekkrosmaita, yes14:57
pdeoreyes, 14:57
rosmaitaty14:58
abhishekksame time as our weekly meeting14:58
rosmaitathat makes it easier to schedule!14:58
abhishekkyes14:58
pdeore#endmeeting14:59
opendevmeetMeeting ended Thu Apr 28 14:59:32 2022 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)14:59
opendevmeetMinutes:        https://meetings.opendev.org/meetings/glance/2022/glance.2022-04-28-14.00.html14:59
opendevmeetMinutes (text): https://meetings.opendev.org/meetings/glance/2022/glance.2022-04-28-14.00.txt14:59
opendevmeetLog:            https://meetings.opendev.org/meetings/glance/2022/glance.2022-04-28-14.00.log.html14:59
*** dasm is now known as dasm|off20:32
sherwswe're closed23:44

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!