Skip to content

Add playground build and deployment #1

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
27 changes: 27 additions & 0 deletions ui/01-is.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
apiVersion: image.openshift.io/v1
kind: ImageStream
metadata:
name: playground
spec:
lookupPolicy:
local: false
---
apiVersion: image.openshift.io/v1
kind: ImageStream
metadata:
name: python-312
spec:
lookupPolicy:
local: false
tags:
- annotations:
from:
kind: DockerImage
name: registry.redhat.io/ubi8/python-312
generation: 1
importPolicy:
importMode: Legacy
name: latest
referencePolicy:
type: Source
Comment on lines +17 to +27
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Pin the base image by digest for deterministic builds

Tagging registry.redhat.io/ubi8/python-312:latest means every rebuild may silently pick up a newer image, breaking reproducibility and SBOM requirements. Prefer an immutable digest:

-      name: registry.redhat.io/ubi8/python-312
+      # sha256 taken from `podman pull --quiet` output
+      name: registry.redhat.io/ubi8/python-312@sha256:<digest>

Importing by digest still lets you tag it as latest locally while ensuring provenance.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tags:
- annotations:
from:
kind: DockerImage
name: registry.redhat.io/ubi8/python-312
generation: 1
importPolicy:
importMode: Legacy
name: latest
referencePolicy:
type: Source
tags:
- annotations:
from:
kind: DockerImage
# sha256 taken from `podman pull --quiet` output
name: registry.redhat.io/ubi8/python-312@sha256:<digest>
generation: 1
importPolicy:
importMode: Legacy
name: latest
referencePolicy:
type: Source
🧰 Tools
🪛 YAMLlint (1.37.1)

[error] 18-18: trailing spaces

(trailing-spaces)

🤖 Prompt for AI Agents
In ui/01-is.yaml around lines 17 to 27, the base image is referenced using the
tag 'latest', which can lead to non-deterministic builds. Replace the image tag
with a specific immutable digest to ensure reproducibility and compliance with
SBOM requirements. Update the 'name' field to use the digest format (e.g.,
'registry.redhat.io/ubi8/python-312@sha256:<digest>') instead of 'latest' while
keeping the local tag as 'latest' if needed.

29 changes: 29 additions & 0 deletions ui/02-bc.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
apiVersion: build.openshift.io/v1
kind: BuildConfig
metadata:
name: playground
spec:
failedBuildsHistoryLimit: 5
nodeSelector:
output:
to:
kind: ImageStreamTag
name: playground:latest
postCommit: {}
resources: {}
runPolicy: Serial
source:
contextDir: llama_stack/distribution/ui
git:
ref: main
uri: https://github.com/meta-llama/llama-stack.git
type: Git
strategy:
sourceStrategy:
from:
kind: ImageStreamTag
name: python-312:1-40.1747189120
namespace: llamastack
type: Source
successfulBuildsHistoryLimit: 5
49 changes: 49 additions & 0 deletions ui/03-dc.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: playground
spec:
progressDeadlineSeconds: 600
replicas: 1
revisionHistoryLimit: 10
selector:
matchLabels:
deployment: playground
strategy:
rollingUpdate:
maxSurge: 25%
maxUnavailable: 25%
type: RollingUpdate
template:
metadata:
annotations:
openshift.io/generated-by: OpenShiftNewApp
creationTimestamp:
labels:
deployment: playground
spec:
containers:
- command:
- streamlit
- run
- app.py
- "--server.port=8501"
- "--server.address=0.0.0.0"
env:
- name: LLAMA_STACK_ENDPOINT
value: http://llama-test-milvus-service-llamastack.apps.rosa.akram.vsil.p3.openshiftapps.com
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like your own private cluster address should we instead provide a generic http://example-llama-stack-service.com example instead?

