aws-ecr-proxy icon indicating copy to clipboard operation
aws-ecr-proxy copied to clipboard

Consider adding support for process manager e.g supervisor

Open mqasimsarfraz opened this issue 7 years ago • 3 comments

This is a great project but as an improvement it should be considered to use a process manager e.g supercvisor along with crond to run multiple scripts in reliable way. Let me know if the idea seems interesting I will be happy to contribute.

mqasimsarfraz avatar Nov 11 '17 20:11 mqasimsarfraz

Thanks Qasim. Regarding the improvements I'm sure there are many ways of improving the project but I believe it depends also on your use case. The recommended way to use this container is with an orchestrator, something which can do health checks. In the repository there is a kubernetes deployment with a health check, that will ensure the container is restarted in case of a problem. /ping usually gives you the registry status. I believe the container is used most of the time as a temporary side container for CI. Will be nice to hear how others are using this container. To be honest I would not recommend the additional of supervisord and cron. This should encourage everyone to use this project, as it was intended, with a container orchestrator or as a temporary side container.

Any PR is welcome as long as it doesn't break the backwards compatibility and it keeps things clean and slim.

catalinpan avatar Nov 13 '17 00:11 catalinpan

Thanks @catalinpan for a detailed response. I agree with your idea of health checks, containers in production should be coupled with orchestration engines. Just for clarity if renew_token.sh script crashes for some reason the repository will restart after the token expires and heath checks returns an error. Is this understanding correct?

mqasimsarfraz avatar Nov 13 '17 00:11 mqasimsarfraz

Kubernetes health check expects code 200. As you've mentioned if the renew_token.sh fails for any reason and the token is not renewed the health check will not get a 200 and it will fail. The orchestrator should restart the container in this case. As I've said there is space for improvements and that renew_token.sh script can even kill nginx if it cannot renew the token. This should restart the container without the orchestrator attention.

catalinpan avatar Nov 14 '17 10:11 catalinpan