Skip to content
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

Feature: create a Dockerfile for containerization of the repo-query #15

Merged
merged 1 commit into from Aug 3, 2023

Conversation

jpmcb
Copy link
Member

@jpmcb jpmcb commented Aug 3, 2023

Description

This PR encompasses about a days worth of research and investigation into building and containerizing the repo-query engine. Ideally, for easier deployments, we'd use a flexible container image and some container orchestration service to spin these up:

  • This uses the debian slim-bullseye base image which has the rust toolchain.
    • Installs the build-essential package for the g++ C++ compiler in order to build some of the onnx / rust dependencies
    • We also install libssl-dev (the openssl libraries for the native-tls dependency) and pkg-config (to enable the rust toolchain to find the openssl libraries). This isn't super ideal: it'd be great if we could not consume openssl as a dependency but this change would require some upstream changes to a few libraries we use.
      • You'll notice a few dependencies have default features turned off and reqwest has the rustls-tls feature turned on. This was an attempt to remove the native-tls dependency from our chain of dependencies but I decided to leave this change since we probably shouldn't be consuming all default freatures from reqwest or tokio to reduce the attack surface area of new CVEs that may arrise within their http/tls chains
  • Enables the load-dynamic feature on the ort dependency which enables us to drop the onnx runtime libraries next to the repo-query binary and get picked up with the ORT_DYLIB_PATH env variable.
  • The makefile will enable some easier and more cohesive operations for contributors
  • A few clippy and formatting changes after running make lint

This PR also includes a makefile for easier operations and a few rust fixes for the edge case where env variables are empty strings.

With this running, i can first run the qdrant container:

❯ docker run -p 6333:6333 -p 6334:6334 qdrant/qdrant
           _                 _
  __ _  __| |_ __ __ _ _ __ | |_
 / _` |/ _` | '__/ _` | '_ \| __|
