Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 2 additions & 14 deletions .github/workflows/lint.yaml

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I removed some of the tests as they don't provide much value. They only check if templating works and not what is produced.

Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,10 @@ jobs:
- name: Lint chart
run: helm lint chart/

- name: Template chart (NebariApp disabled)
run: helm template test chart/ --set nebariapp.enabled=false

- name: Template chart (NebariApp enabled)
run: |
helm template test chart/ \
--set nebariapp.enabled=true \
--set nebariapp.hostname=test.example.com \
--set nebariapp.serve.enabled=true \
--set nebariapp.serve.hostname=test.example.com \
--set nebariapp.dashboard.enabled=true \
--set nebariapp.dashboard.hostname=dashboard.example.com

- name: Template chart (NebariApp + auth enabled)
run: |
helm template test chart/ \
--set nebariapp.enabled=true \
--set nebariapp.hostname=test.example.com \
--set nebariapp.dashboard.enabled=true \
--set nebariapp.dashboard.hostname=dashboard.example.com \
--set nebariapp.auth.enabled=true
3 changes: 2 additions & 1 deletion .github/workflows/test.yaml

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As mentioned in another comment, I'm questioning if we should have this workflow at all. Are we expecting this to be deployed anywhere but nebari?

Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ jobs:
run: |
helm install rayserve chart/ \
--set kuberay-operator.enabled=false \
--set nebariapp.enabled=false \
--set nebariapp.serve.enabled=false \
--set nebariapp.dashboard.enabled=false \
--set head.resources.requests.cpu=500m \
--set head.resources.requests.memory=512Mi \
--set head.resources.limits.cpu=1 \
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Helm
chart/charts/
chart/Chart.lock

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure why we ignored the lock file. It is important for a reproducible deployment.


# Dev environment
dev/.cache/
Expand Down
9 changes: 9 additions & 0 deletions chart/Chart.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
dependencies:
- name: nebari-app
repository: oci://quay.io/reiemp/charts
version: 0.1.2
- name: kuberay-operator
repository: https://ray-project.github.io/kuberay-helm/
version: 1.3.0
digest: sha256:9e9345b359fb1be3ec07a39e1fbe3ce0b600309715741d318087170486ba8cde
generated: "2026-08-28T17:43:29.042013605+02:00"
3 changes: 3 additions & 0 deletions chart/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ type: application
version: 0.4.1
appVersion: "2.43.0"
dependencies:
- name: nebari-app
repository: oci://quay.io/reiemp/charts

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

version: "0.1.2"
- name: kuberay-operator
version: 1.3.0
repository: https://ray-project.github.io/kuberay-helm/
Expand Down
15 changes: 0 additions & 15 deletions chart/templates/NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,12 @@
The RayService controller manages the Ray cluster and Serve proxy.
Serve is pre-initialized with host 0.0.0.0 on port 8000.

{{- if .Values.nebariapp.enabled }}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Flag for myself: reinstate.


NebariApp is ENABLED. The nebari-operator will configure:
{{- if ne (toString (dig "serve" "enabled" true .Values.nebariapp)) "false" }}
- HTTPRoute for Ray Serve: {{ .Values.nebariapp.hostname }}
{{- end }}
{{- if .Values.nebariapp.dashboard.enabled }}
- HTTPRoute for Ray Dashboard: {{ .Values.nebariapp.dashboard.hostname }}
{{- end }}
{{- if .Values.nebariapp.auth.enabled }}
- OIDC authentication via {{ .Values.nebariapp.auth.provider }}
{{- end }}

{{- else }}

To access locally:

kubectl port-forward svc/{{ include "nebari-rayserve.serve-service-name" . }} 8000:8000 -n {{ .Release.Namespace }}
kubectl port-forward svc/{{ include "nebari-rayserve.head-service-name" . }} 8265:8265 -n {{ .Release.Namespace }}

{{- end }}

=== Deploying Models ===

