PM-1428 Dockerfile, docker compose

[Smorci]

Dockerfile tested locally with commands mentioned in the README.

Repository root will be the package root according to best practices.

[johnmarcou]

suggestion: It is nice to provide a docker-compose.yaml file to test docker image building and docker container running for the application. I suggest to start with this:

services:
  bot:
    image: gpt-survey-summarizer
    build: .
    command: bot
    environment:
      CLIENT_ID: ""
      DISCORD_TOKEN: ""
      GUILD_ID: ""
      REDIS_URL: "redis://redis:6379"
      SUMMARIZE_FREQUENCY_SECONDS: ""
  summarizer:
    image: gpt-survey-summarizer
    build: .
    command: summarizer
    environment:
      OPENAI_API_KEY: ""
      REDIS_URL: "redis://redis:6379"
      SUMMARIZE_FREQUENCY_SECONDS: ""
  redis:
    image: redis

This simplify local testing using:

$ docker-compose up --build

[johnmarcou]

thought: We could add an entrypoint.sh script and make sure that the Redis endpoint is accessible before starting the processes, and therefore avoid a crashloop when deployed on Kubernetes, but let's keep this for later for now and implement if we feel the need in the future.

[johnmarcou]

thought: We could add an entrypoint.sh script and make sure that the Redis endpoint is accessible before starting the processes, and therefore avoid a crashloop when deployed on Kubernetes, but let's keep this for later for now and implement if we feel the need in the future.

I had a look at how to implement this. Here is an example about how bitnami/argo-cd helm chart is doing it: https://github.com/bitnami/charts/blob/main/bitnami/argo-cd/templates/server/deployment.yaml#L106-L112.

They are using an init-container to check the redis connection before start the pod's containers.

The while_retry function is baked into the container image as following:

retry_while() {
    local cmd="${1:?cmd is missing}"
    local retries="${2:-12}"
    local sleep_time="${3:-5}"
    local return_value=1

    read -r -a command <<< "$cmd"
    for ((i = 1 ; i <= retries ; i+=1 )); do
        "${command[@]}" && return_value=0 && break
        sleep "$sleep_time"
    done
    return $return_value
}

The function is doing 12 retries, and finally times out to let the container exit to lead to CrashLoopBackOff.

This is a nice implementation, and avoids to change the entrypoint of the container. I think we wouldn't want to go that far at the moment, especially we are not yet facing any issue. Also, the application is still at early development stage, many things can still change. Let's reconsider this only if we have crash-loop issues in the future

I suggest to revert the entrypoint.sh change for now. What do you think @Smorci?

[johnmarcou]

suggestion: Would you mind adding this to the CODEOWNERS file in the mean time please?

* @es92
+ /Dockerfile @MinaFoundation/infra-eng-reviewers
+ /docker-compose.yaml @MinaFoundation/infra-eng-reviewers

[Smorci]

I suggest to revert the entrypoint.sh change for now. What do you think @Smorci?

We could leave the entrypoint and make it only check for the REDIS_URL env var, and start the desired application. I suggest just removing the connection check functionality.

[johnmarcou]

I suggest to revert the entrypoint.sh change for now. What do you think @Smorci?

We could leave the entrypoint and make it only check for the REDIS_URL env var, and start the desired application. I suggest just removing the connection check functionality.

If we implement only config checking for REDIS_URL, then we should do it for other required variables too. Also, the process would already fail on start anyway, so the entrypoint wouldn't bring much value.

I think we should keep things simple, and fully implement the entrypoint only when we need it.