eliza icon indicating copy to clipboard operation
eliza copied to clipboard

fix: Update Dockerfile

Open Freytes opened this issue 1 year ago • 9 comments

The last line of the docker file needs to be:

CMD ["tail", "-f", "/dev/null"]

Or else it runs the container twice and fails because it's already running on port 3000

Freytes avatar Dec 03 '24 06:12 Freytes

@HashWarlock i know you set this up like this for a reason, wanna just take a look and confirm

lalalune avatar Dec 03 '24 08:12 lalalune

What actually starts it now though?

odilitime avatar Dec 03 '24 15:12 odilitime

The last line which was modified enters the container once completed.

Freytes avatar Dec 03 '24 17:12 Freytes

I believe you are running the container using docker-compose which is also running a command on the container, I recommend removing that line from the compose file, not sending the Dockerfile to nothing at /dev/null

@Freytes I'd revert the Dockerfile change and leave as it was on origin/main and instead make this PR remove line 3 from the docker-compose file:

Remove command: ["pnpm", "start"] in docker-compose.yaml

This will solve your problem while also allowing the Dockerfile to deploy to many clouds such as AWS seamlessly. @odilitime

rarepepi avatar Dec 03 '24 21:12 rarepepi

What actually starts it now though?

Right now the code for pnpm docker is here https://github.com/ai16z/eliza/blob/main/scripts/docker.sh . This was implemented originally by @oberlinstands and probably had a different dev workflow bc the eliza was not built and only the packages were installed. This is why the /dev/null may have been required bc the dev needed to build eliza then pnpm start inside the container.

In the package.json file we can see pnpm docker will execute this script for all 3 docker commands build, run and bash.

"docker:build": "bash ./scripts/docker.sh build",
"docker:run": "bash ./scripts/docker.sh run",
"docker:bash": "bash ./scripts/docker.sh bash",
"docker:start": "bash ./scripts/docker.sh start",
"docker": "pnpm docker:build && pnpm docker:run && pnpm docker:bash",

https://github.com/ai16z/eliza/blob/438c1f1400e365510cae9c19dfc35ca4f663512d/package.json#L20C10-L20C78

If checking the docker run script, we can see that the command is executed based on this logic

 # Start building the docker run command
CMD="docker run --platform linux/amd64 -p 3000:3000 -d"

# Add base mounts
for mount in "${BASE_MOUNTS[@]}"; do
   CMD="$CMD -v \"$(pwd)/$mount\""
done

# Add package mounts
for package in "${PACKAGES[@]}"; do
   CMD="$CMD -v \"$(pwd)/packages/$package/src:/app/packages/$package/src\""
done

# Add core types mount separately (special case)
CMD="$CMD -v \"$(pwd)/packages/core/types:/app/packages/core/types\""

# Add container name and image
CMD="$CMD --name eliza eliza"

# Execute the command
eval $CMD
;;

For the last command docker bash, then we see that this. may be the culprit since it will try run

# Check if the container is running before executing bash
if [ "$(docker ps -q -f name=eliza)" ]; then
   docker exec -it eliza bash
else
   echo "Container 'eliza' is not running. Please start it first."
   exit 1
fi
;;

If I run a test on pnpm docker, I will see this result: image

I think the change we are looking for is to remove bash command from the pnpm docker package script & let the docker image be built and run. If there are errors whenever this happens, then execute pnpm docker:bash to debug the docker image & find out what the problems are.

HashWarlock avatar Dec 03 '24 23:12 HashWarlock

Is this good to go?

lalalune avatar Dec 11 '24 22:12 lalalune

@lalalune I believe the original PR opener was trying to revert back to an old version where the Docker image wasn't running anything because they are using docker-compose which would double run the start command.

I had previously gotten a PR merged #796 to fix the Docker setup to work better in Cloud environments like AWS ECS by adding a new non-interactive flag to the start script. I think the solution here would be to remove the double running of a command in the docker-compose file and leave the Docker image as is or else it would break Cloud running environments.

I could fix this PR and make sure all 3 ways of running with Docker work:

  1. Cloud Deploy Images
  2. Docker Compose
  3. docker package.json script

rarepepi avatar Dec 11 '24 23:12 rarepepi

@lalalune I believe the original PR opener was trying to revert back to an old version where the Docker image wasn't running anything because they are using docker-compose which would double run the start command.

I had previously gotten a PR merged #796 to fix the Docker setup to work better in Cloud environments like AWS ECS by adding a new non-interactive flag to the start script. I think the solution here would be to remove the double running of a command in the docker-compose file and leave the Docker image as is or else it would break Cloud running environments.

I could fix this PR and make sure all 3 ways of running with Docker work:

  1. Cloud Deploy Images
  2. Docker Compose
  3. docker package.json script

that would be great!

lalalune avatar Dec 15 '24 10:12 lalalune

@lalalune I created #1139 to merge into new develop branch to fix docker-compose file issue from original poster

rarepepi avatar Dec 16 '24 20:12 rarepepi

#1139 has replaced this PR. closing it.

odilitime avatar Dec 17 '24 16:12 odilitime