-
Notifications
You must be signed in to change notification settings - Fork 2k
Enable Ollama integration test #322
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
Enable Ollama integration test #322
Conversation
Currently, test is disable because in every run the image should be downloaded and then pull the model. Now, taking advantage of Testcontainers and a GHA, a new image is created on-the-fly with the model in it and then cached, so, next executions will reuse the cached image instead. Fixes spring-projects#121
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'd like to speed up the IT tests as well. I'm curious as to the rationale in going from a one liner to use test containers
@Container
static GenericContainer<?> ollamaContainer = new GenericContainer<>("ollama/ollama:0.1.23").withExposedPorts(11434);
To now using two static classes and a static method. I'm not quite up on my testcontainer knowledge, but would have expected something more terse code-wise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason is to create the new Ollama image with the orca-mini model in it. We are doing it programmatically but another approach could be to reuse an existing image from Docker Hub that has the model and the one line could remain just pointing to the new image, removing the rest of the code. However, not sure if there is any preference to use any image out there or creating their own.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a tricky situation I encountered in other projects as well. On the one hand, it'd be better to use official images. On the other hand, downloading the model every time is not much feasible, requiring some mitigation strategies with additional configuration. I looked for existing images out there, but I ended up publishing my own Ollama-based images to make sure they are always up-to-date and for having multi-architecture support: https://github.com/ThomasVitale/llm-images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an issue in ollama to provide specific images ollama/ollama#2161
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That image has been closed with 'won't do' - so closing this issue. We will have to setup a separate integration test profile for ollama along with other model providers that aren't currently running in CI
|
@eddumelendez , I'm trying to run this locally. The first attempt failed with 404 if not mistaken and a consecutive runs cause: May you know what is going on? |
|
Did a second attempt (after removing all docker images) and the error it fails with is : |
|
Hi @tzolov, I've reviewed and fixed the issue. PR is updated and everything should work as expected now. |
|
Thanks @eddumelendez , just merged it but it seems we don't have the right account for using docker caching ? https://github.com/spring-projects/spring-ai/actions/runs/8070608896 |
|
Looks like need to check |
|
Yeh, got this as well. But i'm not sure how safe is to white list an unverified action? |
|
@eddumelendez i've whitelisted it but now it fails on this |
|
Let me try on my fork.
Totally understand. Another alternative could be have persistent runners |
|
Couldn't reproduce it https://github.com/eddumelendez/spring-ai/actions/runs/8072468166/job/22054281356#step:8:22 Can you enable debug logs, please? |
|
This seems to have been merged in 246ba17. |
|
@izeye it is merged but disabled because of the #322 (comment) we still have to resolve. |
|
Closing as model specific images are not going to be provided by ollama. We will pick this up as part of a larger effort to have more CI coverage outside the current github action that runs on each commit. |
|
FWIW I just tried my demo in a github action and the ollama models (x2) pulled in 30sec. It's almost not worth trying to cache. If only my network was that fast. It actually takes longer to pull the ollama and chromadb docker images than it does to pull the models. I also tried using https://docs.docker.com/build/ci/github-actions/cache/ to see if it would help, but it fails to create the cache because (I think) ollama is running as root so the file permissions are fubar. |
|
Update. I got it working with this @Bean
@ServiceConnection
public ChromaDBContainer chroma() {
return new ChromaDBContainer(DockerImageName.parse("ghcr.io/chroma-core/chroma:0.5.5"));
}
@Bean
@ServiceConnection
public OllamaContainer ollama() throws Exception {
@SuppressWarnings("resource")
OllamaContainer ollama = new OllamaContainer(DockerImageName.parse("ollama/ollama:0.3.2"))
// Not recommended strategy from testcontainers, but the only practical way to
// make it work locally
.withFileSystemBind("ollama", "/root/.ollama", BindMode.READ_WRITE);
return ollama;
}
@Bean
ApplicationRunner runner(OllamaContainer ollama) {
return args -> {
logger.info("Pulling models...");
ollama.execInContainer("ollama", "pull", "albertogg/multi-qa-minilm-l6-cos-v1");
ollama.execInContainer("ollama", "pull", "mistral");
ollama.execInContainer("chmod", "go+r", "-R", "/root/.ollama");
logger.info("...done");
};
}but creating the cache (on the first run) and unpacking it (subsequently) takes about 1 minute. So it's not efficient for this case. Maybe it would work better with different models or more models. Here's the workflow: name: Java CI with Maven
on:
push:
branches: [ main ]
jobs:
build:
name: Build and Deploy On Push
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Set up JDK 17
uses: actions/setup-java@v4
with:
java-version: '17'
distribution: 'temurin'
cache: maven
- name: Cache LLM Data
id: cache-models
uses: actions/cache@v4
with:
path: ollama
key: ${{ runner.os }}-models
- name: Install with Maven
run: |
./mvnw -B installUPDATE: I also tried caching the docker images with the The file system bind and the exec in container steps in the test context make a big difference to local development though, so definitely worth including those in the tests here (and docs). |
|
We might be able to close this issue now. All integration tests for Spring AI Ollama are now enabled and running on Testcontainers, also on the GitHub Actions workflow. They are based on the newly introduced "model auto-pull" feature #1554 It's used for the integration test setup but it's also a feature to improve the developer experience, making it possible to pull models automatically at startup time if not available yet. |
|
I don't see the ollama tests running in the CI log, still something to sort out. @dsyer thanks for taking the time to dig into it. We do pull several models in the range of IT tests ,but that info you provided is very useful. Let's see how it goes. I'd like to move this issue to spring-projects/spring-ai-integration-tests#5 as we now have a more dedicated environment to run all the tests, thus decreasing the already considerable time spent for our mainline build. |

Currently, test is disable because in every run the image should
be downloaded and then pull the model. Now, taking advantage of
Testcontainers and a GHA, a new image is created on-the-fly with
the model in it and then cached, so, next executions will reuse the
cached image instead.
Fixes #121