Skip to content

Conversation

theblinkingusb
Copy link
Contributor

Not a helm pro here by any means but I believe the ollama.modelsMountPath is missing a condition to check when the mountPath has been changed thereby affecting postStart hooks in the ollama.create functionality.

cat <<EOF > {{ include "ollama.modelsMountPath" $ }}/{{ .name }}
evaluates to a /root/ path. So then this PR would match the behavior to
mountPath: {{ .Values.ollama.mountPath | default "/root/.ollama" }}

Summary of changes:
Update _helpers.tpl file to respect custom mountPaths when using ollama.create templates/modelfiles.
Update deployment.yaml to retrieve same variable value
Update Chart.yaml per PR requirements.

Please let me know if I need to change anything here - I was able to run this locally against my desired chart values (with custom templates + non root) - thanks

Checklist:

  • I updated the artifacthub.io/changes annotation in Chart.yml according to the documentation
  • Optional: I updated in README.md the Helm Values

Not a pro here but believe the ollama.modelsMountPath is missing a condition to check when the base path has been changed 
in a poststart create condition - https://github.com/otwld/ollama-helm/blob/main/templates/deployment.yaml#L183 evaluates to a /root/ path. So then this PR would match the behavior to https://github.com/otwld/ollama-helm/blob/main/templates/deployment.yaml#L123
update chart per PR requirement
Retrieve root variable name to match other template call.
Wrap nullable levels and fix default
@theblinkingusb
Copy link
Contributor Author

For clarification - this bug appeared when using the ollama.create + template feature in the chart while also changing off root path - that's when I noticed the postStart hook failing due to pointing at the root dir.

@jdetroyes
Copy link
Contributor

jdetroyes commented Jul 5, 2025

hello @theblinkingusb

Thanks good catch!

@jdetroyes jdetroyes merged commit 6e3bee3 into otwld:main Jul 5, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants