Repository: skypilot-org/skypilot
Pull request: [API server] allow multiple deployment in one k8s
<!-- Describe the changes in this PR -->

close #4777 

A quick fix to allow me test multiple API server in one k8s. After this PR, I can deploy an API server under `host:port/test1` and use `sky api login -e http://host:port/test1` to access the server.

Minimal Helm command to deploy more API servers after the first one:

```bash
helm upgrade --install second-server charts/skypilot \
  --namespace skypilot \
  --set ingress.authCredentials=$AUTH_STRING \
  --set ingress-nginx.enabled=false \
  --set ingress.path=/yourpathhere
```

TODO: udpate doc

<!-- Describe the tests ran -->
<!-- Unit tests (tests/test_*.py) are part of GitHub CI; below are tests that launch on the cloud. -->

Tested (run the relevant ones):

- [x] Code formatting: `bash format.sh`
- [x] Tested manually with 2 API servers deployed on 1 k8s
- [ ] All smoke tests: `pytest tests/test_smoke.py` 
- [ ] Relevant individual smoke tests: `pytest tests/test_smoke.py::test_fill_in_the_name` 
- [ ] Backward compatibility tests: `conda deactivate; bash -i tests/backward_compatibility_tests.sh`


Problem statement:
[API Server] Failed to install multiple API servers in one k8s
1. The chart installs ingress-nginx by default, which conflicts with my existing ingress-nginx. And looks like is no option to disable it.

```bash
$ helm template aylei charts/skypilot --namespace skypilot --set ingress.authCredentials=$AUTH_STRING | grep -A 12 IngressClass
kind: IngressClass
metadata:
  labels:
    helm.sh/chart: ingress-nginx-4.11.3
    app.kubernetes.io/name: ingress-nginx
    app.kubernetes.io/instance: aylei
    app.kubernetes.io/version: "1.11.3"
    app.kubernetes.io/part-of: ingress-nginx
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/component: controller
  name: nginx
spec:
  controller: k8s.io/ingress-nginx
```

2. NodePort conflicts by default:

```
Error: INSTALLATION FAILED: 2 errors occurred:
	* Service "aylei-ingress-controller-np" is invalid: spec.ports[0].nodePort: Invalid value: 30050: provided port is already allocated
```

3. Ingress path conflicts with installed API server:

```
	* admission webhook "validate.nginx.ingress.kubernetes.io" denied the request: host "_" and path "/" is already defined in ingress skypilot/skyplatform-ingress
```

Hints from issue discussion:

Commit message: [API server] allow multiple deployment

Signed-off-by: Aylei <rayingecho@gmail.com>

Patch to review:
diff --git a/charts/skypilot/Chart.yaml b/charts/skypilot/Chart.yaml
index 743099cacfb..e6580c1c375 100644
--- a/charts/skypilot/Chart.yaml
+++ b/charts/skypilot/Chart.yaml
@@ -8,3 +8,4 @@ dependencies:
   - name: ingress-nginx
     version: 4.11.3
     repository: https://kubernetes.github.io/ingress-nginx
+    condition: ingress-nginx.enabled
diff --git a/charts/skypilot/templates/ingress-nodeport.yaml b/charts/skypilot/templates/ingress-nodeport.yaml
new file mode 100644
index 00000000000..f6ac91d64d2
--- /dev/null
+++ b/charts/skypilot/templates/ingress-nodeport.yaml
@@ -0,0 +1,26 @@
+{{- if and .Values.ingress.nodePort.enable .Values.ingress.nginx.enabled }}
+apiVersion: v1
+kind: Service
+metadata:
+  name: {{ .Release.Name }}-ingress-controller-np
+  namespace: {{ .Release.Namespace }}
+spec:
+  type: NodePort
+  ports:
+    - port: 80
+      targetPort: 80
+      {{- if .Values.ingress.httpNodePort }}
+      nodePort: {{ .Values.ingress.httpNodePort }}
+      {{- end }}
+      name: http
+    - port: 443
+      targetPort: 443
+      {{- if .Values.ingress.httpsNodePort }}
+      nodePort: {{ .Values.ingress.httpsNodePort }}
+      {{- end }}
+      name: https
+  selector:
+    app.kubernetes.io/component: controller
+    app.kubernetes.io/instance: skypilot
+    app.kubernetes.io/name: ingress-nginx
+{{- end}}
diff --git a/charts/skypilot/templates/ingress.yaml b/charts/skypilot/templates/ingress.yaml
index 73b05c196b3..2b5e05b4700 100644
--- a/charts/skypilot/templates/ingress.yaml
+++ b/charts/skypilot/templates/ingress.yaml
@@ -1,3 +1,4 @@
+{{- if .Values.ingress.enabled }}
 apiVersion: networking.k8s.io/v1
 kind: Ingress
 metadata:
@@ -10,6 +11,9 @@ metadata:
     nginx.ingress.kubernetes.io/proxy-read-timeout: "3600"
     nginx.ingress.kubernetes.io/proxy-send-timeout: "3600"
     nginx.ingress.kubernetes.io/proxy-body-size: "1000m"