image: image-registry.openshift-image-registry.svc:5000/llamastack/playground@sha256:00a3008aa70832bc679513b21cacc4e2ae5e875ee648dfbd60264a8b2b42d65b
imagePullPolicy: IfNotPresent
name: playground
ports:
- containerPort: 8501
protocol: TCP
resources: {}
terminationMessagePath: "/dev/termination-log"
terminationMessagePolicy: File
Comment on lines +26 to +44
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

No security context, probes or resource limits – fails basic hardening policies

Static analysis flags privilege escalation & root user. Also missing liveness/readiness probes, which Streamlit readily supports via /_stcore/health.

Minimal hardened snippet:

         resources:
-          {}
+          limits:
+            memory: "512Mi"
+            cpu: "500m"
+          requests:
+            memory: "256Mi"
+            cpu: "100m"
+        securityContext:
+          allowPrivilegeEscalation: false
+          runAsNonRoot: true
+          seccompProfile:
+            type: RuntimeDefault
+        livenessProbe:
+          httpGet:
+            path: /_stcore/health
+            port: 8501
+          initialDelaySeconds: 30
+          periodSeconds: 10
+        readinessProbe:
+          httpGet:
+            path: /_stcore/health
+            port: 8501
+          initialDelaySeconds: 5
+          periodSeconds: 10
🤖 Prompt for AI Agents
In ui/03-dc.yaml between lines 26 and 44, the container spec lacks security
context settings to prevent privilege escalation and avoid running as root, and
it also misses liveness and readiness probes for health checks. Add a
securityContext section to disable privilege escalation and run as a non-root
user, and define livenessProbe and readinessProbe using HTTP GET requests to the
Streamlit health endpoint /_stcore/health on port 8501. Also, specify resource
limits and requests to enforce resource usage constraints.

dnsPolicy: ClusterFirst
restartPolicy: Always
schedulerName: default-scheduler
securityContext: {}
terminationGracePeriodSeconds: 30
19 changes: 19 additions & 0 deletions ui/04-svc.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
apiVersion: v1
kind: Service
metadata:
name: playground
spec:
internalTrafficPolicy: Cluster
ipFamilies:
- IPv4
ipFamilyPolicy: SingleStack
ports:
- name: http
port: 8501
protocol: TCP
targetPort: 8501
selector:
deployment: playground
sessionAffinity: None
type: ClusterIP
13 changes: 13 additions & 0 deletions ui/05-route.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
apiVersion: route.openshift.io/v1
kind: Route
metadata:
name: playground
spec:
port:
targetPort: http
to:
kind: Service
name: playground
weight: 100
wildcardPolicy: None
Comment on lines +6 to +13
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

❓ Verification inconclusive

Route lacks TLS termination and explicit host – external access will default to HTTP on a generated hostname
You are creating a public-facing entry-point but leave both spec.host and spec.tls unset.
• Users will be redirected to an automatically generated *.apps.<cluster> hostname with plain HTTP.
• Most browsers display security warnings or block mixed-content calls when the UI later fetches HTTPS back-end endpoints.

Add an explicit host (or parameterise it) and enable edge termination:

   to:
     kind: Service
     name: playground
     weight: 100
+  tls:
+    termination: edge        # or re-encrypt/passthrough
+    insecureEdgeTerminationPolicy: Redirect
+  host: playground.example.com   # 👉 make this configurable in overlays / Helm values

Confirm whether the cluster is front-ended by a load balancer that injects TLS automatically; if not, the above is required.


Route lacks TLS termination and explicit host – external access will default to HTTP on a generated hostname
You are creating a public-facing entry-point but leave both spec.host and spec.tls unset.
• Users will be redirected to an automatically generated *.apps.<cluster> hostname with plain HTTP.
• Most browsers display security warnings or block mixed-content calls when the UI later fetches HTTPS back-end endpoints.

Add an explicit host (or parameterise it) and enable edge termination:

   to:
     kind: Service
     name: playground
     weight: 100
