*** dasm|off is now known as dasm | 13:10 | |
pdeore | #startmeeting glance | 14:00 |
---|---|---|
opendevmeet | Meeting 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 |
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 |
dansmith | o/ | 14:00 |
pdeore | lets wait few minutes for everyone to join | 14:00 |
alistarle | o/ | 14:02 |
rosmaita | o/ | 14:02 |
mrjoshi | o/ | 14:02 |
pdeore | ok let's start | 14:03 |
pdeore | #topic release/periodic jobs | 14:03 |
pdeore | Milestone 1 is just 3 weeks away | 14:03 |
pdeore | I think almost all the specs are up for review | 14:03 |
pdeore | we will go through the list in next topic | 14:04 |
pdeore | Periodic job, all green except POST_FAILURE for fips job and one time out | 14:04 |
pdeore | moving ahead | 14:04 |
pdeore | #topic Specs For Review | 14:04 |
pdeore | Most of the specs up for review which needs some attention | 14:04 |
pdeore | New API for instant caching of image : https://review.opendev.org/c/openstack/glance-specs/+/838642 | 14:05 |
pdeore | this one needs new revision | 14:05 |
pdeore | Expanding store details - https://review.opendev.org/c/openstack/glance-specs/+/835606 (one +2) | 14:05 |
pdeore | Delete API for metadef resource types - https://review.opendev.org/c/openstack/glance-specs/+/818192 | 14:05 |
abhishekk | For 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 |
dansmith | pdeore: I was mostly okay with the stores-detail one, | 14:05 |
dansmith | so if it has added detail now hopefully it's good, I'll do that after the meeting | 14:05 |
pdeore | cool | 14:06 |
pdeore | the 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 |
pdeore | s/rd/rs | 14:06 |
pdeore | Update proposal for duplicate image download - https://review.opendev.org/c/openstack/glance-specs/+/734683 | 14:07 |
pdeore | this one also needs to be updated | 14:07 |
pdeore | spec-lite: ability to purge all rows by glance-manage - https://review.opendev.org/c/openstack/glance-specs/+/819622 (one +2) | 14:07 |
abhishekk | this one is also sitting from last cycle | 14:07 |
pdeore | yeah | 14:08 |
pdeore | So all members, kindly review the spec and give your suggestions | 14:08 |
pdeore | moving ahead | 14:09 |
abhishekk | ack | 14:09 |
pdeore | #topic Secure RBAC | 14:09 |
dansmith | pdeore: where were you on the meeting??! | 14:09 |
pdeore | We 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 week | 14:09 |
dansmith | that was annoying.. meetpad-- | 14:09 |
dansmith | sorry you got bit by that :( | 14:10 |
pdeore | dansmith, I was there but i was not able to hear anything :/ | 14:10 |
dansmith | pdeore: I know, I'm poking fun :) | 14:10 |
abhishekk | :D | 14:10 |
pdeore | :D :D | 14:10 |
dansmith | rescheduled quickly for next week and using another tool, so hopefully will be better :) | 14:10 |
abhishekk | pdeore, as I added in etherpad we do need to have that validation for POST APIs in client as well as API side | 14:11 |
pdeore | yeah hopefully :) | 14:11 |
dansmith | the heat cross-scope thing actually affects glance I imagine | 14:11 |
pdeore | abhishekk, ack | 14:11 |
dansmith | in that anything we nail down to system scope may be a problem for heat users that currently create things as admin | 14:11 |
* jokke_ sneaks in | 14:12 | |
dansmith | "we" meaning glance | 14:12 |
abhishekk | oh | 14:12 |
pdeore | ohh ok | 14:12 |
abhishekk | and I guess they are going to work on that during this cycle | 14:12 |
dansmith | it'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 week | 14:12 |
dansmith | abhishekk: I don't know.. very little people working on heat these days, like horizon, who is also impacted | 14:13 |
abhishekk | ack, I will try to be there for next meeting | 14:13 |
abhishekk | I think then we are pretty much sorted out for RBAC | 14:13 |
dansmith | yeah | 14:13 |
pdeore | kindly please have a look at the glance policy matrix as well, #link https://review.opendev.org/c/openstack/glance/+/838857 | 14:14 |
abhishekk | ok,lets wait for the outcome of next meeting then | 14:14 |
pdeore | I hope I have not missed anything there | 14:14 |
abhishekk | pdeore, ack, have you updated it after our discussion? | 14:14 |
pdeore | yes | 14:14 |
abhishekk | cool,I will have a look today or tomorrow | 14:15 |
pdeore | abhishekk, Thanks ! | 14:15 |
abhishekk | we will shift next topic to last | 14:15 |
pdeore | ok lets move ahead | 14:15 |
abhishekk | as it will be lengthy discussion | 14:15 |
pdeore | sure | 14:16 |
pdeore | #topic Policy Refactor part 2 | 14:16 |
pdeore | As per discussion in PTG, the DNM patch has been submitted | 14:16 |
pdeore | #link https://review.opendev.org/c/openstack/glance/+/839491 | 14:16 |
abhishekk | yeah and as I stated functional tests are passing as well as no impact on tempest side | 14:16 |
jokke_ | ++ | 14:16 |
abhishekk | I skipped affected unittests so that you guys can get idea about it | 14:17 |
abhishekk | So once minimum two member will give go ahead on DNM patch I will start removing the code | 14:17 |
abhishekk | s/I/or pdeore | 14:18 |
dansmith | good stuff | 14:18 |
pdeore | let's move ahead | 14:18 |
pdeore | croelandt, 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 used | 14:18 |
jokke_ | so ++ on cleanup from me | 14:19 |
abhishekk | +1 | 14:19 |
croelandt | yes, as discussed during PTG, we'll have a review "party" next week | 14:19 |
croelandt | kindly add your name in the little list so we can schedule it :) | 14:19 |
abhishekk | I think we should utilize the meeting time for it | 14:19 |
croelandt | I'll put together a small (no) list of patches | 14:20 |
croelandt | abhishekk: works for me :) | 14:20 |
abhishekk | +1 | 14:20 |
pdeore | +1 | 14:20 |
croelandt | and I guess that's it, I'll promise it will be fun | 14:20 |
croelandt | for some definition of "fun" | 14:20 |
abhishekk | :D | 14:21 |
jokke_ | join review party, they say, it'll be fun, they say | 14:21 |
pdeore | :D :D | 14:21 |
jokke_ | So that means no meeting next week? | 14:21 |
pdeore | yup :) | 14:22 |
abhishekk | we will keep 10 minutes and discuss face to face if there is anything in agenda | 14:22 |
pdeore | please add your availability to time proposed by croelandt | 14:22 |
croelandt | jokke_: instead you get two hours of patch reviews! | 14:22 |
pdeore | ok, let's move ahead | 14:23 |
pdeore | #topic glance-download import method | 14:23 |
pdeore | New glance-download import method - https://review.opendev.org/c/openstack/glance-specs/+/836132 | 14:23 |
pdeore | new patch set is updated for this | 14:23 |
abhishekk | alistarle, pslestang do you have anything to add here | 14:23 |
pdeore | Expanding store details - https://review.opendev.org/c/openstack/glance-specs/+/835606 | 14:23 |
abhishekk | I guess the spec is still missing info about minimum copying of metadata | 14:24 |
abhishekk | do you guys have any difficulty about it? | 14:24 |
pdeore | oops, please ignore last link .. :/ | 14:24 |
alistarle | Oh we thought we should follow the jokke_ comment | 14:25 |
abhishekk | that is a comment not the decision,right? | 14:25 |
alistarle | But no issue we can update if we need | 14:25 |
alistarle | I understand we have to then, so we'll do that shortly | 14:26 |
abhishekk | So jokke is insisting that we should stick to the fundamental of design | 14:26 |
abhishekk | that is, glance-direct method should just download the data like web-download | 14:26 |
abhishekk | s/glance-download | 14:27 |
dansmith | alistarle: 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 |
abhishekk | So I have added one more suggestion to the spec | 14:27 |
abhishekk | Can we have two plugins for this solution | 14:27 |
abhishekk | 1. glance-download internal import method for importing the image from another glance | 14:27 |
abhishekk | 2. import-metadata external plugin like inject metadata for importing metadata from another glance | 14:27 |
abhishekk | And if glance-download method is enabled then it should have import-metadata enabled as well. | 14:27 |
alistarle | Yeah sorry, we sync up our side just before the meeting also | 14:27 |
pslestang | hello | 14:28 |
dansmith | two plugins means we have to hit the remote api more, probably less code-reuse, more opportunity to have it mis-configured | 14:28 |
dansmith | but if that's what we have to do then, it's better than noting | 14:28 |
abhishekk | I think this is better idea | 14:28 |
pslestang | abhishekk: if we do the import-metadata plugin we will have to be sure that it is run before the inject-metadat plugin | 14:28 |
abhishekk | we can work on second part if alistarle and pslestang finds it difficult | 14:28 |
dansmith | pslestang: right, easier to mis-configure | 14: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 import | 14:29 |
abhishekk | yeah, but we can document it | 14:29 |
alistarle | dansmith: agree with the mis-configuration possibility, that's what we thought also | 14:29 |
abhishekk | that will also do | 14:29 |
abhishekk | see my only concern is if the image is unusable then it will be additional overhead to admin to get it in usable state | 14:31 |
dansmith | right, copy-image has a ton of flow steps, | 14:31 |
dansmith | so 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 ttaskflow | 14:31 |
abhishekk | yep, like extra flow in glance-download flow | 14: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 available | 14:32 |
alistarle | Ok, but what about updating the disk_format and container_type, shouldn't we put that here also ? In this extra step ? | 14:32 |
dansmith | alistarle: I'm not sure why it wouldn't be the same step | 14:33 |
dansmith | alistarle: you've just fetched the remote metadata and are about to apply it to the local image | 14:33 |
alistarle | Ok the glance-download never call the glance-api, except for downlading the data itself of course | 14: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 is | 14:34 |
alistarle | call to remote glance-api to retrieve container, disk and metadata will be handled by the extra workflow step dedicated to that | 14:34 |
abhishekk | alistarle, 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 glance | 14:35 |
dansmith | maybe 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 |
abhishekk | sounds good | 14: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 payload | 14:36 |
abhishekk | ok | 14:36 |
jokke_ | now we should not just fail it if we don't have the original hashing algo available | 14:36 |
dansmith | to be clear, I'm talking about this: https://github.com/openstack/glance/blob/a42fda92dc8fe160bea1dd4d22cb1135fab89022/glance/async_/flows/api_image_import.py#L802-L858 | 14:36 |
alistarle | jokke_ Yes but we don't have the reference hash, as we don't call glance-api in that step, only data downloading | 14:36 |
dansmith | there can be a flow.add(_get_metadata), followed by flow.add(_get_the_image) | 14:37 |
dansmith | get 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 needed | 14:37 |
abhishekk | fine with me | 14:38 |
alistarle | fine with me also, and we don't fail if we don't have the right hashing algo | 14: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 |
dansmith | I dunno, that seems bad to just quietly say "we couldn't verify this image".. why woudl we do that? | 14:40 |
dansmith | like saying "I couldn't find the key to verify the image signature, so I'm going to assume it's fine" ? | 14:40 |
abhishekk | later seems better | 14: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 there | 14:41 |
dansmith | jokke_: if there is no hash, then fine, but if there is... | 14:41 |
dansmith | you said "hash_algo" not "hash" above | 14: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 it | 14:42 |
dansmith | regardless, 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 future | 14:42 |
abhishekk | I think alistarle and pslestang got enough idea about what to donow | 14:43 |
abhishekk | s/donow/ do now | 14:43 |
dansmith | jokke_: 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-assuming | 14:43 |
dansmith | abhishekk: 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 |
abhishekk | So I would suggest we can have updated spec before next week and we can discuss it for 5/10 minutes in our review party | 14:44 |
alistarle | Ok fine, regarding the glanceclient, we plan to do another spec dedicated to that right ? | 14:44 |
abhishekk | if croelandt permits | 14:44 |
abhishekk | alistarle, yes | 14:44 |
jokke_ | dansmith: and with that force the user to do it all manually, in which case we do not enforce it either | 14:44 |
dansmith | jokke_: 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 algo | 14:45 |
abhishekk | 15 minutes left, I have one topic in Open discussion | 14:45 |
dansmith | jokke_: 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 case | 14:46 |
dansmith | abhishekk: good to move on, I can follow up with jokke_ in -glance after this about hashing generalities :) | 14:47 |
dansmith | pdeore: ^ | 14:47 |
abhishekk | dansmith, cool | 14:48 |
jokke_ | I'm fine with that | 14:48 |
pdeore | ok, lets move to open discussion | 14:48 |
pdeore | #topic Open Discussion | 14:48 |
abhishekk | that is me | 14:48 |
abhishekk | today we noticed that our gate is broken (doc job) due to oslo.policy 3.12.0 which is released last night | 14:49 |
abhishekk | I have reported a bug | 14:49 |
abhishekk | #link https://bugs.launchpad.net/oslo.policy/+bug/1970725 | 14:49 |
abhishekk | so 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 it | 14:50 |
abhishekk | nah, I didn't, wanted to make sure this is the right step | 14:50 |
dansmith | I think you can, but I would check with people in -qa first | 14: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 requirements | 14:51 |
jokke_ | When the gate is working, it generally much easier to find time for long term solutions | 14:52 |
abhishekk | ack, will submit the patch right away after the meeting | 14:52 |
jokke_ | and affects much less people that way | 14:52 |
abhishekk | yep | 14:52 |
abhishekk | that's it from me | 14:53 |
abhishekk | will submit a patch after the meeting | 14:53 |
jokke_ | Is it just something we are doing in our docs job or are others broken as well? | 14:53 |
dansmith | I saw it broken in another project as well, | 14:54 |
rosmaita | probably broke cinder too because rajat put up the ptach to oslo.oplicy | 14:54 |
dansmith | yeah | 14:54 |
jokke_ | ok, so if we're not even the only one, then for sure bl it on requirements and free up gate | 14:54 |
abhishekk | ack | 14:54 |
jokke_ | abhishekk: put the bug link into the commit message and ping me when the patch is up, please | 14:55 |
jokke_ | I ninja it in | 14:55 |
abhishekk | yes | 14:55 |
abhishekk | I will add related-bug tag | 14:55 |
pdeore | 5 min left, anyone has anything else to discuss? | 14:55 |
jokke_ | sounds good | 14:55 |
abhishekk | nothing from me | 14:55 |
pdeore | ok, Thanks everyone for joining !! | 14:57 |
rosmaita | just catching up ... we will do review party during next week's meeting? | 14:57 |
jokke_ | thanks all | 14:57 |
pdeore | See you all in review party!! | 14:57 |
abhishekk | thank you | 14:57 |
abhishekk | rosmaita, yes | 14:57 |
pdeore | yes, | 14:57 |
rosmaita | ty | 14:58 |
abhishekk | same time as our weekly meeting | 14:58 |
rosmaita | that makes it easier to schedule! | 14:58 |
abhishekk | yes | 14:58 |
pdeore | #endmeeting | 14:59 |
opendevmeet | Meeting ended Thu Apr 28 14:59:32 2022 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 14:59 |
opendevmeet | Minutes: https://meetings.opendev.org/meetings/glance/2022/glance.2022-04-28-14.00.html | 14:59 |
opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/glance/2022/glance.2022-04-28-14.00.txt | 14:59 |
opendevmeet | Log: https://meetings.opendev.org/meetings/glance/2022/glance.2022-04-28-14.00.log.html | 14:59 |
*** dasm is now known as dasm|off | 20:32 | |
sherws | we're closed | 23:44 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!