feat(ollama): add Ollama chart - #312
Conversation
Adds a self-hosted Ollama chart, filling a gap in the existing AI/LLM lineup (localai, litellm, chromadb, holmesgpt): a single-pod StatefulSet with persistent model storage, optional automatic model pulling via an init container (values.models), and generic nodeSelector/tolerations passthrough for targeting GPU nodes. Verified end-to-end on minikube: install, automatic model pull, inference via /api/generate, pod-restart persistence, and clean uninstall.
Follows the same pattern as the holmesgpt/localai additions.
Follows the same manual packaging step used in the holmesgpt PR (#307): package the chart into docs/, then regenerate docs/index.yaml so helm.zop.dev serves it.
c79a6f6 to
cf9c1ef
Compare
Adds a photo/video backup chart deploying all four components from Immich's own reference architecture: immich-server, immich-machine- learning, a bespoke Postgres StatefulSet, and the zopdev redis chart. Postgres can't reuse the zopdev postgres chart -- Immich's search features need a vector extension baked into the image (ghcr.io/immich-app/postgres), which the shared bitnami-based chart doesn't have. Env vars, health endpoints, and image tags were verified against Immich's actual source rather than assumed. Verified end-to-end on minikube: all four components reach Ready, /api/server/ping and the web UI respond, server logs confirm real Postgres/Redis/machine-learning connections (not just HTTP up), Postgres has the full migrated schema, and all three stateful pods recover cleanly with data intact after a simultaneous restart. Two bugs caught and fixed during testing: - Same PrometheusRule null-label issue as postgres (fixed via postgres.name) hits the redis subchart too -- fixed with redis.name. - The machine-learning StatefulSet had no startupProbe, so its liveness probe was killing the container before cold-start model loading finished. Added one, matching the server's pattern. Also packages the chart to docs/immich-v0.0.1.tgz, regenerates docs/index.yaml, and lists immich in the root README's Applications table, matching #307/#312.
arunesh-j
left a comment
arunesh-j
left a comment
There was a problem hiding this comment.
Review — feat(ollama): add Ollama chart
Reviewed at cf9c1ef in an isolated worktree, installed on minikube. 1 blocking, 2 should-fix, 2 notes.
Clean before the findings: helm lint passes (CI reproduction), defaults render, the ingress fail guard genuinely fires when enabled is set without a host, schema checker reports 0/0, the root README APPLICATIONS table is updated, every value in values.yaml is documented in the chart README, docs/index.yaml was properly regenerated (0/182 created: timestamps carried, added: 1, removed: 0, changed: 0, 183 digests verified), and the chart renders no CRD-dependent objects, so unlike the datastore charts it installs on a cluster without prometheus-operator. Installed with 0 restarts and the headless-service serviceName comment is the kind of thing that saves someone a re-release.
1. [Blocking] The icon hotlinks ollama.com
charts/ollama/Chart.yaml:7 — icon: "https://ollama.com/public/ollama.png".
Every icon in this repo is served from infrastructure zopdev controls. I checked all 33 charts: 30 use storage.googleapis.com/zopdev-* or zop.dev/logo.png, 3 have no icon at all, and none hotlinks a third party. This would be the first.
The URL resolves today (HTTP 200), so this is not about it being broken — it is that the asset renders on a zop.dev product surface while living somewhere the repo cannot control: it can 404, be replaced, or start serving something else, at which point the fix requires a chart re-release.
Fix: upload the logo to the zopdev bucket and point icon: at it, as qdrant (#310) and litellm do. A reviewer cannot upload on your behalf.
2. [Should fix] The model-pull init container reports success when the pull fails
charts/ollama/templates/statefulset.yaml:36-50. The script is:
ollama serve &
pid=$!
until ollama list >/dev/null 2>&1; do sleep 1; done
ollama pull <model>
kill $pidThere is no set -e, and kill $pid is the last command — so the container's exit status is kill's, not the pull's. A failed pull is swallowed.
Reproduced on cluster with --set 'models={definitely-not-a-real-model-xyz}':
init container log: Error: pull model manifest: file does not exist
init exit code: 0
main container: Ready, 0 restarts
The pod came up serving an API with no model, which is precisely what the comment above the block says it prevents ("so the API never serves a model that isn't there yet"). A typo in models currently produces a healthy-looking release and a runtime surprise on the first request.
Fix — make the pull the exit status, and keep the cleanup:
set -e
ollama serve &
pid=$!
trap 'kill $pid' EXIT
until ollama list >/dev/null 2>&1; do sleep 1; done
ollama pull <model>Worth bounding the wait loop in the same pass: until ollama list has no timeout, so if ollama serve never becomes usable the init container blocks forever and the only symptom is a pod stuck in Init:0/1 until helm's own timeout expires.
3. [Should fix] Published version string is missing the v prefix
docs/index.yaml — the new entry is version: 0.0.1 while its URL is ollama-v0.0.1.tgz. The repo's own tooling flags the disagreement:
[!] ollama 0.0.1: url filename 'ollama-v0.0.1.tgz' != expected 'ollama-0.0.1.tgz'
The packaged Chart.yaml inside the tarball says version: 0.0.1, so the chart was built with a plain helm package ./charts/ollama (which emits ollama-0.0.1.tgz) and the file was then renamed. Every other entry in the index is v-prefixed and self-consistent — litellm v0.0.2 | litellm-v0.0.2.tgz, postgres v0.0.14 | postgres-v0.0.14.tgz, holmesgpt v0.0.1 | holmesgpt-v0.0.1.tgz.
Installs do work (helm fetches whatever urls: says), so this is consistency rather than breakage: helm search repo zop/ollama will show 0.0.1 where every sibling shows vX.Y.Z, and the tarball's own metadata disagrees with its filename.
Fix, per CONTRIBUTING steps 4–6: helm package ./charts/ollama --version "v0.0.1", move it into docs/, delete the renamed artifact, then regenerate with helm repo index . --url https://helm.zop.dev.
4. [Note] Nothing says the API is unauthenticated, and the chart ships an ingress
Ollama has no built-in authentication — upstream's FAQ documents only OLLAMA_HOST for changing the bind address, with no auth mechanism anywhere. Verified on cluster: GET /api/tags returns 200 with no credentials.
That is fine inside a cluster, but ingress.enabled=true publishes an inference API that anyone who resolves the host can use — to run inference on your GPU, or to POST /api/pull arbitrary models onto the volume. Neither README.md nor NOTES.txt mentions it (I grepped for auth/expose/secure/token and found nothing).
One line under the ingress section — that the API is unauthenticated and an ingress should be paired with authentication at the proxy, or restricted to trusted networks — would set expectations. Compare qdrant, which generates an API key and states that all data requests require it.
5. [Note] Runs as root, with no securityContext
Verified in the running pod: uid=0(root) gid=0(root). The chart sets no pod or container securityContext, so it inherits the image's root default, and the models volume is root-owned.
Not a defect — the image expects to own /root/.ollama, and I confirmed the volume is writable, so there is nothing to fix for it to work. Raising it only because qdrant in this repo pins runAsNonRoot with a matching fsGroup, so a reader comparing the two will wonder whether root here was a decision or an oversight. A one-line comment in values.yaml would answer that.
Verified versus read
Verified on cluster (minikube, per-PR namespace and release, explicit --context on every call): install reached Ready with 0 restarts; container runs as uid=0 and the mounted volume is writable; the image sets OLLAMA_HOST=0.0.0.0:11434, so upstream's 127.0.0.1 default does not apply here; GET / → 200 (the probe path is valid) and /api/tags → 200 unauthenticated; a bogus models entry produced the exit-0 behaviour in finding 2; helm upgrade applied cleanly. Torn down afterwards, including released PVs.
Read, not executed: the GPU scheduling path (nodeSelector/tolerations/nvidia.com/gpu) — no GPU node available — and a real multi-GB model pull, so the init container's happy path was exercised only against a failing pull.
Three things I checked specifically because they looked like findings, and am not reporting: the absence of a ServiceMonitor (Ollama serves no Prometheus endpoint — /metrics returns 404, so there is nothing to scrape); the absence of fsGroup (irrelevant while the container runs as root, and the volume is writable); and the missing configmap.yaml/secret.yaml from the usual new-chart shape (this chart has no config file and no credentials to generate).
- Init container now fails when a model pull fails or the server never comes up, instead of always exiting 0 (set -e + trap on cleanup, replacing an unconditional trailing kill; bounded the server-startup wait to 60s). A bad models entry now produces a failed release instead of a healthy-looking pod serving no model. - Repackaged with the v-prefixed version stamp every other chart in the index uses (v0.0.1), matching litellm/holmesgpt's convention; the bare 0.0.1 previously recorded disagreed with its own tarball filename. - Documented that the API is unauthenticated and that an ingress should be paired with auth/network restriction. - Documented that the container runs as root as a deliberate fit with the upstream image's own default, not an oversight.
|
Thanks for the thorough review — addressed in
Re-verified all of the above on minikube. |
arunesh-j
left a comment
arunesh-j
left a comment
There was a problem hiding this comment.
Re-review of cb8162b
Findings 2–5 are resolved and verified on cluster. Finding 1 (icon) is still open, so this isn't approvable yet — but it's the only thing left, and the path forward doesn't need bucket access.
2. Init container swallowing pull failures — fixed, verified
The shipped script is set -e + trap 'kill $pid' EXIT with the trailing unconditional kill removed, and the readiness wait bounded to 60 tries with an explicit error. I re-ran the exact repro rather than take the reply on trust:
before (cf9c1ef): init exit 0 → pod Ready, serving no model
now (cb8162b): init exit 1 → Init:Error, main container never starts
log: "Error: pull model manifest: file does not exist"
Happy path re-checked too: with models empty, no init container is rendered and the pod reaches Ready with 0 restarts. The bounded wait is the right call and I'd not asked for the error message on timeout — that's a good addition, since Init:Error with a reason beats a pod hanging silently.
Also noted: you documented the semantics under Model pulling, including that only the server startup is time-bounded and a slow-but-working pull isn't killed. That distinction is exactly right and worth having written down.
3. Version prefix — fixed, verified
All three now agree, and the repo's own checker no longer flags ollama:
packaged Chart.yaml: version: v0.0.1
tarball: docs/ollama-v0.0.1.tgz
index entry: v0.0.1 | ollama-v0.0.1.tgz
Index still isolated and properly regenerated: added: 1 (ollama v0.0.1), removed: 0, changed: 0, 0% of created: timestamps carried. The only remaining finding from verify-index is the pre-existing chromadb filename casing, which is not yours.
4. Unauthenticated API — fixed
### Security (README:117) states there is no upstream auth, that every endpoint answers uncredentialed callers, that ingress.enabled publishes both inference and POST /api/pull to anyone resolving the host, and to pair it with proxy-layer auth or restrict to trusted networks. That covers it.
5. Root user — fixed
values.yaml now explains root is deliberate and matches the upstream image's expectation of owning /root/.ollama, with the same point in the README. That answers the "decision or oversight?" question, which is all I was after.
1. Icon — still open, and here are the options
Chart.yaml:7 is unchanged: icon: "https://ollama.com/public/ollama.png".
Understood on bucket access — I can't upload either, so neither of us can apply the ideal fix. But it doesn't have to block on that, because there are two ways to land this without an upload:
- Drop the
icon:line. Three charts already ship without one —karpenter-gcp,mosquitto,openobserve-standalone— so the chart renders logo-less on helm.zop.dev, which is a cosmetic gap rather than a supply-chain one. Add the zopdev-hosted URL in a follow-up when someone with access uploads the asset. - Use
https://zop.dev/logo.png, the generic fallback the other icon-less building-block charts use. Less apt for a product with its own logo, but on-domain.
The one option I'd push back on is merging as-is: the hotlink is the only variant where a third party controls what renders on a zop.dev surface, and it's the state that needs a chart re-release to undo. Either alternative above is strictly better and takes one line.
Happy to re-review as soon as that's decided — everything else is done.
Replaces the ollama.com hotlink with a zopdev-hosted asset, matching every other chart in the repo. Repackaged and reindexed accordingly.
|
Icon fixed in `95b12ce`: Uploaded to the zopdev bucket, repackaged, and reindexed — `Chart.yaml`, the tarball, and the index entry all agree. That was the last open item; everything else was already confirmed in the previous re-review. |
arunesh-j
left a comment
arunesh-j
left a comment
There was a problem hiding this comment.
Re-review of 95b12ce
Finding 1 (icon) is fixed and verified — all five findings from this review are now closed.
One new should-fix turned up, unrelated to anything changed here; it has been present since the
first revision and I simply did not catch it in the earlier rounds.
1. Icon — fixed, verified
Chart.yaml:7 now points at the zopdev bucket, and I checked the asset rather than the URL
string:
HTTP 200 · 7487 bytes · PNG 181x256 RGBA · storage.googleapis.com/zopdev-blog-resources
I fetched and opened it: it is the Ollama llama mark, not a placeholder or a wrong asset. The
bucket is zopdev-blog-resources, which is the one to prefer for new work.
The part worth calling out, because it is the step that is easy to miss: the site reads
released metadata, so a Chart.yaml edit alone would have changed nothing. You repackaged, so
the new URL is present in all three places it needs to be — the working tree, the tarball's own
Chart.yaml, and the icon: field of the docs/index.yaml entry. Digest matches the tarball on
disk, and the repackage carried nothing stale: every file in docs/ollama-v0.0.1.tgz is
byte-identical to the working tree, with Chart.yaml differing only in the intended
version: 0.0.1 → v0.0.1 rewrite that --version performs.
Index is still isolated and properly generated: added: 1 (ollama v0.0.1), removed: 0, changed: 0, 0% of created: timestamps carried, only the pre-existing Chromadb casing left.
2–5 — re-confirmed independently
I re-checked these against the tree rather than carrying forward the previous re-review:
the init container has set -e, trap 'kill $pid' EXIT and the 60-try bounded wait with an
explicit error; the version prefix agrees across Chart.yaml, tarball and index and
verify-values.py/verify-index.py are clean on ollama; the README Security section
(README.md:117) covers the unauthenticated API and the ingress exposure; and the root-user
rationale is stated in the values.yaml header.
New — [Should fix] The init container inherits none of .Values.env
charts/ollama/templates/statefulset.yaml:35-58 vs 63-73
.Values.env is applied to the main container only. The init container runs the same image, does
network I/O and writes the models volume, but gets no environment at all:
init pull-models env: NONE
main ollama env: ['HTTPS_PROXY', 'OLLAMA_MODELS']
Two concrete ways that bites, in increasing order of how quietly it fails:
HTTPS_PROXY/HTTP_PROXY. Behind an egress proxy the pull cannot reach the registry.
This one at least fails loudly now, thanks to theset -efix —Init:Errorwith the pull's
message.OLLAMA_MODELS. This sets the model directory; I confirmed the semantics in upstream's
envconfig/config.goatv0.32.9(Models()returns$OLLAMA_MODELSwhen set, else
$HOME/.ollama/models). Set it in.Values.envand the init container pulls into the default
/root/.ollamawhile the server looks somewhere else. The pull succeeds, the init container
exits 0,/returns 200, the pod goes Ready — and the API serves no model. That is precisely
the state the comment at line 32 says this init container exists to prevent, reached by a
different route than the one already fixed.
Fix is a two-line copy of the same range already used on the main container:
{{- if .Values.env }}
env:
{{- range $key, $value := .Values.env }}
- name: {{ $key }}
value: {{ $value | quote }}
{{- end }}
{{- end }}Worth doing even if OLLAMA_MODELS seems unlikely: whatever the reason someone sets an env var
for the server, the puller writing to a different place than the server reads from is a trap that
gives no signal.
New — [Note] The init container has no resources
Same block. It requests nothing, so it is BestEffort — the first thing evicted under node memory
pressure, which is exactly the condition a multi-GB model pull creates. charts/scylladb also
omits resources on its init container, so this is not off-pattern, but that one runs sysctl and
exits; this one runs an inference server and downloads for minutes. Reasoned, not observed — I did
not reproduce an eviction.
Verified versus read, this round
Verified: the icon URL fetched and inspected as an image; icon present in tree, tarball and
index; tarball digest matches and contents match the tree file-by-file; helm lint clean;
verify-values.py OK 0/0; verify-index.py isolated and regenerated; renders clean for
defaults, ingress with host, the two-model list and the GPU passthrough; the ingress.host guard
fires; no duplicate kind+name; the release emits only Service and StatefulSet, so it still
needs no prometheus-operator CRDs; root README row present in APPLICATIONS; image tag
ollama/ollama:0.32.9 exists (with a deliberately bogus tag returning 404 to prove the probe),
and v0.32.9 is a real upstream release; OLLAMA_MODELS semantics read from upstream source at
that tag.
Not re-run on cluster. The last cluster verification was cb8162b; since then only chart
metadata changed (icon, plus the repackage), no template or values change, so an install would
exercise nothing new. The new finding above is a render-level defect and is evidenced by the
rendered manifests rather than a cluster run.
…e main one The model-pull init container previously got none of .Values.env, only the main container did. Setting OLLAMA_MODELS there (the real upstream env var for relocating the models directory) made the puller write to the default /root/.ollama while the server looked wherever OLLAMA_MODELS pointed -- pull succeeds, pod goes Ready, API serves no model. Also gave it the same resources as the main container, since it does the same network/disk work and was previously BestEffort. Verified on minikube with OLLAMA_MODELS set to a custom path: the pulled model now shows up in /api/tags instead of being invisible.
|
Both fixed in `c033a98`:
|
2 participants
Description
Adds a new
charts/ollama/chart deploying Ollama, a self-hosted LLM inference server. Fills a gap alongside the existing AI/LLM charts (localai,litellm,chromadb,holmesgpt) — other charts (or any app) can point at this instead of paying for an external inference API.Design decisions (see chart README for detail):
localai. Ollama has no multi-node primitive worth modeling for v1.servicesubchart dependency — matches thelitellm/localaipattern (the two most similar, most recently added charts) rather than the legacyoutline/wordpress/supersetpattern, which has documented version-coupling pain (see those charts'Chart.yamlcomments).values.models(e.g.["llama3"]), using an init container that runsollama pullagainst the same volume before the main container starts. Empty list (default) renders no init container.nodeSelector/tolerationspassthrough for GPU node targeting instead of a bespoke GPU flag — no chart in this repo has GPU precedent yet, so this reuses the existing generic pattern (openobserve-standalone) rather than inventing new config surface.type: applicationperCONTRIBUTING.md(serves requests, doesn't store user data).Also, matching #307 (holmesgpt):
ollamain the rootREADME.md's Available Charts / Applications table.docs/ollama-v0.0.1.tgzand regeneratesdocs/index.yamlso it's servable fromhelm.zop.dev.Type of Change
Testing
Verified end-to-end on a local minikube cluster:
helm lint— cleanhelm install --dry-run --debug— renders correctly, including the ingressfailguard and the init-container path whenmodelsis set--set models={tinyllama}— init container pulled the model, main container reached Readycurl /api/generate— model responded correctly, confirming inference works end-to-endollama pullrecognized the model already on the PVC (no re-download), confirming the persistence designhelm uninstall— clean teardown, PVC correctly left behind (documented in README)Checklist
helm lintpasses without errorsdocs/index.yaml)🤖 Generated with Claude Code