+  tls:
+    termination: edge        # or re-encrypt/passthrough
+    insecureEdgeTerminationPolicy: Redirect
+  host: playground.example.com   # 👉 make this configurable in overlays / Helm values

Confirm whether the cluster is front-ended by a load balancer that injects TLS automatically; if not, the above is required.

🤖 Prompt for AI Agents
In ui/05-route.yaml around lines 6 to 13, the route definition lacks both an
explicit host and TLS termination, causing the route to default to HTTP on a
generated hostname, which can lead to browser security warnings. To fix this,
add a `spec.host` field with a specific hostname or parameterize it, and include
a `spec.tls` section with `termination: edge` to enable TLS termination at the
edge. Verify if the cluster uses a load balancer that handles TLS automatically;
if not, these changes are necessary to ensure secure external access.

55 changes: 55 additions & 0 deletions ui/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Deploying llamastack playground ui on OpenShift

## Clone llamastack

```
git clone https://github.com/meta-llama/llama-stack.git
cd llama_stack/distribution/ui
```

## Building image

```
oc import-image ubi8/python-312 --from=registry.redhat.io/ubi8/python-312 --confirm
oc new-app --name=playground . --image-stream="python-312" --context-dir=llama_stack/distribution/ui
```

## Configuring, patching and deploying
Set llamastack endpoint route
```
LS_ROUTE=$(oc get route llamastack -ojsonpath={.spec.host})
oc set env deployment/playground LLAMA_STACK_ENDPOINT=http://$LS_ROUTE
Comment on lines +20 to +21
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Quote the jsonpath expression to avoid shell brace-expansion

Unquoted {} is interpreted by most shells (e.g., zsh) and may fail.

-LS_ROUTE=$(oc get route llamastack -ojsonpath={.spec.host})
+LS_ROUTE=$(oc get route llamastack -o jsonpath='{.spec.host}')

Keeps the command POSIX-portable and immune to globbing.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
LS_ROUTE=$(oc get route llamastack -ojsonpath={.spec.host})
oc set env deployment/playground LLAMA_STACK_ENDPOINT=http://$LS_ROUTE
LS_ROUTE=$(oc get route llamastack -o jsonpath='{.spec.host}')
oc set env deployment/playground LLAMA_STACK_ENDPOINT=http://$LS_ROUTE
🤖 Prompt for AI Agents
In ui/README.md around lines 20 to 21, the jsonpath expression in the oc get
route command is unquoted, which can cause shell brace-expansion issues in some
shells like zsh. To fix this, enclose the jsonpath expression {.spec.host} in
single or double quotes to prevent shell interpretation and ensure POSIX
portability and correct command execution.

```

Change:
- entrypoint to `streamlit` as openshift python image will use app.py only instead.
- port to 8501 as python image uses 8080 instead

```
oc patch deployment playground -p '{"spec":{"template":{"spec":{"containers":[{"name":"playground","command":["streamlit","run","app.py","--server.port=8501","--server.address=0.0.0.0"],"ports":[{"containerPort":8501,"protocol":"TCP"}]}]}}}}'
oc patch svc playground -p '{"spec":{"ports":[{"port":8501,"targetPort":8501,"protocol":"TCP","name":"http"}]}}'
```

Expose `service` through a route and patch it
```
oc expose svc playground
oc patch route playground -p '{"spec":{"port":{"targetPort":"http"}}}'
```


## Getting manifests

```
oc eksporter is > 01-is.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this eksporter supposed to mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @Ygnas ,

it is a krew plugin that does the export of kubernetes resources and clean all namespace dependant, or status dependent data.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks it makes sense now

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mention about it in the README?

oc eksporter bc playground --drop spec.triggers > 02-bc.yaml
oc eksporter deployment playground > 03-dc.yaml
oc eksporter svc playground --drop spec.clusterIPs > 04-svc.yaml
oc eksporter route playground --drop spec.host > 05-route.yaml
```
Comment on lines +42 to +48
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Document the eksporter dependency

