| opendevreview | Sam Morrison proposed openstack/glance master: Fix typo in glance-api-paste https://review.opendev.org/c/openstack/glance/+/923911 | 00:58 |
|---|---|---|
| mnaser | elodilles: fyi Rico has been pushing up some patches from our side to try and fix zed tests | 09:55 |
| opendevreview | Sam Morrison proposed openstack/glance master: Fix /healthcheck url typo in glance-api-paste https://review.opendev.org/c/openstack/glance/+/923911 | 10:45 |
| elodilles | mnaser: thanks for the heads up and for working on this! | 10:57 |
| opendevreview | Pranali Deore proposed openstack/glance-specs master: [Spec] Add new Calculate Hash API https://review.opendev.org/c/openstack/glance-specs/+/922011 | 10:58 |
| mnaser | elodilles: https://review.opendev.org/c/openstack/glance/+/923922 .. I dont see Jay here but this looks good to me too and has a +1 from a glance core | 13:35 |
| pdeore | abhishekk, rosmaita, dansmith, croelandt, mrjoshi glance weekly meeting in 10 minutes at #openstack-meeting | 13:51 |
| opendevreview | Cyril Roelandt proposed openstack/glance master: Python3.12: do not use ssl.wrap_socket https://review.opendev.org/c/openstack/glance/+/922962 | 14:10 |
| abhishekk | croelandt: could you try ^^ in env by running glance-cache-manage and any command related to it? | 14:13 |
| croelandt | abhishekk: oh glacne-cache-manage is not tested through unit/functional tests? | 14:23 |
| abhishekk | I guess not | 14:25 |
| croelandt | ok, let me fire up devstack :) | 14:26 |
| abhishekk | do it with swift | 14:27 |
| abhishekk | I want you to test one more scenario | 14:27 |
| abhishekk | https://review.opendev.org/c/openstack/glance/+/920170 | 14:28 |
| abhishekk | What if only one swift store is configured with both above options in mentioned patch is enabled? | 14:29 |
| abhishekk | croelandt: can you remove your +w on above patch | 14:31 |
| croelandt | done | 14:32 |
| abhishekk | cool, thank you | 14:32 |
| abhishekk | it will not start glance service if single store is configured but error given is misleading, may be need to file another error | 15:09 |
| abhishekk | another bug I mean | 15:09 |
| croelandt | abhishekk: or maybe we treat having multitenant+config like an error | 15:22 |
| croelandt | fail early instead of silently ignoring that | 15:23 |
| croelandt | I think it will be easier to debug for sysadmins | 15:23 |
| abhishekk | agree | 15:23 |
| abhishekk | So you mean to say instead of adding this change here in glance we move to glance-store? | 15:23 |
| croelandt | hmm | 15:24 |
| abhishekk | It is already handled there, problem is here in stores-info we read from glance config and not from in memory store map | 15:24 |
| croelandt | this would be even better not to have store-specific code in Glance | 15:24 |
| croelandt | so we cannot fail in glance_store? | 15:25 |
| abhishekk | https://github.com/openstack/glance_store/blob/master/glance_store/_drivers/swift/store.py#L744 | 15:25 |
| abhishekk | We are failing in glance store and excluding that store if there are multiple stores | 15:25 |
| croelandt | q | 15:26 |
| abhishekk | But in case there is only one store then it fails to start the service which is expected but error message is misleading | 15:26 |
| croelandt | oops | 15:26 |
| abhishekk | So either we need to change the stores-info discovery API to cross check with in memory store map available rather that just returning values from config files | 15:27 |
| croelandt | should get_stores fail in glance/api/v2/discovery.py? | 15:27 |
| croelandt | like if backends == [] then we fail? | 15:28 |
| abhishekk | IN case of single store or multiple stores? | 15:28 |
| croelandt | In any case? If glance has no valid backends configure, is it worth starting the service? | 15:28 |
| abhishekk | If backends == [] then we should return 400 or 404 error I guess | 15:28 |
| croelandt | yes | 15:29 |
| abhishekk | IT does not start service if glance does not have any valid store configured | 15:29 |
| croelandt | ok | 15:29 |
| abhishekk | So stores-info discovery will return 503 error in that case | 15:29 |
| croelandt | so pdeore's patch seems fine, if we just raise 404 in case we found no backends? | 15:29 |
| abhishekk | Right | 15:30 |
| abhishekk | I think that is sufficient to alert admin to go and check what is wrong | 15:30 |
| abhishekk | 404 with Valid reason I will say!! | 15:30 |
| abhishekk | But will it come to there, that is the question | 15:31 |
| croelandt | because you think it might fail with BadStoreConfiguration before? | 15:31 |
| abhishekk | Because if none of the backed is configured correctly, service will not start, that means API will not be served | 15:31 |
| abhishekk | yes | 15:31 |
| croelandt | hm, interesting | 15:32 |
| abhishekk | We need to add log message in glance-store, we are just ignoring it there if there are multiple backends | 15:32 |
| abhishekk | https://github.com/openstack/glance_store/blob/master/glance_store/multi_backend.py#L200 | 15:33 |
| croelandt | that FIXME at line 189 | 15:34 |
| abhishekk | :D | 15:34 |
| croelandt | I don't know if we "just" warn or if we go "BAM! STOP! Configure this shit properly" | 15:34 |
| abhishekk | that is there since beginning | 15:34 |
| croelandt | yeah it's always like this :) | 15:34 |
| croelandt | I think maybe now we just warn | 15:34 |
| croelandt | but next cycle we might want to make it stricter | 15:35 |
| croelandt | Admins don't read logs when everything seems to work | 15:35 |
| abhishekk | In case of single store i.e. traditional single store (not multi) we should go Bam | 15:35 |
| abhishekk | May be good topic for PTG discussion | 15:36 |
| abhishekk | Could you please make a note of it? | 15:36 |
| croelandt | yeah for multistore it's not as simple | 15:36 |
| croelandt | but I still feel that if an admin configures 3 stores and 1 of them is misconfigured... | 15:36 |
| croelandt | they will probably want to know | 15:36 |
| croelandt | aren't we going to remove single store eventually? | 15:37 |
| abhishekk | right, that is why we should discuss this and take decision | 15:37 |
| croelandt | ok | 15:37 |
| abhishekk | that is also TODO since Train :P | 15:37 |
| croelandt | I'll add it to the Etherpad once Pranali has created it | 15:37 |
| abhishekk | ++ | 15:37 |
| abhishekk | thank you!! | 15:37 |
| abhishekk | Meantime I will report a bug tomorrow so that we can have it posted somewhere | 15:38 |
| abhishekk | You can reapprove the patch :D | 15:38 |
| croelandt | yeah it's still an improvement :) | 15:40 |
| abhishekk | ++ | 15:48 |
| opendevreview | Merged openstack/glance master: Fix 500 if multi-tenant swift is used https://review.opendev.org/c/openstack/glance/+/920170 | 22:55 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!