| opendevreview | Abhishek Kekane proposed openstack/glance-specs master: Policy layer refactoring https://review.opendev.org/c/openstack/glance-specs/+/796753 | 05:24 |
|---|---|---|
| *** akekane__ is now known as abhishekk | 08:04 | |
| opendevreview | Dan Smith proposed openstack/glance master: WIP: Check get_image(s) in the API https://review.opendev.org/c/openstack/glance/+/796067 | 13:36 |
| abhishekk | jokke_, rosmaita, dansmith, smcginnis glance weekly meeting in 10 minutes at #openstack-meeting | 13:48 |
| dansmith | abhishekk: the secure tests are failing on listing public images because of that default policy oversight I mentioned: https://d05783eeba6f8fb45435-8e9a9f995b4a6012c53e1bcc573a9fc8.ssl.cf1.rackcdn.com/796067/2/check/glance-secure-rbac-protection-functional/715fa6c/testr_results.html | 13:48 |
| dansmith | abhishekk: so I tweaked that in the latest WIP patch to see if those will work after, | 13:49 |
| dansmith | but otherwise it's passing all those tests, which is surprising to me | 13:49 |
| abhishekk | ack | 13:49 |
| abhishekk | dansmith, mocking could be the reason? | 13:50 |
| dansmith | mocking? those are all running against tempest | 13:50 |
| dansmith | er, in tempest running against devstack | 13:51 |
| dansmith | those tests fail because I'm now being strict about get_images policy, and we need this change: https://review.opendev.org/c/openstack/glance/+/796067/3/glance/policies/image.py | 13:51 |
| dansmith | which allows public images that you don't own | 13:52 |
| abhishekk | ack, as we discussed day b4 | 13:52 |
| dansmith | yup | 13:53 |
| abhishekk | will come back to it after meeting | 13:55 |
| dansmith | abhishekk: it was just FYI, didn't mean action was necessary :) | 13:55 |
| abhishekk | yep, I am still gathering myself together :D | 13:56 |
| dansmith | ack | 13:57 |
| abhishekk | rosmaita, are you joining? | 14:01 |
| *** pdeore_ is now known as pdeore | 14:01 | |
| rosmaita | o/ | 14:02 |
| opendevreview | Dan Smith proposed openstack/glance master: Reproduce bug #1932337 https://review.opendev.org/c/openstack/glance/+/796885 | 15:52 |
| dansmith | abhishekk: whoami-rajat ^ | 15:52 |
| dansmith | found that during the policy stuff, but also, we need to move that lazy migration away from the authorization layer | 15:53 |
| dansmith | it's weird to have it there anyway, but the policy refactor removes the authorization layer from the stack which also means we no longer do the migration | 15:53 |
| dansmith | but I need to fix that bug first | 15:53 |
| abhishekk | dansmith, looking | 15:55 |
| opendevreview | Dan Smith proposed openstack/glance master: Reproduce bug #1932337 https://review.opendev.org/c/openstack/glance/+/796885 | 15:59 |
| opendevreview | Dan Smith proposed openstack/glance master: Fix failed cinder store migration for non-owners https://review.opendev.org/c/openstack/glance/+/796887 | 15:59 |
| dansmith | abhishekk: whoami-rajat that'll need backporting ^ | 15:59 |
| abhishekk | yes | 15:59 |
| dansmith | and has to go in front of the policy stuff for sure | 15:59 |
| abhishekk | I guess you have used .elevated ? | 16:00 |
| dansmith | no, I think the better way is to just delay | 16:00 |
| abhishekk | hmm | 16:01 |
| abhishekk | May be we need to update doc as well related to lazy migration | 16:01 |
| abhishekk | + reno | 16:01 |
| dansmith | ah, does the doc go into detail about this? | 16:02 |
| abhishekk | I think so | 16:02 |
| abhishekk | let me check | 16:02 |
| dansmith | the policy pile gets even deeper :/ | 16:02 |
| dansmith | not deeper in patches so much as just.. it's a deep pile of s**t to dig out | 16:03 |
| abhishekk | :/ | 16:04 |
| abhishekk | No detail about lazy loading in doc just a single line | 16:06 |
| abhishekk | GET call for image will take additional time as we will perform the lazy loading | 16:06 |
| dansmith | ack | 16:06 |
| abhishekk | operation to update legacy image location url to use new image location urls. | 16:06 |
| dansmith | so you want a reno for the master commit, or just the backport? | 16:06 |
| abhishekk | Master will be good | 16:07 |
| dansmith | aight | 16:07 |
| abhishekk | We need to record all efforts required for this policy work somewhere in the spec | 16:09 |
| abhishekk | dansmith, https://review.opendev.org/c/openstack/glance/+/796885/1/glance/tests/functional/v2/test_legacy_update_cinder_store.py | 16:11 |
| abhishekk | line #270 | 16:11 |
| abhishekk | you need to get the image before this line, right? | 16:11 |
| dansmith | I did, on L 259 | 16:12 |
| dansmith | see the next patch? | 16:12 |
| abhishekk | Something is wrong I guess | 16:13 |
| abhishekk | in PS 2 of same patch | 16:13 |
| abhishekk | line #269 to 271 is there by mistake? | 16:14 |
| dansmith | no, | 16:14 |
| dansmith | the next patch removes the return after the bug is fixed to show that the rest of the test continues to work as expected | 16:15 |
| dansmith | er, hmm, actually it shouldn't work without an extra get.. is that your point? | 16:15 |
| abhishekk | yes | 16:15 |
| dansmith | huh, yeah, okay not sure why it's passing now :D | 16:16 |
| abhishekk | strange is not new for our tests :D | 16:17 |
| dansmith | I wonder if it's because that wrapper modifies the image itself, | 16:18 |
| dansmith | so it's returning it migrated, but just can't save it | 16:18 |
| dansmith | yep, that's what is happening | 16:19 |
| dansmith | so it runs _update_cinder_location_and_store() on the image, then tries to save it, the save fails, but we continue to use image for the API return value, | 16:19 |
| dansmith | so the user sees it as migrated, it's just not saved in the DB that way | 16:20 |
| dansmith | you could argue that's good or bad I guess :) | 16:20 |
| abhishekk | :D | 16:20 |
| dansmith | to prevent that we have to make a copy of the image in update_store_in_locations() which is kinda meh | 16:22 |
| dansmith | or refactor that more substantially | 16:22 |
| abhishekk | later I think | 16:28 |
| dansmith | so leave it as I have it here? I can put a comment detailing why that test continues to work, at least | 16:30 |
| abhishekk | sounds good | 16:32 |
| opendevreview | Dan Smith proposed openstack/glance master: Reproduce bug #1932337 https://review.opendev.org/c/openstack/glance/+/796885 | 16:32 |
| opendevreview | Dan Smith proposed openstack/glance master: Fix failed cinder store migration for non-owners https://review.opendev.org/c/openstack/glance/+/796887 | 16:32 |
| *** akekane_ is now known as abhishekk | 16:44 | |
| * abhishekk facing frequent network issues | 16:45 | |
| * abhishekk signing out | 16:55 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!