| (_| | (_| | | | (_| | | | | |_
 \__, |\__,_|_|  \__,_|_| |_|\__|
    |_|

Access web UI at http://localhost:6333/dashboard
...

And then also run our container (after building it)

❯ docker run --env-file ./.env -p 3000:3000  -it --rm open-sauced-repo-query
[2023-08-03T06:21:34Z WARN  ort] ort 1.14 may have compatibility issues with the ONNX Runtime binary found at `./target/release/libonnxruntime.so`; expected GetVersionString to return '1.14.x', but got '1.15.1'
[2023-08-03T06:21:34Z INFO  ort::execution_providers] Successfully registered `CPUExecutionProvider`
[src/db/qdrant.rs:120] &qdrant_url = "http://localhost:6334"
[2023-08-03T06:21:34Z INFO  actix_server::builder] starting 6 workers
[2023-08-03T06:21:34Z INFO  actix_server::server] Actix runtime found; starting in Actix runtime

This also includes a docker-compose.yaml file which can be used to easily spin up the entire service locally with a database:

❯ docker-compose up
[+] Building 0.0s (0/0)                                                                                                                                                   
[+] Running 2/0
 ✔ Container repo-query-qdrant-1      Created                                                                                                                                                                                                                           0.0s
 ✔ Container repo-query-repo-query-1  Recreated                                                                                                                                                                                                                         0.0s
Attaching to repo-query-qdrant-1, repo-query-repo-query-1
repo-query-qdrant-1      |            _                 _
repo-query-qdrant-1      |   __ _  __| |_ __ __ _ _ __ | |_
repo-query-qdrant-1      |  / _` |/ _` | '__/ _` | '_ \| __|
repo-query-qdrant-1      | | (_| | (_| | | | (_| | | | | |_
repo-query-qdrant-1      |  \__, |\__,_|_|  \__,_|_| |_|\__|
repo-query-qdrant-1      |     |_|
repo-query-qdrant-1      |
repo-query-qdrant-1      | Access web UI at http://localhost:6333/dashboard
repo-query-qdrant-1      |
repo-query-qdrant-1      | [2023-08-03T16:29:03.179Z INFO  storage::content_manager::consensus::persistent] Loading raft state from ./storage/raft_state
repo-query-qdrant-1      | [2023-08-03T16:29:03.183Z INFO  storage::content_manager::toc] Loading collection: jpmcb-gopherlogs-main
repo-query-qdrant-1      | [2023-08-03T16:29:03.230Z INFO  qdrant] Distributed mode disabled
repo-query-qdrant-1      | [2023-08-03T16:29:03.230Z INFO  qdrant] Telemetry reporting enabled, id: b0f0f88d-2f15-4952-b5e0-1c799953ad71
repo-query-qdrant-1      | [2023-08-03T16:29:03.230Z INFO  qdrant::tonic] Qdrant gRPC listening on 6334
repo-query-qdrant-1      | [2023-08-03T16:29:03.230Z INFO  qdrant::tonic] TLS disabled for gRPC API
repo-query-qdrant-1      | [2023-08-03T16:29:03.231Z INFO  qdrant::actix] TLS disabled for REST API
repo-query-qdrant-1      | [2023-08-03T16:29:03.231Z INFO  qdrant::actix] Qdrant HTTP listening on 6333
repo-query-qdrant-1      | [2023-08-03T16:29:03.231Z INFO  actix_server::builder] Starting 5 workers
repo-query-qdrant-1      | [2023-08-03T16:29:03.231Z INFO  actix_server::server] Actix runtime found; starting in Actix runtime
repo-query-repo-query-1  | [2023-08-03T16:29:03Z WARN  ort] ort 1.14 may have compatibility issues with the ONNX Runtime binary found at `./target/release/libonnxruntime.so`; expected GetVersionString to return '1.14.x', but got '1.15.1'
repo-query-repo-query-1  | [2023-08-03T16:29:03Z INFO  ort::execution_providers] Successfully registered `CPUExecutionProvider`
repo-query-repo-query-1  | [src/db/qdrant.rs:120] &qdrant_url = "http://qdrant:6334"
repo-query-repo-query-1  | [2023-08-03T16:29:03Z INFO  actix_server::builder] starting 6 workers
repo-query-repo-query-1  | [2023-08-03T16:29:03Z INFO  actix_server::server] Actix runtime found; starting in Actix runtime
repo-query-repo-query-1  | [2023-08-03T16:29:07Z INFO  tracing_actix_web::root_span_builder] HTTP request; http.method=POST http.route=/embed http.flavor=1.1 http.scheme=http http.host=localhost:3000 http.client_ip=172.19.0.1 http.user_agent=insomnia/2023.4.0 http.target=/embed otel.name=HTTP POST /embed otel.kind="server" request_id=6fe2bb64-2d41-44c0-aeac-333bb6dceede

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI
  • 📦 Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

Related to #10

Mobile & Desktop Screenshots/Recordings

N/a

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help

Added to documentation?

TODO: need to add docs in the README.md for this.

  • 📜 README.md
  • 📓 docs.opensauced.pizza
  • 🍕 dev.to/opensauced
  • 📕 storybook
  • 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@jpmcb jpmcb requested a review from Anush008 August 3, 2023 06:26
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@Anush008
Copy link
Member

Anush008 commented Aug 3, 2023

Hey. This is great.

@Anush008
Copy link
Member

Anush008 commented Aug 3, 2023

Maybe I am missing something, how can the Repo-Query container establish a connection with the Qdrant instance running on a separate container? Since it'll be making requests to its port 6334 for connecting to Qdrant, should we set the --network flag as host?

@jpmcb
Copy link
Member Author

jpmcb commented Aug 3, 2023

how can the Repo-Query container establish a connection with the Qdrant instance running on a separate container?

The typical default network for docker is the bridge network which many containers can attach to and enable communication between them all. The bridge network also has the advantage of working in isolation and preventing clients outside docker's known iptable to connect.

So, in my example, repo-query attaches to port 6333 and 6334 on the bridge network. And the repo-query container attaches to 3000 on the bridge network.

Running my example above, you can see both the containers on the bridge network with:

$ docker network inspect bridge

Since they are on the same network, they can communicate. The repo-query client can egress traffic to the qdrant database without also needing the 6333 port attached to it: just like starting a web server locally and using the curl client to hit it on some localhost, curl doesn't need the port allocated to it.

Were you having problems running this? Is there something missing where the repo-query engine also needs the 6333 port? I'm going to add some docs and abit more info to flesh this out fully.

Using the host network can be problematic for local use cases. For people just wanting to try it, there may be port allocation conflicts with allocated ports on the host machine and it opens up any potential client to connect to the container. Again, for the local development flow, this isn't really ideal. But for production needs, you usually do need the host network to ensure the allocated port is accessible from the host's attached network. I.e., using docker's host network removes all isolation gained through the bridge network.

Another thing to mention is that host networks don't work on Docker Desktop: you need a more pure linux type docker daemon deployment. Since Docker Desktop spins up a linux virtual machine, attaching to that virtual machine's host network won't achieve the same result.

@jpmcb
Copy link
Member Author

jpmcb commented Aug 3, 2023

Another thing to consider is that we don't want to have a container that has both the repo-query engine and the qdrant database tied together. Ideally, the repo-query engine can exist as a sole microservice that can connect to any qdrant database (running in a different container, running as a cloud service, etc.) and can execute tasks without being directly attached to monolithic data. Coupling those two together will be painful when scaling is needed.

- Includes a makefile for easier operations
- Fixes problem with empty env variable strings being populated in
  repo-query

Signed-off-by: John McBride <john@opensauced.pizza>
@Anush008
Copy link
Member

Anush008 commented Aug 3, 2023

I had changed my Docker network configs which ended up in the two containers running on different networks instead of the default bridge that you mentioned. After reverting those changes and with docker-compose, the service worked as expected. Thank you.

Copy link
Member

@bdougie bdougie left a comment

Choose a reason for hiding this comment

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

This is nice and clear for me to grok 👍🏾

also works for me locally. Thanks for updating the README with context as well.

@jpmcb jpmcb merged commit e786e30 into open-sauced:alpha Aug 3, 2023
1 check passed
@jpmcb jpmcb deleted the dockerfile branch August 3, 2023 17:50
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.

None yet

3 participants