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

Add support for Tenant ID #1097

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

kevin-bates
Copy link
Member

@kevin-bates kevin-bates commented May 26, 2022

Although Enterprise Gateway states it supports multi-tenancy prior to this change, any "tenant" listing active kernels via /api/kernels will see kernels corresponding to every tenant. As a result, the shutdown of one tenant would terminate the kernels of all tenants.

This pull request introduces tenant_id as a means for associating kernels to a given tenant and thereby adds support for multi-tenancy in a minimally viable way.

The GatewayClient object that resides within Jupyter Server will add support for the configuration of a tenant_id. When configured, the env stanza associated with a kernel's start request will include the tenant_id as a value to the env JUPYTER_GATEWAY_TENANT_ID. In addition, the list kernels call will include a query parameter specifying the same tenant_id. Older client applications or admin applications that do not specify a query parameter will see all kernels - just like today.

EG will use the tenant_id in the start request to manage a list of corresponding kernels. When the client requests the list of active kernels, it will use the kernel ids associated with the given tenant-id to filter the results.

If no tenant-id is specified in the start request, the UNIVERSAL_TENANT_ID will be used. This results in common functionality for both new and legacy applications. Applications not configuring a tenant-id will result in the same behavior seen today (which is really only viable for "single-tenant" installations).

Although I haven't created a PR for the GatewayClient work (waiting for this merge), its changes can be found here.

If tenant id is specified in the kernel start request's body via
tenant_id, or as a query parameter (?tenant_id) to GET /api/kernels,
it will be used to associate kernels with a tenant.  If no tenant ID is
given in the request body or as a query parameter the UNIVERSAL_TENANT_ID
will be used so that all managed kernels use the same logic.
@kevin-bates kevin-bates added this to the v3.0 milestone May 26, 2022
@rahul26goyal
Copy link
Contributor

rahul26goyal commented May 29, 2022

hi @kevin-bates
Can multiple users share a single notebook and work on it simultaneously if connected to the same jupyterlab?
If yes, then they would also share the same kernel?

@kevin-bates
Copy link
Member Author

I'm not sure how real time collaboration (rtc) is implemented at that level but I suspect the "sharing" is at the notebook file level and each user has their own kernel.
If they shared kernels then they'd need to share an EG and a tenant ID as well. Is that where your question is headed?

@lresende
Copy link
Member

lresende commented Jun 8, 2022

We already have things like KERNEL_USERNAME, which provides a sort of a human-readable tenant_id. What is the drawbacks of using something like that instead of a UUID tenant_id?

@kevin-bates
Copy link
Member Author

We already have things like KERNEL_USERNAME, which provides a sort of a human-readable tenant_id. What is the drawbacks of using something like that instead of a UUID tenant_id?

KERNEL_USERNAME is not unique enough - both when the jupyter server is multi-user, but also in order to distinguish between "alice" from two different tenants.

The UUID is not displayed anywhere, only (optionally) configured at the client (jupyter_server). If we went with anything like a "name", then that opens the door for requiring a registry, etc. so that uniqueness can be detected.

enterprise_gateway/services/kernels/handlers.py Outdated Show resolved Hide resolved
enterprise_gateway/services/kernels/handlers.py Outdated Show resolved Hide resolved
docs/source/contributors/system-architecture.md Outdated Show resolved Hide resolved
enterprise_gateway/services/kernels/remotemanager.py Outdated Show resolved Hide resolved
enterprise_gateway/services/kernels/remotemanager.py Outdated Show resolved Hide resolved
enterprise_gateway/services/kernels/remotemanager.py Outdated Show resolved Hide resolved
enterprise_gateway/services/kernels/handlers.py Outdated Show resolved Hide resolved
@kevin-bates kevin-bates requested a review from rahul26goyal June 13, 2022 21:27
@kevin-bates
Copy link
Member Author

@lresende, @rahul26goyal - are you available to review this PR?

@kevin-bates kevin-bates requested a review from akchinSTC July 22, 2022 15:22
@kevin-bates kevin-bates marked this pull request as draft July 25, 2022 21:41
@kevin-bates
Copy link
Member Author

I've moved this to draft. After talking with @lresende, we felt it might be best to hold off on this until we have more firm requirements - particularly with how the tenant identities are configured and managed.

We will revisit down the road as necessary.

@kevin-bates kevin-bates modified the milestones: v3.0, v2.6.0 Jul 25, 2022
@kevin-bates kevin-bates modified the milestones: v2.6.0, Future Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants