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

[Platform] K8s universe names need to be sanitized for auto-generated K8s namespaces #8822

Open
jvigil-yugabyte opened this issue Jun 8, 2021 · 2 comments
Assignees
Labels
area/platform Yugabyte Platform
Milestone

Comments

@jvigil-yugabyte
Copy link
Contributor

jvigil-yugabyte commented Jun 8, 2021

When the K8s provider configuration does not specify the namespace name to use, the paltform attempts to automatically create a namespace for each Universe and gives it a name based on the deployment type, region/zone, and Universe name. By default, Kubernetes restricts the allowed namespace names following this regex pattern: [a-z0-9]([-a-z0-9]*[a-z0-9])?. So, it is possible for the user to specify input that would cause an invalid namespace name to be used, which causes the universe creation to fail. There are a couple different ways to handle this. 1) Restrict the allowed region/zone/universe names 2) Automatically sanitize the input.

Example error encountered when attempting to create a Universe name "ShashankTest":

Caused by: java.lang.RuntimeException: The Namespace "yb-dev-ShashankTest-us-west1-a" is invalid: metadata.name: Invalid value: "yb-dev-ShashankTest-us-west1-a": a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name',  or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')
	at com.yugabyte.yw.commissioner.AbstractTaskBase.processShellResponse(AbstractTaskBase.java:113)
	at com.yugabyte.yw.commissioner.tasks.subtasks.KubernetesCommandExecutor.run(KubernetesCommandExecutor.java:222)
	... 5 common frames omitted
@jvigil-yugabyte jvigil-yugabyte self-assigned this Jun 8, 2021
@jvigil-yugabyte
Copy link
Contributor Author

CC: @iSignal

@SergeyPotachev SergeyPotachev added the area/platform Yugabyte Platform label Jun 9, 2021
@iSignal
Copy link
Contributor

iSignal commented Jun 10, 2021

Good catch! We don't have to name the namespaces after the name of the cluster really, so something like sanitize(clustername)+random_hash(10 chars) might work.

Alternately, when we move to the "new naming convention" we can stop creating a separate namespace per AZ (the current method actually prevents us from generating a single LB for multi-AZ scenarios).

@hsu880 hsu880 added this to the 2.7.x milestone Jun 11, 2021
nkhogen added a commit that referenced this issue Apr 11, 2022
…-generated K8s namespaces (#8822)

Summary:
This change allows uppercase universe names. The code for getting namespace is deeply nested and changing its param may lead to much bigger change. I think this change is minimal to take care of the issue.

Change description:

1. The existing regex was allowing universe names like --ABC. It has been changed to not allow this by restricting the start and ending chars to alphanumeric chars only. For non-boundary chars, it remains the same as before.
2. Old namespaces which have already been created in k8s have already passed the k8s restriction (i.e no caps and length). For them, there is no additional padding done for backward compatibility.
3. Only the names not passing the k8s restrcition are modified to add 9 chars suffix derived from the hash of the name.
4. Similar changes done for helm release name as it was also complaining.

Test Plan:
1. Added unit tests
2. Manually created and edited a universe in minikube locally.
3. The change should affect only the names which do not conform to the name k8s validation.

```
This is from platform log for nsing-Test-Universe-2

helm' 'upgrade' 'yb-admin-nsingh-test-universe2-b294f052' '/Users/nkhogen/experiment/k8s/helm/yugabyte-2.13.1.tgz' '-f' '/var/folders/wh/r0qbd7454bx9q5ks7zgkwp4h0000gp/T/6712ab10-d727-4c77-b52a-83da17256ac31578400980055061488.yml' '--namespace' 'yb-admin-nsingh-test-universe2-b294f052' '--timeout' '900s' '--wait' - logging stdout=/var/folders/wh/r0qbd7454bx9q5ks7zgkwp4h0000gp/T/shell_process_out8794086235554227257tmp, stderr=/var/folders/wh/r0qbd7454bx9q5ks7zgkwp4h0000gp/T/shell_process_err7666501670236561566tmp

Naorems-MacBook-Pro-2:k8s nkhogen$ k get ns
NAME                                      STATUS   AGE
default                                   Active   9h
kube-node-lease                           Active   9h
kube-public                               Active   9h
kube-system                               Active   9h
yb-admin-nsingh-test-universe2-b294f052   Active   5m42s
yb-nsingh

Naorems-MacBook-Pro-2:k8s nkhogen$ k -n yb-admin-nsingh-test-universe2-b294f052 get pods
NAME           READY   STATUS    RESTARTS   AGE
yb-master-0    2/2     Running   0          2m43s
yb-tserver-0   0/2     Pending   0          116s
```

Reviewers: sdu, sanketh

Reviewed By: sanketh

Subscribers: jenkins-bot, yugaware

Differential Revision: https://phabricator.dev.yugabyte.com/D16404
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform Yugabyte Platform
Projects
None yet
Development

No branches or pull requests

4 participants