change {username}{servername} to {userid} to avoid volume length restrictions of 63 characters #1200
Conversation
|
/retest |
|
How is userid set? Could we add pod labels for the username (but truncate that to appropriate lengths) so its easy for people find their pods. |
|
i've kept the pod name as is which for IAP ends up being accounts.google:kam.d.kasravi@intel.com (where '.:@' are escaped). I've only changed the pvc name which is the attribute with the 63 character length description. The calculation of userid is a simple integer beginning from 1. So the pod has mount with names like volume-1, ... The userid is taken from the local sqllite db. Should tf-hub-0 die then these userid's are rebuilt. |
|
I could try labeling pods by using the email from IAP however it would still be escapified - it wouldn't have the accounts.google: prefix though. |
|
Sorry I missed that it was only the PVC. Don't we want to change the pod name template? I assume they will run into the same length issues. It looks like the pod name is Whereas volume name is I think pod names are DNS names So aren't they subject to the same 63 character limit? So I think we will eventually hit the same issue if we don't truncate those names as well. |
|
Good point on recognizing both as being identifiers. |
…x accounts.google: for iap and truncating escaped self.user.name to 63 characters
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
TFJob test is failing with That doesn't seem like it would be caused by this PR. Filed #1218 |
|
/retest |
|
@jlewi thanks - i didn't think so either |
kubeflow/core/kubeform_spawner.py
Outdated
| @@ -111,6 +113,33 @@ def get_env(self): | |||
| env['GOOGLE_APPLICATION_CREDENTIALS'] = '{}/{}.json'.format(SERVICE_ACCOUNT_SECRET_MOUNT, gcp_secret_name) | |||
| return env | |||
|
|
|||
| def _parse_user_name(self, username): | |||
There was a problem hiding this comment.
Can you add a TODO to add a unittest?
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jlewi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
this error about tfjobs is probably coming from the tf-operator repo |
|
/retest |
|
failure in junit_e2e-gke-v1a2.xml - simple-tfjob-gke seems spurious since this only affects jupyterhub |
|
/retest |
…rictions of 63 characters (kubeflow#1200) * shorten pvc, pod names * change {username}{servername} to {userid} to avoid volume length restrictions * remove extraneous changes * avoid identifier length limitations for pvc and pod by removing prefix accounts.google: for iap and truncating escaped self.user.name to 63 characters * add comment about overriding KubeSpawner method * fix _parse_user_name error * add TODO unit test for _parse_user_name
* Support string metrics * Fix error in e2e tests
Fixes #1177
This change is