+    {{- if ne .Values.ingress.path "/" }}
+    nginx.ingress.kubernetes.io/rewrite-target: /$2
+    {{- end }}
     nginx.ingress.kubernetes.io/server-snippets: |
       location / {
         proxy_set_header Upgrade $http_upgrade;
@@ -27,19 +31,16 @@ spec:
   rules:
   - http:
       paths:
-      - pathType: Prefix
-        path: "/"
+      {{- /* TODO(aylei): check whether this works for ingress-controllers other than nginx */}}
+      - pathType: {{ if eq .Values.ingress.path "/" }}Prefix{{ else }}ImplementationSpecific{{ end }}
+        path: {{ .Values.ingress.path }}{{ if ne .Values.ingress.path "/" }}(/|$)(.*){{ end }}
         backend:
           service:
             name: {{ .Release.Name }}-api-service
             port:
               number: 80
----
-# GKE load balancer used for ingress does not work with websocket connections
-# (required for our k8s SSH tunneling), so we create a NodePort service that
-# exposes the ingress controller.
-# NOTE: If httpNodePort is not set, Kubernetes will assign random ports in the 
-# NodePort range (default 30000-32767).
+{{- end }}
+{{- if and .Values.ingress.nodePort.enable .Values.ingress.nginx.enabled }}
 apiVersion: v1
 kind: Service
 metadata:
@@ -64,3 +65,4 @@ spec:
     app.kubernetes.io/component: controller
     app.kubernetes.io/instance: skypilot
     app.kubernetes.io/name: ingress-nginx
+{{- end}}
\ No newline at end of file
diff --git a/charts/skypilot/values.yaml b/charts/skypilot/values.yaml
index 9c6c0d535b3..24a7740f2f6 100644
--- a/charts/skypilot/values.yaml
+++ b/charts/skypilot/values.yaml
@@ -81,12 +81,21 @@ ingress:
   authSecret: null
   # Basic auth credentials in format "username:encrypted_password" (only used if ingress.authSecret is not set)
   authCredentials: "username:$apr1$encrypted_password"
-  # Specific nodePort to use for the ingress controller
-  # If not set, Kubernetes will assign random ports in the NodePort range (default 30000-32767)
-  httpNodePort: 30050 # Set to null to automatically assign a random port
-  httpsNodePort: 30051 # Set to null to automatically assign a random port
+  # The base path of API server, you may use differenet path to expose multiple API server through a unified ingress-controller
+  path: '/'
+  nodePort:
+    # GKE load balancer used for ingress does not work with websocket connections
+    # (required for our k8s SSH tunneling), in such case NodePort should be enabled
+    # to exposes the ingress controller.
+    # Note: nodePort svc will only be created when both ingress.nodePort.enable=true AND ingress-nginx.enabled=true
+    enabled: true
+    # Specific nodePort to use for the ingress controller
+    # If not set, Kubernetes will assign random ports in the NodePort range (default 30000-32767)
+    httpNodePort: null # Set to null to automatically assign a random port
+    httpsNodePort: null # Set to null to automatically assign a random port
 
 ingress-nginx:
+  enabled: true
   controller:
     service:
       # Use ClusterIP here to disable the LoadBalancer created by nginx. 


Code review comments:
diff_hunk: @@ -81,12 +81,21 @@ ingress:
   authSecret: null
   # Basic auth credentials in format "username:encrypted_password" (only used if ingress.authSecret is not set)
   authCredentials: "username:$apr1$encrypted_password"
-  # Specific nodePort to use for the ingress controller
-  # If not set, Kubernetes will assign random ports in the NodePort range (default 30000-32767)
-  httpNodePort: 30050 # Set to null to automatically assign a random port
-  httpsNodePort: 30051 # Set to null to automatically assign a random port
+  # The base path of API server, you may use differenet path to expose multiple API server through a unified ingress-controller
+  path: '/'
+  nodePort:
+    # GKE load balancer used for ingress does not work with websocket connections
+    # (required for our k8s SSH tunneling), in such case NodePort should be enabled
+    # to exposes the ingress controller.
+    # Note: nodePort svc will only be created when both ingress.nodePort.enable=true AND ingress-nginx.enabled=true
+    enabled: true
+    # Specific nodePort to use for the ingress controller
+    # If not set, Kubernetes will assign random ports in the NodePort range (default 30000-32767)
+    httpNodePort: null # Set to null to automatically assign a random port
+    httpsNodePort: null # Set to null to automatically assign a random port
path: charts/skypilot/values.yaml
text: @user1:
We do need the nodeport to default to some value. This is so that the prerequisites for installing API server are easy and clear at the start. Having a dynamic nodeport makes setting up firewalls harder.

I think we should stick the existing method and folks who wish to change the default nodeport (for multiple deployments or otherwise) can use --set ingress.httpNodePort.

@author:
Ah, I see, switching back!