Expand Down
111 changes: 20 additions & 91 deletions chart/templates/nebariapp.yaml
Original file line number Diff line number Diff line change
@@ -1,96 +1,25 @@
{{- if and .Values.nebariapp.enabled (ne (toString (dig "serve" "enabled" true .Values.nebariapp)) "false") }}
{{- if .Values.nebariapp.hostname }}
{{- if .Values.nebariapp.serve.enabled -}}
# NebariApp for Ray Serve endpoint
apiVersion: reconcilers.nebari.dev/v1
kind: NebariApp
metadata:
name: {{ include "nebari-rayserve.fullname" . }}
labels:
{{- include "nebari-rayserve.labels" . | nindent 4 }}
spec:
hostname: {{ .Values.nebariapp.hostname }}
service:
name: {{ .Values.nebariapp.service.name | default (include "nebari-rayserve.serve-service-name" .) }}
port: {{ .Values.nebariapp.service.servePort }}
routing:
routes:
- pathPrefix: /
pathType: PathPrefix
Comment on lines -15 to -18

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This block was hardcoded here and was moved into the values.

{{- if .Values.nebariapp.auth.enabled }}
auth:
enabled: true
provider: {{ .Values.nebariapp.auth.provider }}
provisionClient: {{ .Values.nebariapp.auth.provisionClient }}
redirectURI: {{ .Values.nebariapp.auth.redirectURI }}
scopes:
{{- toYaml .Values.nebariapp.auth.scopes | nindent 6 }}

{{- end }}
gateway: {{ .Values.nebariapp.gateway }}
{{- end }}
{{ include "nebari-app.nebariApp" (dict
"metadata" (dict
"name" (include "nebari-rayserve.fullname" .)
"namespace" .Release.Namespace
"labels" (include "nebari-rayserve.labels" . | fromYaml)
)
"spec" (omit .Values.nebariapp.serve "enabled")
"tplCtx" .
) -}}
{{- end }}
---
{{- if and .Values.nebariapp.enabled .Values.nebariapp.dashboard.enabled }}
{{- if .Values.nebariapp.dashboard.enabled -}}
# NebariApp for Ray Dashboard
apiVersion: reconcilers.nebari.dev/v1
kind: NebariApp
metadata:
name: {{ include "nebari-rayserve.fullname" . }}-dashboard
labels:
{{- include "nebari-rayserve.labels" . | nindent 4 }}
spec:
hostname: {{ required "nebariapp.dashboard.hostname is required when dashboard is enabled" .Values.nebariapp.dashboard.hostname }}
service:
name: {{ .Values.nebariapp.service.name | default (include "nebari-rayserve.head-service-name" .) }}
port: {{ .Values.nebariapp.service.dashboardPort }}
routing:
routes:
- pathPrefix: /
pathType: PathPrefix
{{- if .Values.nebariapp.auth.enabled }}
auth:
enabled: true
provider: {{ .Values.nebariapp.auth.provider }}
provisionClient: {{ .Values.nebariapp.auth.provisionClient }}
redirectURI: {{ .Values.nebariapp.auth.redirectURI }}
scopes:
{{- toYaml .Values.nebariapp.auth.scopes | nindent 6 }}