Previous feedback asked for clarification, but the README still assumes the plugin exists. Add installation instructions or an alternative (kubectl neat, oc adm export).

+# Prerequisite
+kubectl krew install eksporter   # https://github.com/…/eksporter

Without this, newcomers hit “command not found”.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
```
oc eksporter is > 01-is.yaml
oc eksporter bc playground --drop spec.triggers > 02-bc.yaml
oc eksporter deployment playground > 03-dc.yaml
oc eksporter svc playground --drop spec.clusterIPs > 04-svc.yaml
oc eksporter route playground --drop spec.host > 05-route.yaml
```
# Prerequisite
kubectl krew install eksporter # https://github.com/…/eksporter
oc eksporter is > 01-is.yaml
oc eksporter bc playground --drop spec.triggers > 02-bc.yaml
oc eksporter deployment playground > 03-dc.yaml
oc eksporter svc playground --drop spec.clusterIPs > 04-svc.yaml
oc eksporter route playground --drop spec.host > 05-route.yaml
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

42-42: Fenced code blocks should have a language specified
null

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
In ui/README.md around lines 42 to 48, the README uses the `oc eksporter`
command without explaining how to install or obtain the `eksporter` plugin,
which causes confusion for new users. Add a section before these commands that
provides installation instructions for the `eksporter` plugin or suggest
alternative tools like `kubectl neat` or `oc adm export` with brief usage notes.
This will ensure users know how to get the required tool before running the
commands.


## Installing everything using manifests

```
oc create -f .
```

7 changes: 7 additions & 0 deletions ui/export-manifests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
oc eksporter is > 01-is.yaml
oc eksporter bc playground --drop spec.triggers > 02-bc.yaml
oc eksporter deployment playground > 03-dc.yaml
oc eksporter svc playground --drop spec.clusterIPs > 04-svc.yaml
oc eksporter route playground --drop spec.host > 05-route.yaml
Comment on lines +1 to +5
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Missing shebang and defensive flags – script fails shellcheck & may silently succeed with partial output

  1. SC2148 – add #!/usr/bin/env bash.
  2. Use set -euo pipefail so CI stops on failures (e.g., missing eksporter plugin).
  3. Consider validating that each target exists before export.
+#!/usr/bin/env bash
+set -euo pipefail
+
 oc eksporter is                                    > 01-is.yaml
 oc eksporter bc playground --drop spec.triggers    > 02-bc.yaml
 oc eksporter deployment playground                 > 03-dc.yaml
 oc eksporter svc playground --drop spec.clusterIPs > 04-svc.yaml
 oc eksporter route playground --drop spec.host     > 05-route.yaml
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
oc eksporter is > 01-is.yaml
oc eksporter bc playground --drop spec.triggers > 02-bc.yaml
oc eksporter deployment playground > 03-dc.yaml
oc eksporter svc playground --drop spec.clusterIPs > 04-svc.yaml
oc eksporter route playground --drop spec.host > 05-route.yaml
#!/usr/bin/env bash
set -euo pipefail
oc eksporter is > 01-is.yaml
oc eksporter bc playground --drop spec.triggers > 02-bc.yaml
oc eksporter deployment playground > 03-dc.yaml
oc eksporter svc playground --drop spec.clusterIPs > 04-svc.yaml
oc eksporter route playground --drop spec.host > 05-route.yaml
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

(SC2148)

🤖 Prompt for AI Agents
In ui/export-manifests.sh lines 1 to 5, the script lacks a shebang and defensive
shell options, causing shellcheck failures and potential silent errors. Add the
shebang line `#!/usr/bin/env bash` at the top, then add `set -euo pipefail` to
ensure the script exits on errors or unset variables. Additionally, before each
`oc eksporter` command, add checks to verify the target resource exists to
prevent partial or empty output.