| opendevreview | Rajat Dhasmana proposed openstack/glance stable/yoga: Disable import workflow in glance cinder jobs https://review.opendev.org/c/openstack/glance/+/841747 | 04:17 |
|---|---|---|
| opendevreview | Rajat Dhasmana proposed openstack/glance master: Override GLANCE_USE_IMPORT_WORKFLOW in cinder jobs https://review.opendev.org/c/openstack/glance/+/841837 | 07:05 |
| *** abhishekk is now known as akekane|home | 08:31 | |
| *** akekane|home is now known as abhishekk | 08:31 | |
| opendevreview | Rajat Dhasmana proposed openstack/glance master: Override GLANCE_USE_IMPORT_WORKFLOW in cinder jobs https://review.opendev.org/c/openstack/glance/+/841837 | 08:31 |
| opendevreview | Merged openstack/glance-tempest-plugin master: Implement API protection testing for metadef resource types https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802792 | 09:40 |
| opendevreview | Merged openstack/glance-tempest-plugin master: Implement API protection testing for metadef properties https://review.opendev.org/c/openstack/glance-tempest-plugin/+/802794 | 09:41 |
| opendevreview | Rajat Dhasmana proposed openstack/glance master: Override GLANCE_USE_IMPORT_WORKFLOW in cinder jobs https://review.opendev.org/c/openstack/glance/+/841837 | 10:38 |
| *** whoami-rajat__ is now known as whoami-rajat | 10:40 | |
| dansmith | abhishekk: I can work on a fix | 14:47 |
| abhishekk | dansmith, ack, thank you! | 14:47 |
| abhishekk | (but if you have any important work in hand then I can proceed after 30 mins) | 14:48 |
| opendevreview | Dan Smith proposed openstack/glance-tempest-plugin master: Fix list namespaces test for parallelism https://review.opendev.org/c/openstack/glance-tempest-plugin/+/841910 | 15:00 |
| dansmith | abhishekk: untested ^ because I don't have a stack running right now, but I think it'll work | 15:00 |
| abhishekk | that was quicker than expected | 15:01 |
| abhishekk | dansmith, I think you need to modify expected to remove those which are not included in actual? | 15:05 |
| abhishekk | I will try to test it in local env | 15:06 |
| dansmith | oh, actual and expected are reversed in normal meaning | 15:06 |
| abhishekk | yeah | 15:07 |
| abhishekk | https://paste.opendev.org/show/bGtZqY5c1gqU7DP6MpYp/ | 15:07 |
| opendevreview | Dan Smith proposed openstack/glance-tempest-plugin master: Fix list namespaces test for parallelism https://review.opendev.org/c/openstack/glance-tempest-plugin/+/841910 | 15:08 |
| dansmith | I'm not sure what that paste is, but ^ just uses the other name, that should work right? | 15:09 |
| dansmith | I'm in a call now, but I'll try to get a stack up so I can test | 15:09 |
| abhishekk | I will try to test it | 15:09 |
| dansmith | ack | 15:10 |
| whoami-rajat | abhishekk, hey, the rbac job on glance gate is failing consistently and specifically the test_list_namespaces test https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_46a/841837/3/check/glance-secure-rbac-protection-functional/46acdec/testr_results.html | 15:31 |
| whoami-rajat | maybe this is a fix for that https://review.opendev.org/c/openstack/glance-tempest-plugin/+/841910 | 15:32 |
| abhishekk | dansmith, failing with, line #44, TypeError: string indices must be integers | 15:50 |
| abhishekk | also after fixing it, still failing with 404 not found | 15:52 |
| dansmith | abhishekk: ack, just got off a call, have to tend to something urgent and then I'll poke at it locally | 16:04 |
| abhishekk | it does work with, https://paste.opendev.org/show/bV7M43td0jRaVRS0Drl2/ | 16:04 |
| abhishekk | sure | 16:04 |
| abhishekk | take your time | 16:04 |
| dansmith | abhishekk: that's not a full diff so I can't even tell where you're applying that.. to assertListNamespaces? | 16:08 |
| abhishekk | yes | 16:08 |
| abhishekk | sorry for that | 16:08 |
| dansmith | that's on top of my patch? | 16:08 |
| dansmith | or instead? | 16:09 |
| abhishekk | instead | 16:13 |
| abhishekk | in your patch you are just modifying for project_ns | 16:13 |
| abhishekk | we are also creating namespace for alternate_project called as alt_ns | 16:14 |
| dansmith | okay, but there you're just filtering both lists so you're only checking the union of the two, | 16:14 |
| dansmith | so I was going for a more "any of them that look like they belong to us should be checked | 16:14 |
| dansmith | although maybe that's bad in case a namespace is left from a previous run or someone has a legitimately-named pre-existing ns | 16:15 |
| dansmith | abhishekk: if you want your approach, push it up and we can review in context | 16:15 |
| abhishekk | can I push it to your PS? | 16:15 |
| dansmith | sure | 16:15 |
| abhishekk | cool | 16:15 |
| opendevreview | Abhishek Kekane proposed openstack/glance-tempest-plugin master: Fix list namespaces test for parallelism https://review.opendev.org/c/openstack/glance-tempest-plugin/+/841910 | 16:36 |
| dansmith | okay I'm confused now | 16:46 |
| dansmith | the 404 is actually coming from the list call itself | 16:46 |
| dansmith | nothing to do with our assert loop right? | 16:47 |
| dansmith | and since we're just checking that "namespace is in resp" we shouldn't care about extra namespaces that are shown | 16:47 |
| dansmith | because I don't think your list filtering does anything.. it removes things we didn't create from expected_ns, but...we don't assert anything about those namespaces, just that the ones we created are in there | 16:48 |
| dansmith | and we're 404ing in our list: | 16:48 |
| dansmith | resp = self.do_request('list_namespaces', | 16:48 |
| dansmith | maybe this is a bug in the server? | 16:49 |
| dansmith | like maybe internally list is returning all, and then we try to do a get on each one to have the details, one is deleted by another test, and we 404? | 16:49 |
| dansmith | we're catching NotFound in the list and returning HTTPNotFound, | 16:52 |
| dansmith | but that makes no sense right? | 16:52 |
| dansmith | NotFound for a list operation should mean "you got the wrong url" | 16:52 |
| dansmith | https://github.com/openstack/glance/blob/d7fa7a0321ea5a56ec130aa0bd346749459ccaf2/glance/api/v2/metadef_namespaces.py#L124-L125 | 16:53 |
| abhishekk | I have just tested that in one execution I am getting 25 namespaces in response | 16:57 |
| abhishekk | That comment might make sense | 16:58 |
| abhishekk | We are creating 4 namespaces for each test, 2 public and private for project A and 2 for project B | 16:58 |
| abhishekk | the test never fails for admin | 16:59 |
| dansmith | okay, but.. do you see my point? we're getting a 404 from the server during list.. | 17:00 |
| abhishekk | For Member and Reader execution, 1 private namespace of other project will not be accessible and it is raising 404 | 17:00 |
| dansmith | the client (the test) isn't doing anything wrong | 17:00 |
| abhishekk | 403 wrapped with 404 | 17:00 |
| abhishekk | yes | 17:01 |
| dansmith | oh, permission problem underneath you mean | 17:01 |
| dansmith | that makes sense because I don't see where we're doing any db ops other than the list in the api | 17:01 |
| abhishekk | yes | 17:02 |
| dansmith | ack, but either way, the test is okay and is uncovering a legit server bug I think | 17:02 |
| dansmith | the lock or running single-threaded just covers it up | 17:02 |
| abhishekk | right | 17:03 |
| dansmith | can we repro this in a deterministic test that creates things that will trigger a 403? that way we can make sure this doesn't regress | 17:03 |
| dansmith | maybe with alt_persona | 17:05 |
| abhishekk | how it will trigger 403 since we converting it to 404 at server side | 17:05 |
| dansmith | no, I mean reproduce the behavior (the 404 for 403 reasons) | 17:05 |
| abhishekk | ok | 17:05 |
| abhishekk | let me try it | 17:05 |
| abhishekk | it is failing for Admin as well :/ | 17:10 |
| dansmith | hrm, doesn't that go against the assertion that it's a 403 under the covers? | 17:10 |
| abhishekk | yes | 17:13 |
| dansmith | abhishekk: maybe we should revert (or skip so pranli doesn't kill us) and open a bug since it's a server problem anyway? | 17:14 |
| dansmith | *pranali | 17:14 |
| abhishekk | I think lets skip 3 list tests at the moment | 17:15 |
| dansmith | did you get a test that repros? or are you testing manually? | 17:16 |
| abhishekk | I am testing manually | 17:18 |
| dansmith | okay | 17:18 |
| abhishekk | https://paste.opendev.org/show/bUYhj2mYlSLS9VGvluuQ/ | 17:20 |
| abhishekk | this is the log of failing reader list namespace test | 17:20 |
| abhishekk | and here you can see it is asserting for public namespace of Admin test class | 17:21 |
| abhishekk | which might already have been deleted | 17:21 |
| abhishekk | might >> surely | 17:21 |
| dansmith | yep | 17:21 |
| dansmith | you're running in parallel there though, how do you know surely? | 17:22 |
| abhishekk | because before this test, {3} glance_tempest_plugin.tests.rbac.v2.metadefs.test_objects.ProjectAdminTests.test_update_object [1.159995s] ... ok | 17:22 |
| abhishekk | it was executed | 17:22 |
| dansmith | sure but you don't know when the cleanup was run, since those are deleted during addCleanup right? | 17:23 |
| abhishekk | yes, I think cleanup runs right after test exits | 17:24 |
| dansmith | well, maybe that's synchronous with the test anyway, | 17:24 |
| dansmith | but still, I don't think you can say when the actual test is running and failed, | 17:24 |
| dansmith | because the test runner is collecting results from threads | 17:24 |
| abhishekk | hmm | 17:25 |
| dansmith | but regardless, | 17:25 |
| dansmith | I think you should hack your glance to raise on NotFound so you can see what the underlying error was | 17:25 |
| dansmith | i.e. disable the handler from my github link above and then re-repro | 17:25 |
| abhishekk | ok | 17:26 |
| dansmith | and if you still get 404, then something underneath is raising the HTTPNotFound, which could be api_policy | 17:26 |
| abhishekk | 500 | 17:28 |
| dansmith | right | 17:28 |
| dansmith | because it's not caught, but the log should have the deets now right? | 17:28 |
| abhishekk | https://paste.opendev.org/show/bpYXn0KCAFIxt0NDJSZR/ | 17:29 |
| dansmith | the glance log | 17:30 |
| abhishekk | ack | 17:30 |
| abhishekk | https://paste.opendev.org/show/b6aSzdafOJOLbH7Fggrp/ | 17:32 |
| abhishekk | this is expected,right | 17:33 |
| dansmith | isn't this what I was theorizing earlier? | 17:37 |
| dansmith | we do a list, and then we do a get() on each item, | 17:37 |
| dansmith | the list returns all, but the get() returns notfound because we should not be able to get each one? | 17:37 |
| abhishekk | yes | 17:37 |
| dansmith | i.e. db enforcing policy | 17:37 |
| abhishekk | so in test we should wrap this in NotFound try catch and ignore the error with comment? | 17:38 |
| dansmith | no, because the test is legitimately doing a list and expecting to only see its own resources, | 17:38 |
| dansmith | but the server is erroneously returning NotFound (which you should never get from a list operation) because it's trying to show some resources you don't own | 17:39 |
| dansmith | oh, sorry, hang on | 17:40 |
| dansmith | the server is actually hitting the deleted namespace, not the permission issue right? | 17:41 |
| abhishekk | yes | 17:41 |
| dansmith | L113 is the actual "no result" from the db when getting by name | 17:41 |
| abhishekk | which is created by other test | 17:41 |
| dansmith | so the server needs to ignore that case, and not error the whole list operation | 17:41 |
| abhishekk | yeah, | 17:41 |
| abhishekk | I think that is the right solution | 17:42 |
| abhishekk | *approach | 17:42 |
| abhishekk | I think you will add better note than me :D | 17:42 |
| dansmith | oh and this is not even erroring on namespaces, but on the rs_type | 17:44 |
| abhishekk | lance.common.exception.MetadefNamespaceNotFound: Metadata definition namespace=b7761601321344e4a80c392673953d9e_public_ProjectAdminTests was not found. | 17:46 |
| abhishekk | May 16 17:21:31 akekane devstack@g-api.service[3692]: ERROR | 17:46 |
| abhishekk | where do you see rs_type :o | 17:46 |
| dansmith | May 16 17:21:31 akekane devstack@g-api.service[3692]: ERROR glance.common.wsgi File "/opt/stack/glance/glance/api/v2/metadef_namespaces.py", line 101, in index | 17:47 |
| dansmith | May 16 17:21:31 akekane devstack@g-api.service[3692]: ERROR glance.common.wsgi repo_rs_type_list = rs_repo.list(filters=filters) | 17:47 |
| abhishekk | yes | 17:47 |
| abhishekk | and from there inside it is calling get for namespace | 17:47 |
| dansmith | the rs_type listing attempts to fetch the namespace (again) and that's what is failing | 17:48 |
| dansmith | right | 17:48 |
| opendevreview | Dan Smith proposed openstack/glance master: WIP: Fix failing namespace list for rs_type https://review.opendev.org/c/openstack/glance/+/841970 | 17:48 |
| dansmith | abhishekk: try this ^ ? | 17:48 |
| abhishekk | ack | 17:48 |
| abhishekk | it helped | 17:52 |
| dansmith | ...bug? | 17:53 |
| dansmith | er ...but? | 17:53 |
| abhishekk | yeah, bug | 17:54 |
| abhishekk | this is getting more and more confusing | 17:55 |
| dansmith | abhishekk: ? | 18:12 |
| abhishekk | around | 18:12 |
| dansmith | what is "more and more confusing" ? did it fix it or is something else broken? | 18:13 |
| abhishekk | the question is why it is failing only while filtering rs_types (might be we are not calling and not at line 90 | 18:14 |
| abhishekk | it fixes it | 18:14 |
| dansmith | I assume it's because we've pulled the list, a namespace gets deleted, and then we try to iterate that list and do get() on each one right? | 18:15 |
| abhishekk | yes | 18:15 |
| abhishekk | sorted | 18:16 |
| abhishekk | I have ran WIP continuously (more than 100) and it not failing now | 18:17 |
| dansmith | okay about to push with test | 18:17 |
| abhishekk | have you reported a bug? | 18:18 |
| abhishekk | may be we should backport it as well | 18:18 |
| dansmith | no, can you while I finish this? | 18:18 |
| abhishekk | ack | 18:19 |
| dansmith | if you need to go I can do it | 18:19 |
| abhishekk | just a minute | 18:19 |
| abhishekk | https://bugs.launchpad.net/glance/+bug/1973631 | 18:23 |
| abhishekk | and for these many days I thought this issue is because we are running it parallel and not 1 by 1 :D | 18:24 |
| dansmith | ... that is why right? | 18:25 |
| opendevreview | Dan Smith proposed openstack/glance master: Fix failing namespace list delete race https://review.opendev.org/c/openstack/glance/+/841970 | 18:25 |
| abhishekk | yeah | 18:26 |
| abhishekk | nice test | 18:29 |
| dansmith | I dunno about nice, but.. it works :) | 18:31 |
| abhishekk | :D | 18:31 |
| abhishekk | I will keep watch from mobile | 18:31 |
| abhishekk | signing out for the day | 18:31 |
| dansmith | ack | 18:31 |
| abhishekk | good day o/~ | 18:31 |
| dansmith | o/ | 18:31 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!