Tuesday, 2020-06-30

« Monday, 2020-06-29 Index Wednesday, 2020-07-01 »
*** Liang__ has joined #openstack-glance01:10
*** gyee has quit IRC02:04
*** coreycb has quit IRC02:36
*** mnasiadka has quit IRC02:36
*** coreycb has joined #openstack-glance02:38
*** mnasiadka has joined #openstack-glance02:38
*** rcernin has quit IRC03:38
*** rcernin has joined #openstack-glance03:46
*** udesale has joined #openstack-glance03:52
*** m75abrams has joined #openstack-glance04:07
*** evrardjp has quit IRC04:33
*** evrardjp has joined #openstack-glance04:33
*** ratailor has joined #openstack-glance05:00
*** rcernin has quit IRC07:07
*** rcernin has joined #openstack-glance07:08
*** bhagyashris|pto is now known as bhagyashris07:16
*** rcernin has quit IRC07:23
*** tosky has joined #openstack-glance07:28
*** k_mouza has joined #openstack-glance07:29
*** bhagyashris is now known as bhagyashris|lunc07:29
*** bhagyashris|lunc is now known as bhagyashris08:37
*** rchurch has quit IRC08:40
*** rchurch has joined #openstack-glance08:43
*** tkajinam has quit IRC08:55
*** priteau has joined #openstack-glance09:08
*** priteau has quit IRC09:54
*** Liang__ has quit IRC10:01
*** mugsie has quit IRC10:07
*** mugsie has joined #openstack-glance10:10
openstackgerritAbhishek Kekane proposed openstack/glance master: Fix race condition in copy image operation  https://review.opendev.org/73759610:21
openstackgerritAbhishek Kekane proposed openstack/glance master: Fix race condition in copy image operation  https://review.opendev.org/73759610:46
abhishekkdansmith, ^10:47
*** udesale_ has joined #openstack-glance11:04
*** udesale has quit IRC11:06
*** bhagyashris is now known as bhagyashris|brb11:32
*** gregwork has quit IRC12:00
*** mordred has quit IRC12:04
*** bhagyashris|brb is now known as bhagyashris12:10
*** mordred has joined #openstack-glance12:14
*** ratailor has quit IRC12:22
*** priteau has joined #openstack-glance12:45
openstackgerritSean McGinnis proposed openstack/glance master: Make test-setup.sh compatible with mysql8  https://review.opendev.org/73861513:16
*** jdillaman has joined #openstack-glance13:45
abhishekkjokke, o/13:57
*** m75abrams has left #openstack-glance14:03
dansmithabhishekk: are you going to flesh out the patch in the middle to fix the images/ tests?14:05
abhishekkdansmith, I guess I will add that one line change in my race condition patch14:07
dansmithinstead of just fix the tests?14:07
abhishekkwith TODO to revisit the FakeImage tests14:07
abhishekkdansmith, ?14:08
dansmithit would be better to not import that hack line, and instead replace id with image_id and fix any tests that pass id= to FakeImage() at least right?14:11
dansmithI'll just do if it if you're not going to14:11
abhishekkdansmith, will do that14:11
dansmith A comment on that FakeImage that it should be combined with a common fixture or something would be good too, but at least making it right would be an improvement14:12
dansmithokayt14:12
abhishekkit will not take much time to fix it, I will give it a try tonight and will post fully working patch by tomorrow14:13
dansmithokay14:14
dansmithgot the ack from smcginnis yesterday on the auth approach, still waiting for rosmaita before I go much further on that14:15
abhishekkack14:15
rosmaitadansmith: what's the review url?14:16
dansmithrosmaita: https://review.opendev.org/#/c/738103/14:16
rosmaitaty14:16
dansmithsubtle nudging is hard on IRC :)14:16
rosmaitathat was fairly subtle14:16
dansmithwell, un-subtle in that you get a ding when I say your name.. would be more subtle if I could walk by your cube and whisper "review mah patch" :D14:17
smcginnis:)14:17
rosmaitathat wouldn't be subtle, it would be creepy14:20
dansmithheh14:27
rosmaitadansmith: just want to make sure i understand the use-case -- we're going to introduce a policy to allow user A to copy an image owned by user B; B continues to own the image,and the problem is that A is not allowed to modify B's image to set the new location, so we need to elevate to an admin context to make that change and also to handle writing the progress-tracking image metadata14:41
jokkeabhishekk: \o14:42
abhishekkdansmith, jokke To use fixture instead of FakeImage will be lot of work, ATM I will change id to image_id in FakeImage with TODO comment and will submit another patch to get rid of FakeImage class14:44
dansmithrosmaita: among other limitations, yes that's the gist14:45
rosmaitaok, thanks14:45
dansmithrosmaita: in reality it'll be more like "allow users to copy public images to other stores so they can use them locally"14:45
dansmithi.e. images that aren't charged to anyone14:46
rosmaitayeah, that makes me feel better14:46
jokkeabhishekk: yeah, I'm fine with that as long as we're testing with something that at least looks like what would be coming from the code :P14:46
dansmithor on a cloud where things aren't charged at all14:46
dansmithabhishekk: yes, I just at least think that FakeImage needs fixing in the short term14:47
rosmaitayeah, the operator wants public images available, but doesn't want to pre-stage them in every possible location, so this functionality will allow end-users to stage them on-demand14:47
rosmaitathat sounds like a good idea, actually14:47
abhishekkdansmith, jokke ack14:47
*** k_mouza has quit IRC14:47
jokkerosmaita: and even more, it allows Nova to stage them on-demand when the user wants to boot something that is not available on the site it was requested14:48
dansmithright, that's the use-case here.. nova doing this to pre-optimize the location of the image when booting on a remote deployment with its own ceph cluster14:48
dansmithrosmaita: i.e. https://specs.openstack.org/openstack/nova-specs/specs/victoria/approved/rbd-glance-multistore.html14:48
rosmaitadansmith: ty, that is the missing link14:49
dansmithrosmaita: now when I walk by your cube, I'll just whisper ".....the missing link...."14:53
rosmaita:D14:53
*** k_mouza has joined #openstack-glance14:56
rosmaitadansmith: https://review.opendev.org/#/c/738103/1 looks like a good start14:59
dansmithrosmaita: cool, thanks15:00
openstackgerritAbhishek Kekane proposed openstack/glance master: Fix race condition in copy image operation  https://review.opendev.org/73759615:06
* abhishekk dinner break15:36
*** gyee has joined #openstack-glance15:38
* tosky use the break to remind people about https://review.opendev.org/#/c/733023/ - which will be backported to ussuri and then train once it gets merged15:50
*** udesale_ has quit IRC16:39
openstackgerritErno Kuvaja proposed openstack/glance master: Deprecation cleanout Registry and related  https://review.opendev.org/73867116:49
openstackgerritErno Kuvaja proposed openstack/glance master: Removal of 'enable_v2_api'  https://review.opendev.org/73867216:49
openstackgerritErno Kuvaja proposed openstack/glance master: Cleanup remove api v1 and registry code  https://review.opendev.org/73867316:49
openstackgerritErno Kuvaja proposed openstack/glance master: Update sample configs post deprecation removals  https://review.opendev.org/73867416:49
jokkerosmaita, smcginnis, abhishekk: ^^ ;)16:50
openstackgerritErno Kuvaja proposed openstack/glance master: Don't include plugins on 'copy-image' import  https://review.opendev.org/73867516:50
jokkeabhishekk, dansmith: ^^16:50
dansmithah cool16:51
abhishekkjokke, ack16:52
dansmithabhishekk: jokke: is there an example of running a whole flow in the unit tests I can look at? I haven't gone looking, but figure you can point me if there is or warn me if it's hard for some reason17:03
toskyif it's more complicated than testing a method or so, shouldn't that be a functional test?17:04
abhishekk dansmith AFAIK, there is no test which asserts the  entire import flow17:05
jokkedansmith: tosky: yeah I'd assume we haven't even tried to get that under unittests ... would expect mocking about the whole namespace for that17:05
abhishekktosky, we have functional tests for copy image operation and import image in multiple stores17:05
dansmithoh is there a functional test for it?17:06
abhishekkdansmith, we have functional tests for copy image operation and import image in multiple stores17:06
toskyI just know there are functional tests - my comment was more about the proper place for this kind of test17:06
* tosky is just passing by17:06
dansmithabhishekk: can you point me? I don't see anything "import" or "copy" related in the functional filename list at least17:07
abhishekkbut to add functional tests for conversion or decompression plugin we actually need to download qcow2 or compressed image which is difficult17:07
abhishekkdansmith, ack, give me a minute17:08
dansmithyeah mocking out the actual operations is fine, I just wanted to run the whole flow with suitable mocks17:08
dansmithI already found things that aren't covered in the unit tests because I broke them and didn't get a failure17:08
dansmithin the import flow17:09
openstackgerritDan Smith proposed openstack/glance master: WIP: Admin import action wrapper  https://review.opendev.org/73810317:10
dansmithabhishekk: before you disappear for the day, can you skim this fleshed-out import operation? https://review.opendev.org/#/c/738103/2/glance/async_/flows/api_image_import.py17:11
dansmithI haven't tested it other than to make sure the existing import unit tests work,17:11
dansmithbut just thought I'd get a quick approval of all the things I have to change in the import flow (or at least, that I think I'll need to change)17:12
abhishekkdansmith, ack17:13
abhishekkdansmith, http://paste.openstack.org/show/795392/17:14
dansmithah okay17:15
abhishekkthese are the functional tests we have around import operations17:15
dansmiththanks17:15
abhishekkIf you have a look around test, https://github.com/openstack/glance/blob/master/glance/tests/functional/v2/test_images.py#L540217:16
abhishekkthis fails sometimes due to race condition, need more specific logic17:16
dansmiththe whole import process is full of race conditions, although I'm not sure if or which of those would manifest in functional tests17:17
*** k_mouza has quit IRC17:18
dansmithhmm, functional tests seem to try to create things in my real system var:17:23
dansmith2020-06-30 10:18:27.316 5298 ERROR glance_store._drivers.filesystem [-] Unable to create datadir: /var/lib/glance/os_glance_tasks_store: PermissionError: [Errno 13] Permission denied: '/var/lib/glance'17:23
dansmithis that going to cause the test to fail?17:24
jokkedansmith: did not fail on that in my system and this user has no permissions to do that either17:26
dansmithyeah, I backed up to master and it does pass.. not sure why, but.. :)17:27
abhishekkdansmith, actionwrapper patch looks good, though we might have another bug which we need to fix or take care in your patch17:29
abhishekkhttps://review.opendev.org/#/c/738103/2/glance/async_/flows/api_image_import.py@23817:29
abhishekkHere we also need to set image-status as queued (IMO), if all_stores_must_succeed is False and and we are importing image in 5 stores, as soon as image is uploaded to 1st store image status will be set to active17:30
dansmithokay, IMHO that should be fixed before this patch so that this is just moving stuff and not fixing things in the process17:32
abhishekknow imagine if this operation fails while importing image to any other store, it will go to above line, remove image locations from all stores, removes checksum, hashing, size etc but image status will still remian active17:32
dansmithyep17:32
abhishekkalso at some places we pass from_state attribute to image_repo.save method which needs to be added in your action class method as well17:37
dansmithabhishekk: I'm always asserting the image state hasn't changed in every case.. certainly that's desirable right?17:37
dansmithsee L10317:37
abhishekkaha :)17:38
dansmithI guess maybe we need to stash the previous state though, for cases where the state is changing as part of the import17:38
dansmithhaven't gotten that far yet17:38
abhishekkprobably17:41
dansmithabhishekk: so, with a couple tweaks since the version you're looking at, all the functional tests pass unchanged17:43
dansmithwhich hopefully means that all works and hasn't broken anything17:43
abhishekkgreat17:44
jokkedansmith: sounds super promising at least17:49
dansmithhopefully17:52
abhishekkdansmith, are you planning to push new revision or after some time?17:54
dansmithabhishekk: after I flesh out tests17:54
abhishekkdansmith, ack17:54
dansmithnot that it matters, but while I'm thinking of it.. I'll be out this Friday for our annual try-to-blow-ourselves-up celebration,17:56
dansmithso will try to have as much up before then, but will be out Friday17:56
abhishekkdansmith, ack, happy holidays17:56
* abhishekk signing out for the day18:06
*** jmlowe has quit IRC18:19
-openstackstatus- NOTICE: Due to a flood of connections from random prefixes, we have temporarily blocked all AS4837 (China Unicom) source addresses from access to the Git service at opendev.org while we investigate further options.18:22
*** jmlowe has joined #openstack-glance18:25
*** ianw_pto is now known as ianw19:04
dansmithjokke: is this going to prevent me from functional'y testing with a different tenant than owns an image?19:34
dansmith        self.api_server.deployment_flavor = 'noauth'19:34
dansmithI would expect that to just not validate the token but still get the tenant from the headers,19:35
jokkedansmith: let me look into it. I can't recall using/touching that for years19:36
dansmithbut X-Tenant-Id in the headers seems to be ignored19:36
dansmiththere are tests that try to do things against a shared image that seem to honor that header...19:39
dansmithah19:42
dansmithI think there must be some magic19:42
dansmithhttps://github.com/openstack/glance/blob/master/glance/tests/functional/v2/test_images.py#L3947-L395019:42
openstackgerritErno Kuvaja proposed openstack/glance master: Deprecation cleanout Registry and related  https://review.opendev.org/73867119:43
openstackgerritErno Kuvaja proposed openstack/glance master: Removal of 'enable_v2_api'  https://review.opendev.org/73867219:43
openstackgerritErno Kuvaja proposed openstack/glance master: Cleanup remove api v1 and registry code  https://review.opendev.org/73867319:43
openstackgerritErno Kuvaja proposed openstack/glance master: Update sample configs post deprecation removals  https://review.opendev.org/73867419:43
dansmithokay yeah, I got it19:50
jokkecool20:02
openstackgerritMerged openstack/glance master: Use grenade-multinode instead of the custom legacy job  https://review.opendev.org/73302320:30
openstackgerritLuigi Toscano proposed openstack/glance stable/ussuri: Use grenade-multinode instead of the custom legacy job  https://review.opendev.org/73869320:40
*** priteau has quit IRC20:52
openstackgerritChristopher Stone proposed openstack/glance_store master: [WIP] Return endpoint set in swift_store_endpoint for single tenant  https://review.opendev.org/73870021:32
openstackgerritChristopher Stone proposed openstack/glance_store master: [WIP] Return endpoint set in swift_store_endpoint for single tenant  https://review.opendev.org/73870021:35
openstackgerritChristopher Stone proposed openstack/glance_store master: [DNM] Use swift_store_endpoint if set with Keystone v3  https://review.opendev.org/73870021:39
openstackgerritChristopher Stone proposed openstack/glance_store master: [DNM] Use swift_store_endpoint if set with Keystone v3  https://review.opendev.org/73870021:43
openstackgerritDan Smith proposed openstack/glance master: Check authorization before import for image  https://review.opendev.org/73754821:44
openstackgerritDan Smith proposed openstack/glance master: Add a test to replicate the owner-required behavior of copy-image  https://review.opendev.org/73810121:44
openstackgerritDan Smith proposed openstack/glance master: WIP: Refactor TaskFactory and Executor to take an admin ImageRepo  https://review.opendev.org/73810221:44
openstackgerritDan Smith proposed openstack/glance master: Make import task capable of running as admin on behalf of user  https://review.opendev.org/73810321:44
openstackgerritDan Smith proposed openstack/glance master: Refactor common auth token code in images test  https://review.opendev.org/73870121:44
openstackgerritDan Smith proposed openstack/glance master: Add a functional test for non-owned image copying  https://review.opendev.org/73870221:44
openstackgerritDan Smith proposed openstack/glance master: Add a policy knob for allowing non-owned image copying  https://review.opendev.org/73870321:44
dansmithkaboom21:44
dansmithrosmaita: gerrit decided to rebase this for me, but it and the next patch are unchanged: https://review.opendev.org/#/c/737548/21:48
dansmithif you have a sec to re-+W it21:49
rosmaitasure21:50
dansmiththanks21:50
openstackgerritChristopher Stone proposed openstack/glance_store master: [DNM] Use swift_store_endpoint if set with Keystone v3  https://review.opendev.org/73870421:51
*** rcernin has joined #openstack-glance22:38
*** tkajinam has joined #openstack-glance22:48
*** rcernin has quit IRC22:48
*** rcernin has joined #openstack-glance22:49
*** tosky has quit IRC22:57
openstackgerritChristopher Stone proposed openstack/glance_store master: [DNM] Use swift_store_endpoint if set with Keystone v3  https://review.opendev.org/73870423:31
openstackgerritChristopher Stone proposed openstack/glance_store master: [DNM] Use swift_store_endpoint if set with Keystone v3  https://review.opendev.org/73870423:59
« Monday, 2020-06-29 Index Wednesday, 2020-07-01 »

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