control/state: add name to rados.Rados object initialization

[adk3798]

If we want to use a keyring that isn't the client.admin keyring, we need to provide the name. The rados.Rados object defaults the name to client.admin, and if that doesn't match the provided keyring you end up hitting an error like

ERROR:control.state:Unable to create omap: [errno 13] RADOS permission denied (error connecting to the cluster). Exiting! INFO:control.server:Terminating SPDK...
INFO:control.server:Exiting the gateway process.
Traceback (most recent call last):
  File "/usr/lib64/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib64/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/src/control/__main__.py", line 35, in <module>
    gateway.serve()
  File "/src/control/server.py", line 99, in serve
    omap_state = OmapGatewayState(self.config)
  File "/src/control/state.py", line 178, in __init__
    conn.connect()
  File "rados.pyx", line 680, in rados.Rados.connect
rados.PermissionDeniedError: [errno 13] RADOS permission denied (error connecting to the cluster)

Setting the correct name gets around this and allows other keyrings with the correct name and permissions to be used

[adk3798]

NOTE: This is where the name is defaulted to client.admin in python librados if it is not provided. https://github.com/ceph/ceph/blob/main/src/pybind/rados/rados.pyx#L425-L426

[trociny]

Thank you for bringing this up. I am not sure though that "name" param from "gateway" section is a good choice for this. It is supposed to have different names for gateways in the same group (if "name" param is empty in the config then the hostname is used), and with your approach it would add a restriction to create a separate ceph user for every gateway. This may be not a big problem but I am not sure we want this restriction. Moreover the gateway name should be "client.XYZ" (this could be solved though by using rados_id param instead of name in your approach).

Instead of re-using the gateway name param I would prefer if we added "name" (or "id") param into "ceph" section and use it. Additionally it would be good to add "mon_host" and "keyring" (or "keyfile") params (which could be passed in "conf" arg when initializing the rados object). Then it would be possible to have setups without ceph.conf file, keeping all ceph related settings for the gateway clients in the ceph mon db.

[adk3798]

Thank you for bringing this up. I am not sure though that "name" param from "gateway" section is a good choice for this. It is supposed to have different names for gateways in the same group (if "name" param is empty in the config then the hostname is used), and with your approach it would add a restriction to create a separate ceph user for every gateway. This may be not a big problem but I am not sure we want this restriction. Moreover the gateway name should be "client.XYZ" (this could be solved though by using rados_id param instead of name in your approach).

Instead of re-using the gateway name param I would prefer if we added "name" (or "id") param into "ceph" section and use it. Additionally it would be good to add "mon_host" and "keyring" (or "keyfile") params (which could be passed in "conf" arg when initializing the rados object). Then it would be possible to have setups without ceph.conf file, keeping all ceph related settings for the gateway clients in the ceph mon db.

sure, if I want it to be something in the "ceph" section do I need to do anything special, or just change this to try and check the config for a "name" option in the "ceph" section (and probably have a default if it isn't specified)

[trociny]

sure, if I want it to be something in the "ceph" section do I need to do anything special, or just change this to try and check the config for a "name" option in the "ceph" section (and probably have a default if it isn't specified)

Yes, I would be fine with just changing self.config.get("gateway", "name") to self.config.get("ceph", "name") in your current version (and adding the new param to the example ceph-nvmeof.conf), though I would personally prefer "id" ("rados_id"), so one would need just to specify something like id = nvme instead of name = client.nvme in the config. And the empty (None) value for the default would be fine for me.

Though other people might have different thoughts...

[adk3798]

sure, if I want it to be something in the "ceph" section do I need to do anything special, or just change this to try and check the config for a "name" option in the "ceph" section (and probably have a default if it isn't specified)

Yes, I would be fine with just changing self.config.get("gateway", "name") to self.config.get("ceph", "name") in your current version (and adding the new param to the example ceph-nvmeof.conf), though I would personally prefer "id" ("rados_id"), so one would need just to specify something like id = nvme instead of name = client.nvme in the config. And the empty (None) value for the default would be fine for me.

Though other people might have different thoughts...

would the idea with id = nvme be that we add "client" to the front by default? We need this to match the name of the keyring we're using, which I expect to be formatted like "client.nvmeof." typically

[trociny]

would the idea with id = nvme be that we add "client" to the front by default? We need this to match the name of the keyring we're using, which I expect to be formatted like "client.nvmeof." typically

It is already done by the Rados python class, see the code you referred previously and how rados_id param is used there. I.e. instead of conn = rados.Rados(conffile=ceph_conf, name=name) you have in your code now you will have conn = rados.Rados(conffile=ceph_conf, rados_id=id)

[adk3798]

would the idea with id = nvme be that we add "client" to the front by default? We need this to match the name of the keyring we're using, which I expect to be formatted like "client.nvmeof." typically

It is already done by the Rados python class, see the code you referred previously and how rados_id param is used there. I.e. instead of conn = rados.Rados(conffile=ceph_conf, name=name) you have in your code now you will have conn = rados.Rados(conffile=ceph_conf, rados_id=id)

ah, I missed this. Alright, I think I understand what you mean now. I'll make the adjustments.

[adk3798]

would the idea with id = nvme be that we add "client" to the front by default? We need this to match the name of the keyring we're using, which I expect to be formatted like "client.nvmeof." typically

It is already done by the Rados python class, see the code you referred previously and how rados_id param is used there. I.e. instead of conn = rados.Rados(conffile=ceph_conf, name=name) you have in your code now you will have conn = rados.Rados(conffile=ceph_conf, rados_id=id)

ah, I missed this. Alright, I think I understand what you mean now. I'll make the adjustments.

now using rados_id over name

[trociny]

LGTM

[baum]

Thank you Adam 👍 LGTM