{{- end }}
{{- with .Values.nebariapp.dashboard.landingPage }}
landingPage:
enabled: {{ .enabled | default false }}
{{- with .displayName }}
displayName: {{ . | quote }}
{{- end }}
{{- with .description }}
description: {{ . | quote }}
{{- end }}
{{- with .icon }}
icon: {{ . | quote }}
{{- end }}
{{- with .category }}
category: {{ . | quote }}
{{- end }}
{{- if kindIs "float64" .priority }}
priority: {{ .priority }}
{{- end }}
{{- with .externalUrl }}
externalUrl: {{ . | quote }}
{{- end }}
{{- with .healthCheck }}
healthCheck:
enabled: {{ .enabled | default false }}
{{- with .path }}
path: {{ . | quote }}
{{- end }}
{{- if .intervalSeconds }}
intervalSeconds: {{ .intervalSeconds }}
{{- end }}
{{- if .timeoutSeconds }}
timeoutSeconds: {{ .timeoutSeconds }}
{{- end }}
{{- end }}
{{- end }}
gateway: {{ .Values.nebariapp.gateway }}
{{ include "nebari-app.nebariApp" (dict
"metadata" (dict
"name" (printf "%s-dashboard" (include "nebari-rayserve.fullname" .))
"namespace" .Release.Namespace
"labels" (include "nebari-rayserve.labels" . | fromYaml)
)
"spec" (omit .Values.nebariapp.dashboard "enabled")
"tplCtx" .
) -}}
{{- end }}
43 changes: 21 additions & 22 deletions chart/values.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
# =============================================================================
# Nebari Integration
# =============================================================================
# Creates a NebariApp CRD that configures routing, TLS, and auth via nebari-operator.
# Set enabled: true when deploying on a Nebari cluster.
# Creates NebariApp CRs that configure routing, TLS, and auth via nebari-operator.
nebariapp:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This whole struct is a mix of individual and common configuration. It is certainly not obvious how it works and is not aligned with what other software packs are doing. I basically moved all common fields into the components so they are now independent from each other. This is BC breaking. I'm going to call out the individual things in my comments below.

enabled: false

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

  1. Why would we not deploy this on nebari given that this is a software pack?
  2. Even if there is such a use case, we still have switches for the individual pieces. Meaning, the global one is just convenience to begin with.

# hostname: rayserve.nebari.example.com # Required when serve.enabled is true

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

  1. No longer on the top level, but rather serve.hostname name

  2. The comment says that it is required, but the template silently skips the NebariApp CR if enabled, but no hostname is available.

    {{- if and .Values.nebariapp.enabled (ne (toString (dig "serve" "enabled" true .Values.nebariapp)) "false") }}
    {{- if .Values.nebariapp.hostname }}

    With this PR the hostname is now required if enabled as the comment states.

serve:
enabled: false # Set to true and provide nebariapp.hostname to expose the serve endpoint externally
enabled: false
hostname: '{{ fail "nebariapps.serve.hostname is required if nebariapps.serve.enabled is true" }}'
service:
name: '{{ include "nebari-rayserve.serve-service-name" . | toJson }}'
port: 8000
gateway: public
routing:
routes:
- pathPrefix: /
pathType: PathPrefix
dashboard:
enabled: true
# hostname: ray-dashboard.nebari.example.com # Separate hostname for dashboard
enabled: false
hostname: '{{ fail "nebariapps.dashboard.hostname is required if nebariapps.dashboard.enabled is true" }}'
service:
name: '{{ include "nebari-rayserve.head-service-name" . | toJson }}'
port: 8265
landingPage:
enabled: false
displayName: "Ray Dashboard"
Expand All @@ -23,21 +32,11 @@ nebariapp:
path: /api/component_activities
intervalSeconds: 30
timeoutSeconds: 5
service:
name: "" # Defaults to <release>-<chart>-serve-svc (serve) or -head-svc (dashboard)
servePort: 8000
dashboardPort: 8265
Comment on lines -26 to -29

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Moved into the components with servePort becoming serve.service.port and dashboardPort becoming dashboard.service.port.

auth:
enabled: false
provider: keycloak
provisionClient: true
redirectURI: /oauth2/callback
scopes:
- openid
- profile
- email
Comment on lines -30 to -38

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed as auth was disabled by default. We only need to put it back if the other values besides enabled are intentionally there or if they have been copied over from somewhere and will be user defined anyway.


gateway: public

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Moved into the components.

gateway: public
routing:
routes:
- pathPrefix: /
pathType: PathPrefix

# =============================================================================
# KubeRay Operator
Expand Down
Loading