osixia / docker-keepalived

Keepalived container image 🐳🌴
MIT License
395 stars 143 forks source link

curl - check script #29

Open ReSearchITEng opened 5 years ago

ReSearchITEng commented 5 years ago

Provided that keepalived is mainly used for web purposes, and provided that keepalived usually needs a check script, I think the image needs curl package.

(see also #27).

ReSearchITEng commented 5 years ago

I noticed that the docker image on docker-hub does not match the Dockerfile. https://github.com/osixia/docker-keepalived/blob/stable/image/Dockerfile -> does contain curl, which the images from https://hub.docker.com/r/osixia/keepalived do not...

BertrandGouny commented 5 years ago

@ReSearchITEng it's the same file use on github and for the docker image curl is installed and then removed check
https://github.com/osixia/docker-keepalived/blob/ed415f61e038ab06b300fbb799561910db96fffd/image/Dockerfile#L36

ReSearchITEng commented 5 years ago

yes, it's there in the Dockerfile, but not in the images... Am I doing something wrong?

 docker run -d --name test osixia/keepalived:latest

 docker exec test curl
rpc error: code = 2 desc = oci runtime error: exec failed: container_linux.go:247: starting container process caused "exec: \"curl\": executable file not found in $PATH"

docker exec test /bin/curl
rpc error: code = 2 desc = oci runtime error: exec failed: container_linux.go:247: starting container process caused "exec: \"/bin/curl\": stat /bin/curl: no such file or directory"

[docker@docker01:0 ~]$ docker exec -ti test bash
bash-4.3# curl
bash: curl: command not found
bash-4.3# which bash
/bin/bash
bash-4.3# which curl
bash-4.3# which curl | wc -l
0
BertrandGouny commented 5 years ago

in the same run statement curl is installed to download keepalived sources and then removed, so yes it's not available in the docker image

install : https://github.com/osixia/docker-keepalived/blob/ed415f61e038ab06b300fbb799561910db96fffd/image/Dockerfile#L11

removed
https://github.com/osixia/docker-keepalived/blob/ed415f61e038ab06b300fbb799561910db96fffd/image/Dockerfile#L36

ReSearchITEng commented 5 years ago

Oh, sorry, I see the remove. so, can we keep it, for the reason presented in this issue: check scripts usually do curl to figure out if their servers are healthy. Other than that, it's a great image, where one can mount inside config and check scripts, without providing huge amounts of params. (We have templates for config, so no need for passing too many params).

Should you agree with it, here is the PR: https://github.com/osixia/docker-keepalived/pull/30#

BertrandGouny commented 5 years ago

Thanks for the PR will try no make a release later today :)

ReSearchITEng commented 5 years ago

Thanks for accepting the change. FYI, we've added your docker image as the default keepalived option for to our project kubernetes installation project: https://github.com/ReSearchITEng/kubeadm-playbook/blob/50a26fd52ae32b6782caf23e941f44667771ede7/roles/keepalived/tasks/main.yaml#L26

linkvt commented 3 years ago

@BertrandGouny ticket can be closed, curl exists inside the container.