-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Update to alpine and dockerize #107
Conversation
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.
Thank you @Xaseron for your contribution.
I'm aware to update to alpine
and use dockerize
(I didn't know this project) instead of the wait-for-it.sh
script.
For ports removal, it could be helpful for debugging to reach the REST API. And we have to access to server UI (ie. Zipkin, SPring Boot Admin, Server Config, Eureka)
I also have a question : why do you inscrease memory limit from 256M to 1024 ?
docker-compose.yml
Outdated
@@ -3,14 +3,14 @@ services: | |||
config-server: | |||
image: mszarlinski/spring-petclinic-config-server | |||
container_name: config-server | |||
mem_limit: 256M | |||
mem_limit: 1024M |
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.
What is the reason to increase the memory limit from 256M to 1024M ?
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 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.
Ok for the port. According to your screenshot, 512M should be ok. Does it?
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.
Yes, it works and i have changed the pull request accordingly.
@mszarlinski could you also review this PR please? |
|
The config container simply crashes during startup with 256M memory limit |
@Xaseron and @mszarlinski, what is your OS? |
% uname -a
% docker version
|
|
Could you post a |
I've reached the memory limit:
And 3 containers have crashed |
Here is
|
Memory limits looks like to depend of OS and configuration. I suggest to increase the memory limit to 512m. It's a good compromise. |
This reverts commit a605aaf
Ok, i've rebased onto master.
Addionally i set the environment variables for the zipkin server. |
Ok thank you @Xaseron. Are you OK for the merge @mszarlinski ? |
* Add alpine jre and dockerize script * Add docker build argument DOCKERIZE_VERSION * Increase memory limit * Use dockerize instead of wait-for-it script * Update documentation dockerize script * Remove unnecessary ports * Revert "Remove unnecessary ports" This reverts commit a605aaf * Set memory limit to 512M * Merge openzipkin * Remove obsolete links * Fix honoring memory limits for openzipkin
Benefits:
mvn clean install -PbuildDocker -DskipTests
) 90s vs 120sThe problem with the wait-for-it.sh script is that it will not fail if the timeout is exceeded and contiune with the execution even if the requested service is not available.
Exposing ports in the docker-compose file is only necessary if the port is accessed outside the docker network and that is only the case for the gateway service.