Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mgr/cephadm: automatically configure dashboard <-> RGW connection #41590

Closed
wants to merge 1 commit into from

Conversation

liewegas
Copy link
Member

@liewegas liewegas commented May 29, 2021

Automatically configure the credential(s) for dashboard to talk to RGW.

We do not provide any endpoint information--we assume that dashboard can figure that out on its own.

Note that this path is triggered any time we add or remove an rgw daemon, which isn't quite right--what we really want is to trigger this change when teh first or last rgw daemon is deployed, and/or when the realm configuration is modified. Once a mgr/rgw module exists, we should probably migrate this code there.

  • configure credentials per-realm if multisite is enabled
  • trigger this update on upgrade

Fixes: https://tracker.ceph.com/issues/51302

@liewegas liewegas force-pushed the cephadm-rgw-dashboard branch 2 times, most recently from b235e8a to a69ea64 Compare June 1, 2021 17:22
@liewegas liewegas force-pushed the cephadm-rgw-dashboard branch 2 times, most recently from f83023b to 9f7e87f Compare June 2, 2021 23:21
Comment on lines +84 to +85
if self.mgr.need_connect_dashboard_rgw and self.mgr.config_dashboard:
self.mgr.need_connect_dashboard_rgw = False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not persistent, which means there is a race between MGR failover and making this connection. Feeling slightly uneasy, but I guess we can fix this later, if it turns out to be a problem

Comment on lines 968 to 989
def update_dashboard(what: str, value: str) -> None:
_, out, _ = self.mgr.check_mon_command({'prefix': f'dashboard get-{what}'})
if out.strip() != value:
if what.endswith('-key'):
self.mgr.check_mon_command(
{'prefix': f'dashboard set-{what}'},
inbuf=value
)
else:
self.mgr.check_mon_command({'prefix': f'dashboard set-{what}',
"value": value})
self.mgr.log.info(f'Updated dashboard {what}')

completion = self.mgr.list_daemons(daemon_type='rgw')
raise_if_exception(completion)
daemons = completion.result
if not daemons:
self.mgr.log.info('No remaining RGW daemons; disconnecting dashboard')
self.mgr.check_mon_command({'prefix': 'dashboard reset-rgw-api-host'})
self.mgr.check_mon_command({'prefix': 'dashboard reset-rgw-api-port'})
self.mgr.check_mon_command({'prefix': 'dashboard reset-rgw-api-scheme'})
return
Copy link
Member

@epuertat epuertat Jun 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alfonsomthd you'd be interested in reviewing this (I meant the whole file, not just this section, but my ability to drag & scroll is affected 🙈 ).

@liewegas, as @Daniel-Pivonka pointed out, there are 2 ways of feeding these params:

  • single RGW daemon deployment: where access key and secret key contain each a single plain value (key)
  • multi-daemon (zone, zg, realm) deployment: where a dict-like syntax needs to be used for each: api-access-key=$ echo -n "{'<daemon1.id>': '', '<daemon2.id>': '', ...}"

We'd really like to see this simplified, but we couldn't find a better way to locate and configure connection to multiple daemons.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the multisite case, do you need a separate keypair for each zone, and the daemon is any daemon for that zone? or one per realm? (I thought the user creds were all per-realm...)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liewegas Currently we need a dict-like syntax per daemon. Form the mgr service map we can see the zone/zonegroup of the daemon but not the realm (and without first creds we cannot consume the admin ops API), so for now the credentials are per-daemon.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#41739

I'm still confused about the limitation, though: what can/can't you see depending on which daemon you connect to? Is the issue that you only see the zonegroup? Or everything in the realm?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liewegas @alfonsomthd if any, credentials would be per zone and not per gateway. In the stable state all users will be synced from the main/primary zone. During its bootstrap, a zone would not have any users defined (it will only hold credentials to connect to the primary as a client).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yehudasa Does this mean that the Dashboard dropdown menu for selecting gateways should instead allow a user to select zones (not gateways, not realms) and then connect to the first daemon that matches the <zone_id> ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alfonsomthd I'm not sure what would make the most sense, maybe gateway selection can be a secondary and optional/advanced selection?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it seems like in most cases the important part is to select the realm? It should be rare that a specific zone needs to be selected?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liewegas A daemon cannot operate on a zone other than its zone, so we need to select a zone.
Regarding credentials, after orch. weekly meeting the agreement is to set credentials on a per-realm basis. In order to do that on Dashboard side (to identify the daemons that run on zones belonging to a certain realm), this is required:
#41739

@epuertat epuertat requested a review from alfonsomthd June 7, 2021 15:21
@liewegas liewegas changed the title mgr/cephadm: automatically configure dashboard <-> mgr connection mgr/cephadm: automatically configure dashboard <-> RGW connection Jun 15, 2021
@liewegas liewegas requested a review from a team as a code owner June 15, 2021 16:31
@liewegas liewegas requested review from avanthakkar and removed request for a team June 15, 2021 16:31
Automatically configure the dashboard creds to talk to RGW.

Fixes: https://tracker.ceph.com/issues/44605
Signed-off-by: Sage Weil <sage@newdream.net>
@liewegas
Copy link
Member Author

jenkins test make check

@github-actions
Copy link

github-actions bot commented Jul 2, 2021

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@liewegas
Copy link
Member Author

liewegas commented Jul 8, 2021

replaced by #42252

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants